jruby / jruby

JRuby, an implementation of Ruby on the JVM
https://www.jruby.org
Other
3.79k stars 922 forks source link

Pry-remote does not work with jruby-9.4.8.0 #8323

Open gacha opened 2 months ago

gacha commented 2 months ago

A Pry remote session fails using jruby-9.4.8.0 but works with jruby-9.3.15.0 and older versions. Tested different Java versions, the same result.

Environment Information

No JAVA_OPTS and no JRUBY_OPTS.

jruby 9.4.8.0 (3.1.4) 2024-07-02 4d41e55a67 OpenJDK 64-Bit Server VM 11.0.23+9-LTS on 11.0.23+9-LTS +jit [arm64-darwin]

Darwin m3.local 23.4.0 Darwin Kernel Version 23.4.0: Fri Mar 15 00:12:37 PDT 2024; root:xnu-10063.101.17~1/RELEASE_ARM64_T6031 arm64

The code

require 'bundler/inline'

gemfile do
  source 'https://rubygems.org'
  gem 'pry-remote'
end

a = 1
binding.remote_pry

Expected Behavior

Pry-remote connects to the process and we see a prompt.

Actual Behavior

We got an error

warning: thread "Ruby-0-Thread-1: /Users/username/.rbenv/versions/jruby-9.4.8.0/lib/ruby/stdlib/drb/drb.rb:1555" terminated with exception (report_on_exception is true):
RangeError: 0x0000000000000fa4 is not id value
         _id2ref at org/jruby/RubyObjectSpace.java:151
          to_obj at /Users/username/.rbenv/versions/jruby-9.4.8.0/lib/ruby/stdlib/drb/drb.rb:366
          to_obj at /Users/username/.rbenv/versions/jruby-9.4.8.0/lib/ruby/stdlib/drb/drb.rb:1528
          to_obj at /Users/username/.rbenv/versions/jruby-9.4.8.0/lib/ruby/stdlib/drb/drb.rb:1847
           _load at /Users/username/.rbenv/versions/jruby-9.4.8.0/lib/ruby/stdlib/drb/drb.rb:1054
            load at org/jruby/RubyMarshal.java:151
            load at /Users/username/.rbenv/versions/jruby-9.4.8.0/lib/ruby/stdlib/drb/drb.rb:597
     synchronize at org/jruby/ext/thread/Mutex.java:171
            load at /Users/username/.rbenv/versions/jruby-9.4.8.0/lib/ruby/stdlib/drb/drb.rb:595
      recv_reply at /Users/username/.rbenv/versions/jruby-9.4.8.0/lib/ruby/stdlib/drb/drb.rb:640
      recv_reply at /Users/username/.rbenv/versions/jruby-9.4.8.0/lib/ruby/stdlib/drb/drb.rb:941
    send_message at /Users/username/.rbenv/versions/jruby-9.4.8.0/lib/ruby/stdlib/drb/drb.rb:1324
  method_missing at /Users/username/.rbenv/versions/jruby-9.4.8.0/lib/ruby/stdlib/drb/drb.rb:1143
            open at /Users/username/.rbenv/versions/jruby-9.4.8.0/lib/ruby/stdlib/drb/drb.rb:1302
  method_missing at /Users/username/.rbenv/versions/jruby-9.4.8.0/lib/ruby/stdlib/drb/drb.rb:1142
     with_friend at /Users/username/.rbenv/versions/jruby-9.4.8.0/lib/ruby/stdlib/drb/drb.rb:1161
  method_missing at /Users/username/.rbenv/versions/jruby-9.4.8.0/lib/ruby/stdlib/drb/drb.rb:1141
             run at /Users/username/.rbenv/versions/jruby-9.4.8.0/lib/ruby/gems/shared/gems/pry-remote-0.1.8/lib/pry-remote.rb:289
          <main> at /Users/username/.rbenv/versions/jruby-9.4.8.0/lib/ruby/gems/shared/gems/pry-remote-0.1.8/bin/pry-remote:4
            load at org/jruby/RubyKernel.java:1220
          <main> at /Users/username/.rbenv/versions/jruby-9.4.8.0/bin/pry-remote:25
enebo commented 2 months ago

I wonder if this is a change to how pry-remote or drb works? This will work if you set JRUBY_OPTS=-X+O for both processes as it appears to use _id2ref and that has never worked unless -X+O is enabled (classes/modules actually will work since we do register those even without -X+O).

gacha commented 2 months ago

The pry-remote has not changed last 7 years. Thanks, for the tip, it works if I use JRUBY_OPTS=-X+O for both processes. But it worked with previous versions without this option. Is it safe to add this at least in the development, are there some downsides?

enebo commented 2 months ago

@gacha yeah this has to involve how drb is implemented. If you look at https://bugs.ruby-lang.org/issues/15711 we have been trying to deal with this method for a while. I wonder if we used to somehow patch drb somehow?

@headius any thoughts here? Did we deal with referencing objects and patch drb and those patches disappeared?

Oh and to answer your question -X+O has a significant effect on performance as we have to maintain a map to ever ruby object in the runtime. It should be fine for development but you would want to measure perf to see how much it impacts things.

headius commented 2 months ago

@enebo It looks like the DRb changes I proposed were only partially implemented, and id2ref is still in use. We can support it with ObjectSpace enabled, but ideally they would use one of the newer mechanisms for weak referencing objects.

We did have a patch in our repository that may never have gotten into DRb proper. I'll see if I can resurrect that.

headius commented 2 months ago

I have pushed https://github.com/ruby/drb/pull/31 to switch the default ID mapping to the WeakIdConv that does not use _id2ref. Both @eregon and I will advocate for getting this merged and released, and once that happens you can update to that version of the drb gem (or to a newer JRuby that includes it) and this will work without full ObjectSpace support.

gacha commented 2 months ago

Thank you for your work!