Project

General

Profile

Actions

Bug #12688

closed

Thread unsafety in autoload

Added by headius (Charles Nutter) over 7 years ago. Updated over 4 years ago.

Status:
Closed
Target version:
-
[ruby-core:76974]

Description

I need clarification here. I expected, based on Ruby's assertion that autoloads are thread-safe, that the following code would never error. Instead, it gets a couple iterations in and raises NameError:

loop do
  class Foo
    autoload :Bar, 'bar.rb'
  end

  go = false
  threads = (1..50).map {Thread.new { 1 until go; print '.'; Foo.const_get(:Bar) }}
  go = true
  threads.each(&:join)
  puts

  self.class.send :remove_const, :Foo
end

And the output with Ruby 2.3.0:

$ ruby23 -I. autoload_breaker.rb 
..................................................
..................................................autoload_breaker.rb:7:in `const_get': uninitialized constant Foo::Bar (NameError)
Did you mean?  Foo::Bar
	from autoload_breaker.rb:7:in `block (3 levels) in <main>'

Is there something wrong with my script? Is my expectation incorrect?

Updated by headius (Charles Nutter) over 7 years ago

Oh, sorry...the source of bar.rb is trivial:

class Foo
  Bar = 1
end

Updated by h.shirosaki (Hiroshi Shirosaki) over 7 years ago

Charles Nutter wrote:

I need clarification here. I expected, based on Ruby's assertion that autoloads are thread-safe, that the following code would never error. Instead, it gets a couple iterations in and raises NameError:

loop do
  class Foo
    autoload :Bar, 'bar.rb'
  end

  go = false
  threads = (1..50).map {Thread.new { 1 until go; print '.'; Foo.const_get(:Bar) }}
  go = true
  threads.each(&:join)
  puts

  self.class.send :remove_const, :Foo
end

Is there something wrong with my script? Is my expectation incorrect?

$".pop would be needed to clear bar.rb in loaded features.
I don't get NameError after adding $".pop.

Updated by rosenfeld (Rodrigo Rosenfeld Rosas) over 7 years ago

I have the same expectations as you, Charles, and as so I'd also expect this to be a bug.

Updated by headius (Charles Nutter) over 7 years ago

$".pop would be needed to clear bar.rb in loaded features.
I don't get NameError after adding $".pop.

But why does it work for a while and then stop working? There still seems to be a threading issue here.

Here's my modified script, with the confirmation dots moved after the constant lookup and Thread.abort_on_exception = true:

Thread.abort_on_exception = true

loop do
  class Foo
    autoload :Bar, 'bar.rb'
  end

  go = false
  threads = (1..50).map {Thread.new { 1 until go; Foo.const_get(:Bar); print '.' }}
  go = true
  threads.each(&:join)
  puts

  self.class.send :remove_const, :Foo
end

It successfully runs for less than 50 dots, and then one of the threads errors out. I don't think it should.

$ ruby23 -I. blah.rb
..................................................
blah.rb:9:in `const_get': uninitialized constant Foo::Bar (NameError)
Did you mean?  Foo::Bar
	from blah.rb:9:in `block (3 levels) in <main>'

Note also this is not even getting to a second iteration; the first iteration of the outer loop fails. If I add $".pop after remove_const, it does run for longer...but it still produces a NameError for me.

$ ruby23 -I. blah.rb
..................................................
..................................................
<about 100 rows omitted>
..................................................
..................................................
..................................................
blah.rb:9:in `const_get': uninitialized constant Foo::Bar (NameError)
Did you mean?  Foo::Bar
	from blah.rb:9:in `block (3 levels) in <main>'

Updated by headius (Charles Nutter) over 7 years ago

It successfully runs for less than 50 dots, and then one of the threads errors out. I don't think it should.

Ok, I can't count...it does run through one full iteration and then probably fails because the autoload doesn't actually load anything. But then it still fails a hundred iterations later, so something's still not right.

Updated by shyouhei (Shyouhei Urabe) over 7 years ago

  • Status changed from Open to Assigned
  • Assignee set to ko1 (Koichi Sasada)

Updated by shyouhei (Shyouhei Urabe) over 7 years ago

We looked at this issue in todays developer meeting and had 2 feelings in common.

  1. The autoload should not render NameError. It definitely is a bug that must be fixed. Ko1 is assigned.
  2. Besides, we want to discourage people from doing something like 1 until go. JRuby+Truffle might optimize this out (as far as I understand). Also generally speaking, sharing local variables across threads is something difficult to do properly.

Updated by ko1 (Koichi Sasada) about 7 years ago

  • Status changed from Assigned to Feedback

I can't reproduce headius's issue. It shows 50 dots and stop at next iteration because autoload is failed.

Inserting $".pop Shirosaki san suggested, I don't get any exception.

I tried on current trunk.

$LOAD_PATH.unshift __dir__

Thread.abort_on_exception = true

loop do
  class Foo
    autoload :Bar, 'bar.rb'
  end

  go = false
  threads = (1..50).map {Thread.new { 1 until go; Foo.const_get(:Bar); print '!' }}
  go = true
  threads.each(&:join)
  puts

  self.class.send :remove_const, :Foo
  $".pop
end
Actions #9

Updated by jeremyevans0 (Jeremy Evans) over 4 years ago

  • Status changed from Feedback to Closed
Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0