From: Charles Oliver Nutter <headius@...>
Date: 2011-11-29T19:43:56+09:00
Subject: [ruby-core:41388] Re: [Backport93 - Backport #5621] Please backport thread-safe autoloading patch

On Mon, Nov 21, 2011 at 4:18 AM, Hiroshi Nakamura <nahi@ruby-lang.org> 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.

>> 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.

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?

- Charlie