Skip to content

Hash lookup sometimes fails with Thread objects. #2928

Open
chanks opened this Issue Feb 9, 2014 · 6 comments

2 participants

@chanks
chanks commented Feb 9, 2014

I was writing a small connection pool and I came across a bug when using threads as hash keys. I tried a bit and couldn't reproduce this outside of my spec suite, so I made a branch to demonstrate it:

https://github.com/chanks/pond/tree/rbx_bug

Here's an example of the behavior I'm seeing. Note that it took two tries, it doesn't always show up the first time. I also started stepping through the logic in Hash#find_item to see where it broke down, but I haven't studied the internals of Hash enough to see what the actual problem is.

chris@lovelace ~/pond $ ruby -v
rubinius 2.2.4 (2.1.0 fd07f670 2014-02-01 JI) [x86_64-linux-gnu]
chris@lovelace ~/pond $ rake
/home/chris/.rvm/rubies/rbx-2.2.4/bin/rbx -S rspec ./spec/pond_spec.rb
......................................................................................................................................................................................................................................................................................................
From: /home/chris/pond/spec/pond_spec.rb @ line 146 :

    141: 
    142:       hash = pond.allocated
    143: 
    144:       unless hash.empty?
    145:         require 'pry'
 => 146:         binding.pry
    147:       end
    148: 
    149:       pond.size.should == 4
    150:       results.should == (1..4).cycle(4).to_a
    151:     end

[1] pry(#<RSpec::Core::ExampleGroup::Nested_1>)> hash
=> {#<Thread:0x1a158 id=4599 dead>=>4}
[2] pry(#<RSpec::Core::ExampleGroup::Nested_1>)> t = hash.keys.first
=> #<Thread:0x1a158 id=4599 dead>
[3] pry(#<RSpec::Core::ExampleGroup::Nested_1>)> hash.delete(t)
=> 4
[4] pry(#<RSpec::Core::ExampleGroup::Nested_1>)> exit
.............................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................
From: /home/chris/pond/spec/pond_spec.rb @ line 146 :

    141: 
    142:       hash = pond.allocated
    143: 
    144:       unless hash.empty?
    145:         require 'pry'
 => 146:         binding.pry
    147:       end
    148: 
    149:       pond.size.should == 4
    150:       results.should == (1..4).cycle(4).to_a
    151:     end

[1] pry(#<RSpec::Core::ExampleGroup::Nested_1>)> hash
=> {#<Thread:0x3132c id=15562 dead>=>3}
[2] pry(#<RSpec::Core::ExampleGroup::Nested_1>)> t = hash.keys.first
=> #<Thread:0x3132c id=15562 dead>
[3] pry(#<RSpec::Core::ExampleGroup::Nested_1>)> hash.delete(t)
=> nil
[4] pry(#<RSpec::Core::ExampleGroup::Nested_1>)> hash
=> {#<Thread:0x3132c id=15562 dead>=>3}
[5] pry(#<RSpec::Core::ExampleGroup::Nested_1>)> key_hash = t.hash
=> 201516
[6] pry(#<RSpec::Core::ExampleGroup::Nested_1>)> entries = hash.send(:instance_variable_get, :@entries)
=> #<Rubinius::Tuple: #<Hash::Bucket:0x32fac @link=nil @next=nil @state=#<Hash::State:0x32fb0 @tail=#<Hash::Bucket:0x32fac ...> @compare_by_identity=false @head=#<Hash::Bucket:0x32fac ...>> @value=3 @previous=nil @key=#<Thread:0x3132c id=15562 dead> @key_hash=0>, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil>
[7] pry(#<RSpec::Core::ExampleGroup::Nested_1>)> key_index = hash.send(:key_index, key_hash)
=> 12
[8] pry(#<RSpec::Core::ExampleGroup::Nested_1>)> entries[key_index]
=> nil
@chanks
chanks commented Feb 15, 2014

Digging into this some more, it seems that the problem is @key_hash in that Hash::Bucket instance being set to 0.

Here's a smaller reproduction that works about half the time for me:

hash = {}

loop do
  threads = 10.times.map do
    Thread.new do
      hash[Thread.current] = Thread.current.hash
    end
  end

  threads.map &:join
  threads.each { |t| hash.delete(t) }
  keys   = hash.keys
  values = hash.values

  raise "bad: #{keys} #{values} #{hash}" unless keys.empty?
end



An exception occurred running bug.rb:

    bad: [#<Thread:0x829c id=8283 dead>] [33436] {#<Thread:0x829c id=8283 dead>=>33436} (RuntimeError)

Backtrace:

          { } in Object#__script__ at bug.rb:15
               Kernel(Object)#loop at kernel/common/kernel.rb:460
                 Object#__script__ at bug.rb:3
  Rubinius::CodeLoader#load_script at kernel/delta/code_loader.rb:66
  Rubinius::CodeLoader.load_script at kernel/delta/code_loader.rb:152
           Rubinius::Loader#script at kernel/loader.rb:649
             Rubinius::Loader#main at kernel/loader.rb:831
@dbussink
Rubinius member

It looks like you expect Hash to be thread safe here, which it is not. If you use the same Hash across multiple threads, you need to lock around any access to that Hash.

Another idea is to look at a gem like https://rubygems.org/gems/thread_safe which provides a thread safe hash implementation.

@chanks
chanks commented Feb 15, 2014

@dbussink Thanks, but I don't think that's it. The hash was protected in my initial reproduction. Here's the shorter reproduction with an added mutex, it exhibits the same behavior:

hash = {}
mutex = Mutex.new

loop do
  threads = 10.times.map do
    Thread.new do
      mutex.synchronize do
        hash[Thread.current] = Thread.current.hash
      end
    end
  end

  threads.map &:join
  threads.each { |t| hash.delete(t) }
  keys   = hash.keys
  values = hash.values

  raise "bad: #{keys} #{values} #{hash}" unless keys.empty?
end
@dbussink
Rubinius member

I've been running this for a while with latest master and can't seem to trigger any failures. Could you try if this still fails on master for you?

@dbussink
Rubinius member

Hmm, seems to have failed now after a long runtime. Not sure what is up though.

@chanks
chanks commented Feb 15, 2014

Yes, I've found that stopping and restarting it helps trigger it sometimes. It's twitchy, though. From what I can discern by stepping through the logic, Hash#[]= will occasionally find the hash of the thread to be 0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.