[ruby-core:41403] Re: [Backport93 - Backport #5621] Please backport thread-safe autoloading patch

Yehuda Katz
(ph) 718.877.1325


On Tue, Nov 29, 2011 at 2:43 AM, Charles Oliver Nutter
<[e-mail]>wrote:

> On Mon, Nov 21, 2011 at 4:18 AM, Hiroshi Nakamura <[e-mail]>
> 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
>
>