Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Hash#any? not always returning proper keys/values #5398

Closed
enebo opened this issue Oct 31, 2018 · 8 comments
Closed

Hash#any? not always returning proper keys/values #5398

enebo opened this issue Oct 31, 2018 · 8 comments
Labels
regression
Milestone
JRuby 9.2.1.0

Comments

@enebo
Copy link
Member

enebo commented Oct 31, 2018

Dumping progress on regression in activesupport test.

checkout rails v5.1.6 and cd into activesupport.

run:

JRUBY_OPTS=--dev TEST=test/share_lock_test.rb TEST_OPTS=--name=test_exclusive_ordering jrake test

Somewhere around new hashing algorithm this test seems to fail. Possible bisects it failed at are (none of these commits compiled properly):

There are only 'skip'ped commits left to test.
The first bad commit could be any of:
70dac53bc8746e162bd2e1c657a21bdf019827fe
dc11e38691bc688949f7d1e81dbc4a6c5d711ac5
7ec8091050cc460567a14f433749cf325db76409
a1ed645ef00e0be0f3522b8f259a7bbab369dd5f
69d19c47d83c8c4dfae7e0734cf3f2202174b226
db6d81a683929c059ef4c0375236630047b5125b
ac560230e16d945222f7deb507b6b4bcacfddce7
We cannot bisect more!
@enebo enebo added this to the JRuby 9.2.1.0 milestone Oct 31, 2018
@headius
Copy link
Member

headius commented Nov 1, 2018

cc @ChrisBr Hashing hasn't really changed, has it?

@headius
Copy link
Member

headius commented Nov 1, 2018

I confirmed the logic for hashing a thread does not appear to have changed...or at least RubyThread#hashCode has not changed since 2017.

@headius
Copy link
Member

headius commented Nov 1, 2018

Could be a problem with Hash#any? not passing the args through properly?

Here's the errors that result from running what @enebo described:

# Running:

warning: thread "Ruby-0-Thread-9: /Users/headius/projects/rails/activesupport/test/share_lock_test.rb:194" terminated with exception (report_on_exception is true):
NoMethodError: undefined method `include?' for nil:NilClass
Did you mean?  include_class
        busy_for_sharing? at /Users/headius/projects/rails/activesupport/lib/active_support/concurrency/share_lock.rb:210
                     any? at org/jruby/RubyHash.java:2344
        busy_for_sharing? at /Users/headius/projects/rails/activesupport/lib/active_support/concurrency/share_lock.rb:210
      busy_for_exclusive? at /Users/headius/projects/rails/activesupport/lib/active_support/concurrency/share_lock.rb:204
          start_exclusive at /Users/headius/projects/rails/activesupport/lib/active_support/concurrency/share_lock.rb:81
                 wait_for at /Users/headius/projects/rails/activesupport/lib/active_support/concurrency/share_lock.rb:219
               wait_while at /Users/headius/projects/jruby/lib/ruby/stdlib/monitor.rb:121
                 wait_for at /Users/headius/projects/rails/activesupport/lib/active_support/concurrency/share_lock.rb:219
          start_exclusive at /Users/headius/projects/rails/activesupport/lib/active_support/concurrency/share_lock.rb:81
             yield_shares at /Users/headius/projects/rails/activesupport/lib/active_support/concurrency/share_lock.rb:185
          start_exclusive at /Users/headius/projects/rails/activesupport/lib/active_support/concurrency/share_lock.rb:80
          mon_synchronize at /Users/headius/projects/jruby/lib/ruby/stdlib/monitor.rb:226
          start_exclusive at /Users/headius/projects/rails/activesupport/lib/active_support/concurrency/share_lock.rb:75
                exclusive at /Users/headius/projects/rails/activesupport/lib/active_support/concurrency/share_lock.rb:147
  test_exclusive_ordering at /Users/headius/projects/rails/activesupport/test/share_lock_test.rb:197
                  sharing at /Users/headius/projects/rails/activesupport/lib/active_support/concurrency/share_lock.rb:160
  test_exclusive_ordering at /Users/headius/projects/rails/activesupport/test/share_lock_test.rb:195
org.jruby.exceptions.NoMethodError: (NoMethodError) undefined method `include?' for nil:NilClass
Did you mean?  include_class
	at RUBY.busy_for_sharing?(/Users/headius/projects/rails/activesupport/lib/active_support/concurrency/share_lock.rb:210)
	at org.jruby.RubyHash.any?(org/jruby/RubyHash.java:2344)
	at RUBY.busy_for_sharing?(/Users/headius/projects/rails/activesupport/lib/active_support/concurrency/share_lock.rb:210)
	at RUBY.busy_for_exclusive?(/Users/headius/projects/rails/activesupport/lib/active_support/concurrency/share_lock.rb:204)
	at RUBY.start_exclusive(/Users/headius/projects/rails/activesupport/lib/active_support/concurrency/share_lock.rb:81)
	at RUBY.wait_for(/Users/headius/projects/rails/activesupport/lib/active_support/concurrency/share_lock.rb:219)
	at RUBY.wait_while(/Users/headius/projects/jruby/lib/ruby/stdlib/monitor.rb:121)
	at RUBY.wait_for(/Users/headius/projects/rails/activesupport/lib/active_support/concurrency/share_lock.rb:219)
	at RUBY.start_exclusive(/Users/headius/projects/rails/activesupport/lib/active_support/concurrency/share_lock.rb:81)
	at RUBY.yield_shares(/Users/headius/projects/rails/activesupport/lib/active_support/concurrency/share_lock.rb:185)
	at RUBY.start_exclusive(/Users/headius/projects/rails/activesupport/lib/active_support/concurrency/share_lock.rb:80)
	at RUBY.mon_synchronize(/Users/headius/projects/jruby/lib/ruby/stdlib/monitor.rb:226)
	at RUBY.start_exclusive(/Users/headius/projects/rails/activesupport/lib/active_support/concurrency/share_lock.rb:75)
	at RUBY.exclusive(/Users/headius/projects/rails/activesupport/lib/active_support/concurrency/share_lock.rb:147)
	at RUBY.test_exclusive_ordering(/Users/headius/projects/rails/activesupport/test/share_lock_test.rb:197)
	at RUBY.sharing(/Users/headius/projects/rails/activesupport/lib/active_support/concurrency/share_lock.rb:160)
	at RUBY.test_exclusive_ordering(/Users/headius/projects/rails/activesupport/test/share_lock_test.rb:195)
E

Error:
ShareLockTest#test_exclusive_ordering:
NoMethodError: undefined method `include?' for nil:NilClass
Did you mean?  include_class
    /Users/headius/projects/rails/activesupport/lib/active_support/concurrency/share_lock.rb:210:in `block in busy_for_sharing?'
    org/jruby/RubyHash.java:2344:in `any?'
    /Users/headius/projects/rails/activesupport/lib/active_support/concurrency/share_lock.rb:210:in `busy_for_sharing?'
    /Users/headius/projects/rails/activesupport/lib/active_support/concurrency/share_lock.rb:204:in `busy_for_exclusive?'
    /Users/headius/projects/rails/activesupport/lib/active_support/concurrency/share_lock.rb:81:in `block in start_exclusive'
    /Users/headius/projects/rails/activesupport/lib/active_support/concurrency/share_lock.rb:219:in `block in wait_for'
    /Users/headius/projects/jruby/lib/ruby/stdlib/monitor.rb:121:in `wait_while'
    /Users/headius/projects/rails/activesupport/lib/active_support/concurrency/share_lock.rb:219:in `wait_for'
    /Users/headius/projects/rails/activesupport/lib/active_support/concurrency/share_lock.rb:81:in `block in start_exclusive'
    /Users/headius/projects/rails/activesupport/lib/active_support/concurrency/share_lock.rb:185:in `yield_shares'
    /Users/headius/projects/rails/activesupport/lib/active_support/concurrency/share_lock.rb:80:in `block in start_exclusive'
    /Users/headius/projects/jruby/lib/ruby/stdlib/monitor.rb:226:in `mon_synchronize'
    /Users/headius/projects/rails/activesupport/lib/active_support/concurrency/share_lock.rb:75:in `start_exclusive'
    /Users/headius/projects/rails/activesupport/lib/active_support/concurrency/share_lock.rb:147:in `exclusive'
    /Users/headius/projects/rails/activesupport/test/share_lock_test.rb:197:in `block in test_exclusive_ordering'
    /Users/headius/projects/rails/activesupport/lib/active_support/concurrency/share_lock.rb:160:in `sharing'
    /Users/headius/projects/rails/activesupport/test/share_lock_test.rb:195:in `block in test_exclusive_ordering'


 Users/headius/projects/rails/activesupport/test/share_lock_test.rb:182

The line in question, in share_lock.rb, is the @waiting.any? line here:

        def busy_for_sharing?(purpose)
          (@exclusive_thread && @exclusive_thread != Thread.current) ||
            @waiting.any? { |t, (_, c)| t != Thread.current && !c.include?(purpose) }
        end

So there's a possibility the variables are not being destructured right, or the data stored in the hash is not structured right.

@ChrisBr
Copy link
Contributor

ChrisBr commented Nov 1, 2018

Hmm, no idea! I will have a look at it tonight or over the weekend...

@enebo
Copy link
Member Author

enebo commented Nov 1, 2018

I do not think this is destructuring issue per se but that the @waiting values end up wrong...when it runs correctly I think eventually we end up with key/values like: #Thread:0x63b86a9d / [:unload, [:unload, :load]] but when we crash we end up with #Thread:0x686d396a / : [:load, [:load]]. I am guessing some virtue of the test eventually removes all these single element nested array? I have not actually tried to understand the point of this test yet so perhaps I should now :)

I should add my glance ot hashCode being called and I see nothing obviously wrong but that this seems to break right from the point of the new hashing stuff forward commit-wise.

@enebo
Copy link
Member Author

enebo commented Nov 1, 2018

On second thought I realize @waiting is a hash so the first 't' is the key. So there is something weirder going on than I think...

@enebo
Copy link
Member Author

enebo commented Nov 1, 2018

Move structuring out of any block params to print out k and value for any? and I see nil, nil as the key and value. If I print out the hash immediately before any? I can see a hash with at least an entry which appears to look fine.

Another weird item is it appears to not happen 100% of the runs. I half feel like somehow our hashing changes because the thread state changes and it cannot find the key so we end up with nil,nil????

@enebo
Copy link
Member Author

enebo commented Nov 1, 2018

I believe I figured this out....I notice inspect() uses our standard visitor and it has explicit logic when walking entries to skip pairs which contain null. I am guessing once we delete hash elements we gain holes in the hash? Once I add a continue for this in any_p_* the test passes.

@enebo enebo changed the title Thread as hash key is broken???? Hash#any? not always returning proper keys/values Nov 1, 2018
@enebo enebo closed this as completed in 19b6e2a Nov 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
regression
Projects
None yet
Milestone
JRuby 9.2.1.0
Development

No branches or pull requests
3 participants
@headius @enebo @ChrisBr