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

Dereference proc from interface impl singleton #5820

Merged
merged 3 commits into from Aug 8, 2019

Conversation

@headius
Copy link
Member

commented Aug 8, 2019

By holding a reference to the proc, we anchor the proc's binding to the singleton class. When methods from the singleton class are cached elsewhere, such as for Java interface proxies (see GH-4968) we end up keeping the proc's binding alive longer than we'd like.

This patch detaches the proc object from the singleton, instead pointing it at the Proc class. The side effects from this include(at least) that any new singleton methods defined on that proc's singleton class will dispatch to hook methods (like singleton_method_defined) on Proc, rather than on the proc object. It's unknown whether this would affect any existing code.

Detach the proc from the singleton class.
By holding a reference to the proc, we anchor the proc's binding
to the singleton class. When methods from the singleton class are
cached elsewhere, such as for Java interface proxies (see GH-4968)
we end up keeping the proc's binding alive longer than we'd like.

This patch detaches the proc object from the singleton, instead
pointing it at the Proc class. The side effects from this include
(at least) that any new singleton methods defined on that proc's
singleton class will dispatch to hook methods (like
singleton_method_defined) on Proc, rather than on the proc object.
It's unknown whether this would affect any existing code.

@headius headius changed the title Detach the proc from the singleton class. Dereference proc from interface impl singleton Aug 8, 2019

headius added some commits Aug 8, 2019

Only detach the proc for "natural" Proc instances.
This avoids detaching the object for user-provided singleton procs
where they may actually want the hard reference and hook callbacks
to work as before.

See GH-4968.
Test for pre-singletonized procs gaining interfaces.
This is a test that the fix for GH-4968 does not break existing
singleton procs' "attached" relationship. The self in the
singleton_method_added hook should still be the proc, rather than
a reattached value as in the fix.
@headius

This comment has been minimized.

Copy link
Member Author

commented Aug 8, 2019

Approved by @enebo and @kares.

@headius headius added this to the JRuby 9.2.8.0 milestone Aug 8, 2019

@headius headius merged commit 642a1e2 into jruby:master Aug 8, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
jruby.jruby Build #20190808.4 succeeded
Details

@headius headius deleted the headius:detach_interface_proc branch Aug 8, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.