Backport #5621: Please backport thread-safe autoloading patch

• Status: Rejected
• Assignee: -

[mperham (Mike Perham)]

This is critical to using Rails in thread-safe mode. JRuby is supposed to have it in the next release, 1.7.

http://redmine.ruby-lang.org/issues/921

Waiting another 12-18 months for 1.9.4 to come out would really suck.

[brianmario (Brian Lopez)]

+1

[jc00ke (Jesse Cooke)]

+1

[nahi (Hiroshi Nakamura)]

Thanks for requests guys! Would you please post a concrete example? i.e. stacktrace or explanation of a problem you're facing.

And does trunk version solve the problem? Current trunk is not so different from ruby_1_9_3 at present. You should be able to try trunk version easily now. I want to make sure that the fix for #921 surely fix the problem.

Thanks in advance.

[fbjork (Fredrik Björk)]

+1

[mperham (Mike Perham)]

My use case: I want to use Rails in thread-safe mode in production so I don't have to fork 20 Ruby processes to handle 20 concurrent requests. When developing, Rails auto loads classes on every request. This auto loading is critical to Rails' rapid development cycle and there's no way I'm going to disable it. Unfortunately the current autoload behavior makes the development environment incompatible with thread-safe mode.

Since no developer wants to lose the rapid development cycle and no developer is crazy enough to run a very different configuration in production than development (going from non-thread-safe to thread-safe between dev and prod is a huge change), no one currently uses thread-safe mode.

I will try trunk in a few days and report back if it solves the problem.

[wycats (Yehuda Katz)]

Unfortunately ruby-head has a deadlock in one of my go-to scenarios for
this feature.

foo.rb

class Foo
sleep 2

def self.shared_attributes
%w(some list)
end

include Bar
end

bar.rb

sleep 1

module Bar
attr_accessor *Foo.shared_attributes
end

test.rb

autoload :Foo, "./foo"
autoload :Bar, "./bar"

t1 = Thread.new { Foo }
t2 = Thread.new { Bar }

t1.join
t2.join

RUBY 1.9.3 OUTPUT

/Users/wycats/Code/tmp/test_autoload/bar.rb:6:in <module:Bar>': undefined method shared_attributes' for Foo:Class (NoMethodError)
from /Users/wycats/Code/tmp/test_autoload/bar.rb:5:in <top (required)>' from test_autoload.rb:5:in block in '

RUBY 1.9.4 OUTPUT

test_autoload.rb:7:in join': deadlock detected (fatal) from test_autoload.rb:7:in '

Yehuda Katz
(ph) 718.877.1325

On Sat, Nov 12, 2011 at 8:58 AM, Mike Perham [e-mail] wrote:

Issue #5621 has been updated by Mike Perham.

My use case: I want to use Rails in thread-safe mode in production so I
don't have to fork 20 Ruby processes to handle 20 concurrent requests.
When developing, Rails auto loads classes on every request. This auto
loading is critical to Rails' rapid development cycle and there's no way
I'm going to disable it. Unfortunately the current autoload behavior makes
the development environment incompatible with thread-safe mode.

Since no developer wants to lose the rapid development cycle and no
developer is crazy enough to run a very different configuration in
production than development (going from non-thread-safe to thread-safe
between dev and prod is a huge change), no one currently uses thread-safe
mode.

I will try trunk in a few days and report back if it solves the problem.

Backport #5621: Please backport thread-safe autoloading patch
http://redmine.ruby-lang.org/issues/5621

Author: Mike Perham
Status: Open
Priority: Normal
Assignee:
Category:
Target version:

This is critical to using Rails in thread-safe mode. JRuby is supposed to
have it in the next release, 1.7.

http://redmine.ruby-lang.org/issues/921

Waiting another 12-18 months for 1.9.4 to come out would really suck.

--
http://redmine.ruby-lang.org

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

(2011/11/13 5:43), Yehuda Katz wrote:

Unfortunately ruby-head has a deadlock in one of my go-to scenarios
for this feature.

Just posting a seed for more discussion... This non-autoload version
causes deadlock with trunk, 1.9.3 and 1.8.7.

foo.rb

class Foo
sleep 2

def self.shared_attributes
%w(some list)
end

require 'bar'
include Bar
end

bar.rb

sleep 1

module Bar
require 'foo'
attr_accessor *Foo.shared_attributes
end

test.rb

t1 = Thread.new { require "foo"; Foo }
t2 = Thread.new { require "bar"; Bar }

t1.join
t2.join
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)

iQEcBAEBAgAGBQJOvxw6AAoJEC7N6P3yLbI2GMMH/1eb15orhb/9Ei1cc4uRBnru
7+QFI1KHcSg9HaHZPbXf61BVgjIpO2d0kmZS9UwMOX2eNtRxnSZbnlo+y3HfK8TZ
aLzcFVS5NDtg+sehORv9ZvcR/sFo5hk+E6ukt186vrtjFFJj5q4pyIklhNOhFoXr
Y4of3jdGv481/+ubyuFKd2UdHh1RcGWB24i/ZSsxJl6FVHyTDw6WieguuoneIAXY
56gt4iaAzZVhHSeoU4tN+973hPIZROh1gNtZ13aBth77b6ryHaPo6U03uMue0T8t
//SMzugh9B/jQIWCmpzm7EdKah0EBLsqGfdQFEcF+JdHFICDqPh14R6uZfBHgB8=
=a/yq
-----END PGP SIGNATURE-----

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

(2011/11/13 1:58), Mike Perham wrote:

My use case: I want to use Rails in thread-safe mode in production
so I don't have to fork 20 Ruby processes to handle 20 concurrent
requests. When developing, Rails auto loads classes on every
request. This auto loading is critical to Rails' rapid development
cycle and there's no way I'm going to disable it. Unfortunately
the current autoload behavior makes the development environment
incompatible with thread-safe mode.

I should read ActiveSupport source code first but please allow me to
post lazy question. Does Rails development mode use autoload? It is
doing constant lookup by itself I guess, but I could be wrong, of course.

You might be using autoload in your library though...

Since no developer wants to lose the rapid development cycle and no
developer is crazy enough to run a very different configuration in
production than development (going from non-thread-safe to
thread-safe between dev and prod is a huge change), no one
currently uses thread-safe mode.

Ah, thank you. I was not aware of the reason. Good to know. I
indeed want to fix that situation.

I will try trunk in a few days and report back if it solves the
problem.

Thank you! I'm looking forward to hear the result.

// NaHi
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)

iQEcBAEBAgAGBQJOvx7bAAoJEC7N6P3yLbI2eIQH/i5pmRNaQgLg8X0BlRLybXXx
3n+N6+JS00I8EreblBpkvDLy62YXJTy3V+ahKDZupozC2oALRdjk8QlYSH4H3kTp
9R6ZzyCqjpfiilowF5uXaxP7qeGIGgYxBvWIxZgLGwdyyi8DK1YgwD6VklOwsfab
uLNQinuG8Rpbl7ic4Uu+kHmIh3IGDQFsXzxwzbBlmMi8DT+rf2CLGnBkCQFY8kyC
wcp/4s+K+5GJWtVM4yJNSMZhduwNbMZs8iAy6iOItEAFTabXUULWexUlkL3RhFVu
HYDwjCf7NqPFalLfwkWxZEdZpLZ7YifVFNW93XvyaKFuxAzgEFvW8vQ4UJbtVz4=
=bzOU
-----END PGP SIGNATURE-----

On Sun, Nov 13, 2011 at 10:35:42AM +0900, Hiroshi Nakamura wrote:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

(2011/11/13 1:58), Mike Perham wrote:

My use case: I want to use Rails in thread-safe mode in production
so I don't have to fork 20 Ruby processes to handle 20 concurrent
requests. When developing, Rails auto loads classes on every
request. This auto loading is critical to Rails' rapid development
cycle and there's no way I'm going to disable it. Unfortunately
the current autoload behavior makes the development environment
incompatible with thread-safe mode.

I should read ActiveSupport source code first but please allow me to
post lazy question. Does Rails development mode use autoload? It is
doing constant lookup by itself I guess, but I could be wrong, of course.

ActiveSupport doesn't use autoload for loading missing constants, but
supporting libraries definitely use autoloading things. It's quite
possible that files loaded using the missing constant hooks contain
autoload directives. :(

--
Aaron Patterson
http://tenderlovemaking.com/

[wycats (Yehuda Katz)]

We don't use autoload for one simple reason: we support nested autoloads
and don't know ahead of time whether an intermediate module is a module or
class. Consider the following file structure:

|-app
| |-models
| | |-admin
| | | |-user.rb

At various times, I have tried to have Rails' internal structure operate by
setting up autoloads ahead of time and letting Ruby do the rest.
Unfortunately, in this case, we would have to explicitly define Admin, but
we have no way to know whether Admin is a module or class. If we define it
as a module, for instance, and user.rb looks like this:

class Admin
class User < ActiveRecord::Base
…
end
end

we get: "Admin is not a class". As a result, cannot set everything up ahead
of time and have to let the first file that is autoloaded define the
intermediate namespaces.

--- ASIDE ---

While we're on the topic, http://redmine.ruby-lang.org/issues/2740 is
probably the major remaining issue with Rails' autoload solution. To
summarize:

module Foo
def self.const_missing(id)
return lookup(id) if can_lookup?(id)
raise NoConstantError
end

module Bar
def self.const_missing(id)
return lookup(id) if can_lookup?(id)
raise NoConstantError
end
end
end

The reason this is needed is that this:

module Foo
module Bar
Baz
end
end

has different semantics than:

module Foo::Bar
Baz
end

Rails can't tell the difference between these two cases, and therefore
guesses that it's probably the first case. But consider this situation:

foo/array.rb

module Foo
class Array
end
end

module Foo::Bar
Array
end

Because we can't tell from the const_missing call that the nesting is
[Foo::Bar], we assume it's [Foo::Bar, Foo] and load in foo/array.rb even
though that is not the semantically correct behavior. In 99% of cases, this
does not cause any problems, but when it fails (for this and other
reasons), it causes pretty extreme confusion.

Yehuda Katz
(ph) 718.877.1325

On Sun, Nov 13, 2011 at 8:00 AM, Aaron Patterson
[e-mail]:

On Sun, Nov 13, 2011 at 10:35:42AM +0900, Hiroshi Nakamura wrote:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

(2011/11/13 1:58), Mike Perham wrote:

My use case: I want to use Rails in thread-safe mode in production
so I don't have to fork 20 Ruby processes to handle 20 concurrent
requests. When developing, Rails auto loads classes on every
request. This auto loading is critical to Rails' rapid development
cycle and there's no way I'm going to disable it. Unfortunately
the current autoload behavior makes the development environment
incompatible with thread-safe mode.

I should read ActiveSupport source code first but please allow me to
post lazy question. Does Rails development mode use autoload? It is
doing constant lookup by itself I guess, but I could be wrong, of course.

ActiveSupport doesn't use autoload for loading missing constants, but
supporting libraries definitely use autoloading things. It's quite
possible that files loaded using the missing constant hooks contain
autoload directives. :(

--
Aaron Patterson
http://tenderlovemaking.com/

[headius (Charles Nutter)]

On Sat, Nov 12, 2011 at 7:24 PM, Hiroshi Nakamura [e-mail] wrote:

test.rb

t1 = Thread.new { require "foo"; Foo }
t2 = Thread.new { require "bar"; Bar }

The problem here is caused by require having a lock per file. At the
time it was added, I believe I warned it would cause deadlocks in the
future.

Yehuda's simple case could be solved by having all autoloads
synchronized against a single lock; only either the Foo autoload or
the Bar autoload could fire, and then the other would fire once it had
completed.

In general, if there were a single lock for requires/autoloads, there
would be no way it could deadlock.

• Charlie

[headius (Charles Nutter)]

On Fri, Nov 18, 2011 at 12:15 AM, Hiroshi Nakamura [e-mail] wrote:

Sure.  This deadlock from cross-require is intentional at this moment
I believe, and that's the reason for the warning 'loading in progress,
circular require considered harmful'.  Related discussion is at #920.

I don't know how Java's classloader avoids this problem.

Each classloader has a single lock, and if it needs to call a parent
classloader the locks are still always acquired in the same order
(meaning deadlock is impossible).

Yehuda's simple case could be solved by having all autoloads
synchronized against a single lock; only either the Foo autoload
or the Bar autoload could fire, and then the other would fire once
it had completed.

I understand that it's not an autoload issue but a concurrent require
issue, and 'disallowing concurrent requires entirely' in #920 is hard
jump for Ruby...

So I hope someone extract requirements from Yehuda's case for solving
Rails' issue, not by changing require/autoload directly.  I don't have
enough time for it now but there would be some more related
assumptions around it I guess.  For example, they don't want to let
developers write 'require' explicitly, a convention for Ruby's module
hierarchy that must sync with filesystem layout, etc...

I don't see how it can be fixed if concurrent requires are allowed,
but individual files have their own locks. If there's ever any
potential for two different locks to be acquired by two threads in
opposing order, it can deadlock.

You're right, though...it's important to point out Yehuda's problem is
not an autoload problem. Autoload thread-safety itself is achievable.
Require thread-safety doesn't exist even today. That's the real
problem.

• Charlie

(2011/11/19 6:31), Charles Oliver Nutter wrote:

Sure. This deadlock from cross-require is intentional at this
moment I believe, and that's the reason for the warning 'loading
in progress, circular require considered harmful'. Related
discussion is at #920.

I don't know how Java's classloader avoids this problem.

Each classloader has a single lock, and if it needs to call a
parent classloader the locks are still always acquired in the same
order (meaning deadlock is impossible).

1. No parallel loading in single classloader
   (single lock, disallowing concurrent loading)

2. Classloaders know class loading order at compile time to avoid
   deadlock

Am I right?

I don't see how it can be fixed if concurrent requires are
allowed, but individual files have their own locks. If there's ever
any potential for two different locks to be acquired by two threads
in opposing order, it can deadlock.

Indeed. Warning would be the best we can do.

You're right, though...it's important to point out Yehuda's problem
is not an autoload problem. Autoload thread-safety itself is
achievable. Require thread-safety doesn't exist even today. That's
the real problem.

Sure. Moreover, I think that require thread-safety could not be
solved easily in Ruby, so I want to find a different way to solve
Yehuda's real problem.

[headius (Charles Nutter)]

On Fri, Nov 18, 2011 at 5:24 PM, Hiroshi Nakamura [e-mail] wrote:

(2011/11/19 6:31), Charles Oliver Nutter wrote:

Sure.  This deadlock from cross-require is intentional at this
moment I believe, and that's the reason for the warning 'loading
in progress, circular require considered harmful'.  Related
discussion is at #920.

I don't know how Java's classloader avoids this problem.

Each classloader has a single lock, and if it needs to call a
parent classloader the locks are still always acquired in the same
order (meaning deadlock is impossible).

1. No parallel loading in single classloader
     (single lock, disallowing concurrent loading)

Correct.

2. Classloaders know class loading order at compile time to avoid
     deadlock

No, but classloading always progresses in the same direction, from
dependent classes to dependencies and from child classloaders to
parent classloaders, so having multiple locks involved can't deadlock.

Also, classloading does not in itself trigger any code execution; not
until the class has successfully loaded do static initializers run.
Ruby's problem with parallel loading is that files are executed
while they're loaded, which makes running them in parallel
inherently unsafe.

Indeed.  Warning would be the best we can do.

It would have to be a warning saying "you are doing concurrent
requires" rather than a warning for circular requires. The deadlock
does not occur in circular requires...only in requires from across
threads that depend on each others' files.

You're right, though...it's important to point out Yehuda's problem
is not an autoload problem. Autoload thread-safety itself is
achievable. Require thread-safety doesn't exist even today. That's
the real problem.

Sure.  Moreover, I think that require thread-safety could not be
solved easily in Ruby, so I want to find a different way to solve
Yehuda's real problem.

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?

• Charlie

[trans (Thomas Sawyer)]

@Yehuda

At various times, I have tried to have Rails' internal structure operate by
setting up autoloads ahead of time and letting Ruby do the rest.
Unfortunately, in this case, we would have to explicitly define Admin, but
we have no way to know whether Admin is a module or class. If we define it
as a module, for instance, and user.rb looks like this:

class Admin
class User < ActiveRecord::Base
…
end
end

we get: "Admin is not a class". As a result, cannot set everything up ahead
of time and have to let the first file that is autoloaded define the
intermediate namespaces.

This is one of the reasons why I know it would better if the distinction between class and module should be dropped. If you think about it the distinction is simply a idealized construct. The reality of the thing lies in it's usage. It's the same idea behind duck typing.

While we're on the topic, http://redmine.ruby-lang.org/issues/2740 is
probably the major remaining issue with Rails' autoload solution. To
summarize:

There is one other that has been a pain for me: there is no way to override the require mechanism used by autoload as it does not use the Kernel method(s).

(2011/11/19 9:08), Charles Oliver Nutter wrote:

Each classloader has a single lock, and if it needs to call a
parent classloader the locks are still always acquired in the same
order (meaning deadlock is impossible).

1. No parallel loading in single classloader
   (single lock, disallowing concurrent loading)

Correct.

2. Classloaders know class loading order at compile time to avoid
   deadlock

No, but classloading always progresses in the same direction, from
dependent classes to dependencies and from child classloaders to
parent classloaders, so having multiple locks involved can't deadlock.

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

Also, classloading does not in itself trigger any code execution; not
until the class has successfully loaded do static initializers run.
Ruby's problem with parallel loading is that files are executed
while they're loaded, which makes running them in parallel
inherently unsafe.

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;

file foo.rb

class Foo
require 'bar'
Bar.something
end

file bar.rb

class Bar
require 'foo'
Foo.something
end

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're right, though...it's important to point out Yehuda's problem
is not an autoload problem. Autoload thread-safety itself is
achievable. Require thread-safety doesn't exist even today. That's
the real problem.

Sure. Moreover, I think that require thread-safety could not be
solved easily in Ruby, so I want to find a different way to solve
Yehuda's real problem.

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.

[headius (Charles Nutter)]

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.

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

[headius (Charles Nutter)]

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

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?

Here's an impl of the latter. The first thread to start requiring
acquires a lock, and subsequent threads that attempt to require simply
warn. It's primitive, since it warns on those other threads for
every require including the topmost, but it demonstrates the idea.

https://gist.github.com/1404402

Example uses:

system ~/projects/jruby $ jruby -e "Thread.new { require 'fileutils'
}; require 'optparse'"
:1 warning: concurrent require of fileutils detected
/Users/headius/projects/jruby/lib/ruby/1.8/fileutils.rb:979 warning:
concurrent require of etc detected

system ~/projects/jruby $ jruby -e "Thread.new { require 'fileutils'
}; Thread.new { require 'optparse' }"
:1 warning: concurrent require of optparse detected

system ~/projects/jruby $ jruby -e "Thread.new { require 'fileutils'
}.join; require 'optparse'"

system ~/projects/jruby $

[wycats (Yehuda Katz)]

Yehuda Katz
(ph) 718.877.1325

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

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

[naruse (Yui NARUSE)]

How about this and what revisions should be backported?

[nahi (Hiroshi Nakamura)]

I don't think it should be backported at this moment.

• No one confirm that 2.0.0 fixes their issue which is existing in 1.9.3. At least no confirmation from the original reporter. (I guess he could be confusing Ruby's "autoload" with Rails' "nested autoload" which is not "autoload")
• The original change is made before 1.9.3 but we (I) delayed this to 2.0.0. We need extra reason to backport this to 1.9 line.

That's said, JRuby 1.6.6 is shipped with thread-safe autoload already, and I didn't receive any problem for now. If it really helps the concrete problem, I think I can create the patch so that release manager investigate it.

[naruse (Yui NARUSE)]

• Status changed from Open to Rejected