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.

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

Jitted blocks do not scope evals properly #3368

headius opened this issue Oct 3, 2015 · 1 comment

Jitted blocks do not scope evals properly #3368

headius opened this issue Oct 3, 2015 · 1 comment


Copy link

headius commented Oct 3, 2015

This is the cause of the current TestMarshal failures.

Reduced case follows.


class X
  def initialize
    p :here
  end { eval 'def initialize; p :there; end' }

Command line and bug (:there should be :here):

$ jruby -Xjit.threshold=0 -e "load './blah.rb';"
2015-10-03T02:18:02.890-05:00: Ruby: done compiling target script: -e
2015-10-03T02:18:02.910-05:00: JITCompiler: done jitting:<block> at /Users/headius/projects/jruby/blah.rb:4
2015-10-03T02:18:02.916-05:00: JITCompiler: done jitting:X.X.initialize at (eval):0

The problem here seems to be that when a block is jitted, it's not framing itself or rooting the eval properly. I have not investigated further than that, but obviously the eval'ed def initialize is going into the surrounding class X rather than into the that launched the block.

This breaks TestMarshal because of test_marshal.rb:263-266.

  classISO8859_1 = do
    attr_accessor "r\xe9sum\xe9".force_encoding(iso_8859_1)
    eval("def initialize(x); @r\xe9sum\xe9 = x; end".force_encoding(iso_8859_1))

This initialize becomes TestMarshal's initialize, As a result, the name of the test passed into (as part of test suite initialization) never reaches the TestCase superclass, and the __name__ remains nil. That's the source of the errors we see in the test run:

[1/1] TestMarshal#test_symbol2 = 0.17 s
  1) Error:
TypeError: no implicit conversion of nil into String
    /Users/headius/projects/jruby/test/mri/lib/test/unit/testcase.rb:17:in `run'
    org/jruby/ `map'
    /Users/headius/projects/jruby/test/mri/lib/test/unit.rb:682:in `block in _run_suites'
    org/jruby/ `each'
    /Users/headius/projects/jruby/test/mri/lib/test/unit.rb:680:in `_run_suites'
    org/jruby/ `each'
    /Users/headius/projects/jruby/test/mri/lib/test/unit.rb:31:in `run'
    /Users/headius/projects/jruby/test/mri/lib/test/unit.rb:799:in `run'
    /Users/headius/projects/jruby/test/mri/lib/test/unit.rb:859:in `run'
    /Users/headius/projects/jruby/test/mri/lib/test/unit.rb:863:in `run'
    test/mri/runner.rb:41:in `<top>'

Note that the method name in the error report is missing.

cc @enebo

@headius headius closed this as completed in e9d5c94 Oct 5, 2015
Copy link
Member Author

headius commented Oct 5, 2015

Closing the loop here...

The problem was that the evalType threadlocal in *IRBlockBody was getting out of sync, due to having MixedModeIRBlockBody wrap a CompiledIRBlockBody. As a result, when CompiledIRBlockBody ran, it did not see that e.g. had set MixedModeIRBlockBody as an eval'ed block, and as a result the search for method target failed to stop at the block.

The fix in e9d5c94 shares a single evalType threadlocal between both the container MixedModeIRBlockBody and the contained CompiledIRBlockBody.

There's remaining work needed to clean up all this shared state and do a better job of unifying the different container objects for interpreted and jitted blocks and methods.

@headius headius added this to the JRuby milestone Oct 5, 2015
@headius headius self-assigned this Oct 5, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet

No branches or pull requests

1 participant