From: Yehuda Katz Date: 2011-11-30T11:41:57+09:00 Subject: [ruby-core:41403] Re: [Backport93 - Backport #5621] Please backport thread-safe autoloading patch --0016364eef7ab794c304b2eaad7b Content-Type: text/plain; charset=ISO-8859-1 Yehuda Katz (ph) 718.877.1325 On Tue, Nov 29, 2011 at 2:43 AM, Charles Oliver Nutter wrote: > On Mon, Nov 21, 2011 at 4:18 AM, Hiroshi Nakamura > wrote: > > I see. Maybe I misunderstood 'a single lock' concept of 1). 1) is not > > needed, and 2) should be enough for safe classloading normally. > > Initialization of classes (and interfaces) are defined in JLS[1] > > carefully, and no deadlock occurred in normal classloading scheme > > (parent to child.) > > > > Customized classloader can cause a deadlock, and Java SE 7 introduced a > > manual way to avoid this for classloader developer. > > > > [1] > > > http://java.sun.com/docs/books/jls/third_edition/html/execution.html#12.4.2 > > [2] > http://docs.oracle.com/javase/7/docs/technotes/guides/lang/cl-mt.html > > Yes, custom classloaders that delegate in odd ways (cross-hierarchy, > where you have crossing child/parent relationships) could certainly > deadlock. It was a bit of an odd situation, though, since normally > classloaders follow a normal child/parent relationship and just go up > a hierarchy. For the cases where you need to do this more complicated > loading, it looks like they've fixed the locking to consider both the > class under load and the classloader loading it, so you don't end up > with a deadlock. > > > Sure. We cannot control code execution order of initializers (that's > > just a chunk of code in Ruby) and deadlock is inherently unavoidable if > > developer writes a 'wrong code' such as; > ... > > As the existing warning 'circular require considered harmful', current > > Ruby treats it as a developer's job to avoid this 'wrong code'. I was > > trying to solve threading issues of require and autoload, and I think we > > solved it at #921 unless developers don't write 'wrong code'. > > > > I understand that Yehuda and you are saying that it's not a 'wrong > > code.' It's hard to define 'wrong code' I agree. I still wanna see > > concrete examples which are not 'wrong code' but I think it's OK to fix > > the issue inherently by introducing global (per VM) file loading lock if > > it doesn't cause serious degradation in 2.0. > > > > Isn't there an application which uses Threads + parallel execution by > > "load" method? I don't have concrete example :) > > You replied later that you meant "require" not "load". I think > Yehuda's case is not too exotic, and could conceivably happen. For the > simple case of two threads autoloading at the same time, if autoload > being made single-threaded (single global lock for all autoloads) > would be enough. If autoloads and normal requires fire at the same > time, however, it could still deadlock. This would happen if an > autoload executes concurrently with another normal require. That's a > bit more exotic. > > I'm stil at a loss to describe a scenario where parallel 'require' is > what you *want* to happen, but I recognize it would be a visible > change from today. > If you have to choose between making requires threadsafe and enabling parallel requires, making requires threadsafe clearly wins. > > >> I think we could claim autoload is thread-safe in itself if it always > >> just used a single lock. In other words, only one thread can be > >> processing autoloads at a time. This doesn't fix the parallel require > >> issue, but it would fix parallel autoloading. > >> > >> I think that's an ok fix, no? > > > > Agreed. At least it's worth trying. > > > > I'll create a new ticket for the change (single lock require.). This > > ticket is for backporting thread-safe autoload (unless you don't write > > 'wrong code' :-) by Mike. I'll wait for the report if the existing > > thread-safe autoload on trunk works for him or not. > > Agreed. At least we can say "concurrent autoloads won't happen" and as > a result they're 100% safe. Concurrent requires still have concerns, > but they're less likely to happen because people genreally don't do > lazy requires in threads unless they're triggered by autoloads. > For reasons I described earlier, Rails emulates autoload via requires, so any threadsafety fix that relies on autoload would either need to mitigate our need to emulate autoload (by allowing something like autoload "Foo::Bar", "foo/bar") or also support threadsafe require. That said, I don't see why we couldn't support threadsafe require for cases where lazy require makes sense. > I think an additional warning about doing requires lazily might be > useful...perhaps any require that happens on some non-main thread that > *isn't* in response to a require should elicit a warning? Or perhaps > when we detect concurrent non-autooad requires (i.e. two threads doing > separate requires at the same time, regardless of the files they're > loading) we should warn? > See above. I would rather just get a single lock around require and make things safe. > > - Charlie > > --0016364eef7ab794c304b2eaad7b Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable
Yehuda Katz
(ph) 718.877.1325


On Tue, Nov 29, 2011 at 2:43 AM, Charles= Oliver Nutter <headius@headius.com> wrote:
On Mon, Nov 21, 2011 at 4:18 AM, Hiroshi Nakamura <nahi@ruby-lang.org> wrote:
> I see. =A0Maybe I misunderstood 'a single lock' concept of 1).= =A01) is not
> needed, and 2) should be enough for safe classloading normally.
> Initialization of classes (and interfaces) are defined in JLS[1]
> carefully, and no deadlock occurred in normal classloading scheme
> (parent to child.)
>
> Customized classloader can cause a deadlock, and Java SE 7 introduced = a
> manual way to avoid this for classloader developer.
>
> [1]
> http://java.sun.com/docs/books/jls/thir= d_edition/html/execution.html#12.4.2
> [2] http://docs.oracle.com/javase/7/docs/tech= notes/guides/lang/cl-mt.html

Yes, custom classloaders that delegate in odd ways (cross-hierarchy,<= br> where you have crossing child/parent relationships) could certainly
deadlock. It was a bit of an odd situation, though, since normally
classloaders follow a normal child/parent relationship and just go up
a hierarchy. For the cases where you need to do this more complicated
loading, it looks like they've fixed the locking to consider both the class under load and the classloader loading it, so you don't end up with a deadlock.

> Sure. We cannot control code execution order of initializers (that'= ;s
> just a chunk of code in Ruby) and deadlock is inherently unavoidable i= f
> developer writes a 'wrong code' such as;
...
> As the existing warning 'circular require consid= ered harmful', current
> Ruby treats it as a developer's job to avoid this 'wrong code&= #39;. =A0I was
> trying to solve threading issues of require and autoload, and I think = we
> solved it at #921 unless developers don't write 'wrong code= 9;.
>
> I understand that Yehuda and you are saying that it's not a 'w= rong
> code.' =A0It's hard to define 'wrong code' I agree. = =A0I still wanna see
> concrete examples which are not 'wrong code' but I think it= 9;s OK to fix
> the issue inherently by introducing global (per VM) file loading lock = if
> it doesn't cause serious degradation in 2.0.
>
> Isn't there an application which uses Threads + parallel execution= by
> "load" method? =A0I don't have concrete example :)

You replied later that you meant "require" not "load&q= uot;. I think
Yehuda's case is not too exotic, and could conceivably happen. For the<= br> simple case of two threads autoloading at the same time, if autoload
being made single-threaded (single global lock for all autoloads)
would be enough. If autoloads and normal requires fire at the same
time, however, it could still deadlock. This would happen if an
autoload executes concurrently with another normal require. That's a bit more exotic.

I'm stil at a loss to describe a scenario where parallel 'require&#= 39; is
what you *want* to happen, but I recognize it would be a visible
change from today.

If you have to choos= e between making requires threadsafe and enabling parallel requires, making= requires threadsafe clearly wins.
=A0

>> I think we could claim autoload is thread-safe in itself if it alw= ays
>> just used a single lock. In other words, only one thread can be >> processing autoloads at a time. This doesn't fix the parallel = require
>> issue, but it would fix parallel autoloading.
>>
>> I think that's an ok fix, no?
>
> Agreed. =A0At least it's worth trying.
>
> I'll create a new ticket for the change (single lock require.). = =A0This
> ticket is for backporting thread-safe autoload (unless you don't w= rite
> 'wrong code' :-) by Mike. =A0I'll wait for the report if t= he existing
> thread-safe autoload on trunk works for him or not.

Agreed. At least we can say "concurrent autoloads won't happ= en" and as
a result they're 100% safe. Concurrent requires still have concerns, but they're less likely to happen because people genreally don't do=
lazy requires in threads unless they're triggered by autoloads.

For reasons I described earlier, Rails emulate= s autoload via requires, so any threadsafety fix that relies on autoload wo= uld either need to mitigate our need to emulate autoload (by allowing somet= hing like autoload "Foo::Bar", "foo/bar") or also suppo= rt threadsafe require.

That said, I don't see why we couldn't support = threadsafe require for cases where lazy require makes sense.
=A0<= /div>
I think an additional warning about doing requires lazily might be
useful...perhaps any require that happens on some non-main thread that
*isn't* in response to a require should elicit a warning? Or perhaps when we detect concurrent non-autooad requires (i.e. two threads doing
separate requires at the same time, regardless of the files they're
loading) we should warn?

See above. I w= ould rather just get a single lock around require and make things safe.
=A0

- Charlie


--0016364eef7ab794c304b2eaad7b--