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

Dump outer variables tables when dumping an iseq to binary #4942

Merged
merged 1 commit into from
Oct 7, 2021
Merged

Dump outer variables tables when dumping an iseq to binary #4942

merged 1 commit into from
Oct 7, 2021

Conversation

tenderlove
Copy link
Member

This commit dumps the outer variables table when dumping an iseq to
binary. This fixes a case where Ractors aren't able to tell what outer
variables belong to a lambda after the lambda is loaded via ISeq.load_from_binary

[Bug #18232] [ruby-core:105504]

XrXr
XrXr reviewed Oct 6, 2021
View reviewed changes
compile.c Outdated Show resolved Hide resolved
@XrXr
Copy link
Member

XrXr commented Oct 6, 2021

Another minor thing, I think you want to bump IBF_DEVEL_VERSION because it's a change to the binary format.

@tenderlove
Copy link
Member Author

Another minor thing, I think you want to bump IBF_DEVEL_VERSION because it's a change to the binary format.

Ya, I mentioned in the RedMine ticket, but I wasn't actually sure what needed to change. I'll fix it now. 🙇

@tenderlove
Dump outer variables tables when dumping an iseq to binary
15c9460 
This commit dumps the outer variables table when dumping an iseq to
binary.  This fixes a case where Ractors aren't able to tell what outer
variables belong to a lambda after the lambda is loaded via ISeq.load_from_binary

[Bug #18232] [ruby-core:105504]
@tenderlove tenderlove merged commit 217df51 into ruby:master Oct 7, 2021
@tenderlove tenderlove deleted the dump-outer-variables branch October 7, 2021 22:39
@yahonda
Copy link

yahonda commented Oct 10, 2021

Since this commit has been merged to the master branch, Rails Active Job integration with async adapter has been failing.

https://buildkite.com/rails/rails/builds/81621#8a717e31-68fb-4014-b6ea-f0311db37182/1076-1097

  • Steps to reproduce
git clone https://github.com/rails/rails
cd rails/activejob
bundle exec rake test:integration:async
  • Actual result
$ ruby -v ; bundle exec rake test:integration:async
ruby 3.1.0dev (2021-10-07T22:39:47Z master 217df51f0e) [x86_64-linux]
/home/yahonda/.rbenv/versions/trunk/bin/ruby -w -I"lib:test" /home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/gems/rake-13.0.3/lib/rake/rake_test_loader.rb "test/integration/queuing_test.rb"
Using async
/home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/gems/zeitwerk-2.5.0.beta5/lib/zeitwerk/kernel.rb:24: warning: method redefined; discarding old require
/home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/gems/bootsnap-1.7.5/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:27: warning: previous definition of require was here
/home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/gems/mail-2.7.1/lib/mail/parsers/address_lists_parser.rb:32454: warning: statement not reached
/home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/gems/mail-2.7.1/lib/mail/parsers/address_lists_parser.rb:32615: warning: statement not reached
/home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/gems/mail-2.7.1/lib/mail/parsers/address_lists_parser.rb:32651: warning: statement not reached
/home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/gems/mail-2.7.1/lib/mail/parsers/address_lists_parser.rb:32740: warning: statement not reached
/home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/gems/mail-2.7.1/lib/mail/parsers/address_lists_parser.rb:32756: warning: statement not reached
/home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/gems/mail-2.7.1/lib/mail/parsers/address_lists_parser.rb:32822: warning: statement not reached
/home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/gems/mail-2.7.1/lib/mail/parsers/address_lists_parser.rb:32863: warning: statement not reached
/home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/gems/mail-2.7.1/lib/mail/parsers/address_lists_parser.rb:32888: warning: statement not reached
/home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/gems/mail-2.7.1/lib/mail/parsers/address_lists_parser.rb:31984: warning: assigned but unused variable - testEof
/home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/gems/httpclient-2.8.3/lib/httpclient/ssl_config.rb:370: warning: assigned but unused variable - pathlen
/home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/gems/httpclient-2.8.3/lib/httpclient/ssl_config.rb:51: warning: method redefined; discarding old initialize
/home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/gems/httpclient-2.8.3/lib/httpclient/ssl_config.rb:58: warning: method redefined; discarding old add_cert
/home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/gems/httpclient-2.8.3/lib/httpclient/ssl_config.rb:58: warning: method redefined; discarding old add_file
/home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/gems/httpclient-2.8.3/lib/httpclient/ssl_config.rb:58: warning: method redefined; discarding old add_path
/home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/gems/cookiejar-0.3.3/lib/cookiejar/cookie.rb:183: warning: mismatched indentations at 'end' with 'else' at 181
/home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/gems/cookiejar-0.3.3/lib/cookiejar/jar.rb:225: warning: mismatched indentations at 'end' with 'else' at 223
/home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/gems/bootsnap-1.7.5/lib/bootsnap/compile_cache/iseq.rb:13:in `to_binary': wrong argument type false (expected Symbol) (TypeError)
	from /home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/gems/bootsnap-1.7.5/lib/bootsnap/compile_cache/iseq.rb:13:in `input_to_storage'
	from /home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/gems/bootsnap-1.7.5/lib/bootsnap/compile_cache/iseq.rb:30:in `fetch'
	from /home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/gems/bootsnap-1.7.5/lib/bootsnap/compile_cache/iseq.rb:30:in `fetch'
	from /home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/gems/bootsnap-1.7.5/lib/bootsnap/compile_cache/iseq.rb:55:in `load_iseq'
	from /home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/gems/bootsnap-1.7.5/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:23:in `require'
	from /home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/gems/bootsnap-1.7.5/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:23:in `block in require_with_bootsnap_lfi'
	from /home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/gems/bootsnap-1.7.5/lib/bootsnap/load_path_cache/loaded_features_index.rb:92:in `register'
	from /home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/gems/bootsnap-1.7.5/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:22:in `require_with_bootsnap_lfi'
	from /home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/gems/bootsnap-1.7.5/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:31:in `require'
	from /home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/gems/zeitwerk-2.5.0.beta5/lib/zeitwerk/kernel.rb:35:in `require'
	from /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/base.rb:315:in `<class:Base>'
	from /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/base.rb:282:in `<module:ActiveRecord>'
	from /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/base.rb:15:in `<main>'
	from /home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/gems/bootsnap-1.7.5/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:23:in `require'
	from /home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/gems/bootsnap-1.7.5/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:23:in `block in require_with_bootsnap_lfi'
	from /home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/gems/bootsnap-1.7.5/lib/bootsnap/load_path_cache/loaded_features_index.rb:92:in `register'
	from /home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/gems/bootsnap-1.7.5/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:22:in `require_with_bootsnap_lfi'
	from /home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/gems/bootsnap-1.7.5/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:31:in `require'
	from /home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/gems/zeitwerk-2.5.0.beta5/lib/zeitwerk/kernel.rb:35:in `require'
	from /home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/tasks/database_tasks.rb:279:in `migrate'
	from /home/yahonda/src/github.com/rails/rails/activejob/test/support/integration/helper.rb:19:in `<top (required)>'
	from /home/yahonda/src/github.com/rails/rails/activejob/test/helper.rb:12:in `require'
	from /home/yahonda/src/github.com/rails/rails/activejob/test/helper.rb:12:in `<top (required)>'
	from /home/yahonda/src/github.com/rails/rails/activejob/test/integration/queuing_test.rb:3:in `require'
	from /home/yahonda/src/github.com/rails/rails/activejob/test/integration/queuing_test.rb:3:in `<top (required)>'
	from /home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/gems/rake-13.0.3/lib/rake/rake_test_loader.rb:17:in `require'
	from /home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/gems/rake-13.0.3/lib/rake/rake_test_loader.rb:17:in `block in <main>'
	from /home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/gems/rake-13.0.3/lib/rake/rake_test_loader.rb:5:in `select'
	from /home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/gems/rake-13.0.3/lib/rake/rake_test_loader.rb:5:in `<main>'
rake aborted!
Command failed with status (1): [ruby -w -I"lib:test" /home/yahonda/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/gems/rake-13.0.3/lib/rake/rake_test_loader.rb "test/integration/queuing_test.rb" ]
/home/yahonda/.rbenv/versions/trunk/bin/bundle:23:in `load'
/home/yahonda/.rbenv/versions/trunk/bin/bundle:23:in `<main>'
Tasks: TOP => test:integration:async
(See full trace by running task with --trace)
$

@tenderlove
Copy link
Member Author

@yahonda thanks! I'll take a look

@tenderlove
Copy link
Member Author

@yahonda I fixed it. It's really really weird, but the source of the actual bug is that anonymous parameters have no name. I filed a redmine issue here: https://bugs.ruby-lang.org/issues/18250

Also a PR here

@yahonda
Copy link

yahonda commented Oct 14, 2021

Thanks for the fix. Let me test it again once #4961 is merged to Ruby master branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@XrXr XrXr XrXr left review comments

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
3 participants
@tenderlove @XrXr @yahonda