Endless loop test in JRuby 1.8 mode

[headius]

Unsure if this is a JRuby issue or a thread_safe issue, but this test appears to loop forever, producing a block warning at the same time:

test/test_cache_loops.rb:82

  def test_add_remove_to_zero_via_merge_pair
    add_remove_to_zero_via_merge_pair
...

I started digging into it but it was too involved for me to figure out tonight.

[headius]

Link to travis output, which isn't very helpful: https://api.travis-ci.org/jobs/11999071/log.txt?deansi=true

[thedarkone]

Yeah, I know... test_loops.rb is some ugly unpenetrable code 😔.

This is what the compiled method looks like:

def _add_remove_to_zero_via_merge_pair_loop_inner_single_key(cache, key, i, length, acc)
  target = i + length
  while i < target
    acc += (cache.merge_pair(key, key) {}) ? -1 : 1 # <- this line is giving the warning
    i += 1
  end
  acc
end

I can repro when running the whole suite, but everything runs fine if test_add_remove_to_zero_via_merge_pair is run by itself, so this does look to be a JRuby bug (also no block warnings are produced this way). I've also run into some JRuby block compilation issues before: https://jira.codehaus.org/browse/JRUBY-7024.

[thedarkone]

So, it wasn't test_add_remove_to_zero_via_merge_pair, but rather test_add_remove_via_compute.

This is enough to repro:
ruby --1.8 -Ilib:test test/test_cache_loops.rb --name=test_add_remove_via_compute

The compiled method then looks like this:

def _add_remove_via_compute_loop_inner_single_key(cache, key, i, length, acc)
  do_add = rand(2) == 1
  target = i + length
  while i < target
    cache.compute(key) do |old_value| # <- this is emits the "multiple values for a block parameter (0 for 1)" warning
      if do_add
        acc += 1 unless old_value
        key
      else
        acc -= 1 if old_value
        nil
      end
    end

    i += 1
  end
  acc
end

[thedarkone]

@headius that wasn't an infinite loop actually, it just spewd too much warnings 😜, but would finish given enough time 😇.

This fixes the warning, now you have to decide if this is a JRuby or thread_safe bug: thedarkone@c4f0266.

[headius]

I was wondering about that...I just got tired of waiting :-D

I think I'd qualify this as a thread_safe bug; in general we never want to see null passed for IRubyObject since it can progress very far away form the original call site before we notice it. The fact that some methods (like yieldSpecific) might take null is a smell in JRuby that we eventually need to fix, but passing null is a bad idea in general.

Go ahead and merge your fix.

[headius]

BTW, I added you and @sferik as collaborators.