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

rb_ary_new_from_values leads to an escaped internal exception #2734

Closed
nirvdrum opened this issue Sep 22, 2022 · 4 comments
Closed

rb_ary_new_from_values leads to an escaped internal exception #2734

nirvdrum opened this issue Sep 22, 2022 · 4 comments
Assignees
Labels

Comments

@nirvdrum
Copy link
Collaborator

When using TruffleRuby at 76cfbe8, I run into a problem with an internal exception escaping. The problem does not exist with cff928c, indicating the issue appeared with the merge of PR truffleruby/3195. I haven't bisected individual commits.

I'm running into the problem with the google-protobuf gem.

git clone https://github.com/protocolbuffers/protobuf.git
cd protobuf/ruby
git checkout bcd175578f4647a5fa81eecee1d3ecc320e72517
jt ruby -S bundle install
jt ruby -S bundle exec rake test

Running the tests, you'll end up with:

truffleruby: an internal exception escaped out of the interpreter,
please report it to https://github.com/oracle/truffleruby/issues.

dead handle 0xbad000000010050 (com.oracle.truffle.api.CompilerDirectives.ShouldNotReachHere)
	from com.oracle.truffle.api.CompilerDirectives.shouldNotReachHere(CompilerDirectives.java:574)
	from com.oracle.truffle.api.CompilerDirectives.shouldNotReachHere(CompilerDirectives.java:520)
	from org.truffleruby.cext.UnwrapNode$UnwrapNativeNode.raiseError(UnwrapNode.java:98)
	from org.truffleruby.cext.UnwrapNode$UnwrapNativeNode.unwrapTaggedObject(UnwrapNode.java:83)
	from org.truffleruby.cext.UnwrapNodeGen$UnwrapNativeNodeGen.executeAndSpecialize(UnwrapNodeGen.java:378)
	from org.truffleruby.cext.UnwrapNodeGen$UnwrapNativeNodeGen.execute(UnwrapNodeGen.java:334)
	from org.truffleruby.cext.UnwrapNode.longToWrapper(UnwrapNode.java:272)
	from org.truffleruby.cext.UnwrapNodeGen.executeAndSpecialize(UnwrapNodeGen.java:130)
	from org.truffleruby.cext.UnwrapNodeGen.execute(UnwrapNodeGen.java:87)
	from org.truffleruby.cext.UnwrapNode$UnwrapCArrayNode.unwrapCArray(UnwrapNode.java:227)
	from org.truffleruby.cext.UnwrapNodeGen$UnwrapCArrayNodeGen.executeAndSpecialize(UnwrapNodeGen.java:987)
	from org.truffleruby.cext.UnwrapNodeGen$UnwrapCArrayNodeGen.execute(UnwrapNodeGen.java:885)
	from org.truffleruby.cext.CExtNodes$RbAryNewFromValues.rbAryNewFromValues(CExtNodes.java:1872)
	from org.truffleruby.cext.CExtNodesFactory$RbAryNewFromValuesFactory$RbAryNewFromValuesNodeGen.execute(CExtNodesFactory.java:9108)
	from org.truffleruby.language.RubyCoreMethodRootNode.execute(RubyCoreMethodRootNode.java:48)
array.c:52:in `rb_ary_new_from_values'
	from array.c:52:in `rb_ary_new_from_values'
	from class.c:85:in `rb_class_new_instance'
	from /Users/nirvdrum/dev/workspaces/gems-to-fix/protobuf/ruby/ext/google/protobuf_c/defs.c:1175:in `DescriptorPool_add_serialized_file'
	from /Users/nirvdrum/dev/workspaces/truffleruby-ws/graal/sdk/mxbuild/darwin-aarch64/GRAALVM_A33290E019_JAVA11/graalvm-a33290e019-java11-22.3.0-dev/Contents/Home/languages/ruby/lib/truffle/truffle/cext_ruby.rb:41:in `add_serialized_file'
	from /Users/nirvdrum/dev/workspaces/gems-to-fix/protobuf/ruby/lib/google/protobuf/descriptor_pb.rb:7:in `<top (required)>'
	from <internal:core> core/kernel.rb:234:in `gem_original_require'
	from /Users/nirvdrum/dev/workspaces/gems-to-fix/protobuf/ruby/lib/google/protobuf/descriptor_dsl.rb:9:in `<top (required)>'
	from <internal:core> core/kernel.rb:234:in `gem_original_require'
	from /Users/nirvdrum/dev/workspaces/gems-to-fix/protobuf/ruby/lib/google/protobuf.rb:56:in `<top (required)>'
	from <internal:core> core/kernel.rb:234:in `gem_original_require'
	from /Users/nirvdrum/dev/workspaces/gems-to-fix/protobuf/ruby/tests/basic_test_pb.rb:4:in `<top (required)>'
	from <internal:core> core/kernel.rb:234:in `gem_original_require'
	from /Users/nirvdrum/dev/workspaces/gems-to-fix/protobuf/ruby/tests/basic.rb:6:in `<top (required)>'
	from <internal:core> core/kernel.rb:234:in `gem_original_require'
	from /Users/nirvdrum/dev/workspaces/truffleruby-ws/graal/sdk/mxbuild/darwin-aarch64/GRAALVM_A33290E019_JAVA11/graalvm-a33290e019-java11-22.3.0-dev/Contents/Home/languages/ruby/lib/gems/gems/rake-13.0.6/lib/rake/rake_test_loader.rb:21:in `block in <main>'
	from /Users/nirvdrum/dev/workspaces/truffleruby-ws/graal/sdk/mxbuild/darwin-aarch64/GRAALVM_A33290E019_JAVA11/graalvm-a33290e019-java11-22.3.0-dev/Contents/Home/languages/ruby/lib/gems/gems/rake-13.0.6/lib/rake/rake_test_loader.rb:6:in `select'
	from /Users/nirvdrum/dev/workspaces/truffleruby-ws/graal/sdk/mxbuild/darwin-aarch64/GRAALVM_A33290E019_JAVA11/graalvm-a33290e019-java11-22.3.0-dev/Contents/Home/languages/ruby/lib/gems/gems/rake-13.0.6/lib/rake/rake_test_loader.rb:6:in `<main>'
@eregon eregon added the cexts label Sep 26, 2022
@eregon eregon self-assigned this Sep 26, 2022
@eregon
Copy link
Member

eregon commented Sep 26, 2022

This is a similar issue to #2720, specifically rb_gc_register_address() is called before the assignment of the corresponding variable, in the backtrace above it's from https://github.com/protocolbuffers/protobuf/blob/f1a939fb69504ff4e1fe386978165faa37fb2979/ruby/ext/google/protobuf_c/protobuf.c#L384-L386.
Moving the rb_gc_register_address(), the tests complete (with 243 tests, 367572 assertions, 9 failures, 5 errors, 0 pendings, 0 omissions, 0 notifications 94.2387% passed).

Given the 16 rb_gc_register_address() in google-protobuf and the fact this is the 2nd time we see this error I think we need an automated way to handle it and not just do PRs upstream, i.e., address #2721.

@nirvdrum
Copy link
Collaborator Author

Thanks for the explanation. I had checked that previous versions of TruffleRuby didn't blow up. What changed on our end?

@eregon
Copy link
Member

eregon commented Sep 30, 2022

76cfbe8 is what changed (see the new .md file for details), handles are weaker now and so avoids leaking but then also reveal such issues which already existed but accidentally worked due to stronger/leaking handles.

@eregon
Copy link
Member

eregon commented Oct 31, 2022

Fixed by 3aa058d

@eregon eregon closed this as completed Oct 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants