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

NoMethodError for ObjectSpace::WeakMap#values #6267

Closed
yahonda opened this issue Jun 4, 2020 · 7 comments · Fixed by #6683
Closed

NoMethodError for ObjectSpace::WeakMap#values #6267

yahonda opened this issue Jun 4, 2020 · 7 comments · Fixed by #6683
Assignees
Milestone

Comments

@yahonda
Copy link

yahonda commented Jun 4, 2020

Environment Information

  • JRuby version (jruby -v) and command line (flags, JRUBY_OPTS, etc)
% jruby -v
jruby 9.2.11.1 (2.5.7) 2020-03-25 b1f55b1a40 OpenJDK 64-Bit Server VM 11.0.2+9 on 11.0.2+9 +jit [darwin-x86_64]
  • Operating system and platform (e.g. uname -a)
% uname -a
Darwin mymacbookpro 19.6.0 Darwin Kernel Version 19.6.0: Sun May 17 22:15:23 PDT 2020; root:xnu-6153.140.21~10/RELEASE_X86_64 x86_64

Expected Behavior

  • Steps to reproduce
weakmap = ObjectSpace::WeakMap.new
weakmap.values

weakmap.values should return empty Array as CRuby does.

  • Result with CRuby 2.7.1
% ruby -v
ruby 2.7.1p83 (2020-03-31 revision a0c7c23c9c) [x86_64-darwin19]
% irb
irb(main):001:0> weakmap = ObjectSpace::WeakMap.new
irb(main):002:0> weakmap.values
=> []
irb(main):003:0>

Actual Behavior

It raises NoMethodError

% ruby -v
jruby 9.2.11.1 (2.5.7) 2020-03-25 b1f55b1a40 OpenJDK 64-Bit Server VM 11.0.2+9 on 11.0.2+9 +jit [darwin-x86_64]
% irb
WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by com.headius.backport9.modules.Modules to method sun.nio.ch.NativeThread.signal(long)
WARNING: Please consider reporting this to the maintainers of com.headius.backport9.modules.Modules
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release
irb(main):001:0> weakmap = ObjectSpace::WeakMap.new
=> #<ObjectSpace::WeakMap:0x189b5fb1>
irb(main):002:0> weakmap.values
Traceback (most recent call last):
        6: from /Users/yahonda/.rbenv/versions/jruby-9.2.11.1/bin/irb:13:in `<main>'
        5: from org/jruby/RubyKernel.java:1189:in `catch'
        4: from org/jruby/RubyKernel.java:1189:in `catch'
        3: from org/jruby/RubyKernel.java:1442:in `loop'
        2: from org/jruby/RubyKernel.java:1048:in `eval'
        1: from (irb):2:in `evaluate'
NoMethodError (undefined method `values' for #<ObjectSpace::WeakMap:0x189b5fb1>)
irb(main):003:0>
@yahonda
Copy link
Author

yahonda commented Jun 4, 2020

This is required to support rsim/oracle-enhanced#2027

@yahonda yahonda changed the title NoMethodError for ObjectSpace::WeakMap#new NoMethodError for ObjectSpace::WeakMap#values Jun 4, 2020
@yahonda
Copy link
Author

yahonda commented Jun 8, 2020

Let me update that his issue needs to be addressed to support rails/rails#39121 .

@headius
Copy link
Member

headius commented Jun 12, 2020

Should be easy enough to support.

@headius headius added this to the JRuby 9.3.0.0 milestone Jun 12, 2020
@headius headius self-assigned this Jun 12, 2020
@headius
Copy link
Member

headius commented Jun 12, 2020

Ok, so I don't have a problem with adding the missing features from Weakmap, but officially it is not a public API. It has never been discussed (naming, behavior, etc) and I believe that needs to happen before it sees usage in third-party libraries. Unfortunately that would mean this should not be used until after Ruby 2.8/3.0, though JRuby would probably implement everything that CRuby provides in our 9.3 release.

https://bugs.ruby-lang.org/issues/16959

I'd like to get some feedback on this before we proceed to implement the rest of Weakmap just because CRuby does it that way.

@headius
Copy link
Member

headius commented Jun 12, 2020

FWIW I don't fault you for using the feature... it's sorely needed in Ruby, as I've pointed out in my ReferenceQueue issue (linked from the Ruby issue above), and now that there's specs it's hard to tell that it's not a public API.

@yahonda
Copy link
Author

yahonda commented Jun 12, 2020

Thanks for the update. I with ObjectSpace::WeakMap#values will be implemented for JRuby.

@headius
Copy link
Member

headius commented Jun 13, 2020

FWIW this can be implemented in Ruby using some JRuby internal APIs (non-public, but useful for cases like this):

require 'jruby'

class org::jruby::RubyObjectSpace::WeakMap
  field_reader :map
end

class ObjectSpace::WeakMap
  def values
    JRuby.ref(self).map.values.reject(&:nil?)
  end
end

# Example usage follows...

wm = ObjectSpace::WeakMap.new

x = nil

# This is inside a block to avoid references leaking to the main scope
tap {
  wm["foo"] = Object.new
  wm["bar"] = x = Object.new

  p wm.values # [#<Object:0x62e136d3>, #<Object:0x51b7e5df>]
}

5.times { JRuby.gc }

p wm.values # [#<Object:0x62e136d3>]

The reject is necessary because the data structure in map does not omit vacated references (I assume the CRuby implementation does this filtering interally too).

yahonda added a commit to yahonda/oracle-enhanced that referenced this issue Jun 16, 2020
yahonda added a commit to yahonda/oracle-enhanced that referenced this issue Jun 16, 2020
Refer jruby/jruby#6267 (comment)

Co-authored-by: Charles Oliver Nutter <headius@headius.com>
yahonda added a commit to yahonda/oracle-enhanced that referenced this issue Jul 2, 2020
Refer jruby/jruby#6267 (comment)

Co-authored-by: Charles Oliver Nutter <headius@headius.com>
headius added a commit to headius/jruby that referenced this issue May 24, 2021
JesseChavez added a commit to JesseChavez/activerecord-jdbc-adapter that referenced this issue Jul 21, 2024
JesseChavez added a commit to JesseChavez/activerecord-jdbc-adapter that referenced this issue Jul 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants