Skip to content

Allow creating WeakRefs outside the main Ractor #88

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

Open
rkh wants to merge 1 commit into
base: master
Choose a base branch
from
+23 −9
Open

Allow creating WeakRefs outside the main Ractor #88

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions lib/weakref.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,24 +27,24 @@ class WeakRef < Delegator
class RefError < StandardError
end

@@__map = ::ObjectSpace::WeakMap.new

##
# Creates a weak reference to +orig+

def initialize(orig)
case orig
when true, false, nil
@delegate_sd_obj = orig
when true, false, nil, Symbol, Integer, Float, Complex, Rational
@weak = false
@store = orig
else
@@__map[self] = orig
@weak = true
@store = ::ObjectSpace::WeakMap.new

@byroot byroot Jul 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This very significantly increase memory usage, because now every WeakRef has its own WeakMap instance (208B for one entry).

@rkh rkh Jul 28, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could keep one WeakRef per Ractor, as #2 does, but only if there was a mechanism to also move/copy the referenced object if necessary. This would be possible to implement but would significantly increase the complexity.

Alternatively, we could follow #2, but would need to somehow prevent WeakRefs from being moved.

@rkh rkh Jul 29, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, what I would really want is a ractor-shareable, mutable WeakMap, but I guess the constraints around non-shareable values could not be resolved.

Or alternatively, a way to mutate an object before and after it has been sent over a port. That way it could hold on to the referenced object for transit.

Both of these are obviously out of scope for this PR.

@byroot byroot Jul 29, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's unfortunate is that since a couple versions, thanks to @peterzhu2118 we have actual weak references in the GC, and WeakRef could be implemented without needing a WeakMap (I think Peter had a prototype).

But since it's all private API and weakref is a gem, it can't use those facilities.

My preferred solution (but not the easiest) would be to request weakref to be made core, so that it can become a real weakref.

It was extracted as a gem because that was the direction to turn the stdlib into gems for easier maintenance, but given it's barely 40 lines of codes, I'd think this is a bit of an overshoot.

@rkh rkh Jul 29, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I agree, that would be a proper solution.

@store[self] = orig
end
super
end

def __getobj__(&_block) # :nodoc:
@@__map[self] or defined?(@delegate_sd_obj) ? @delegate_sd_obj :
Kernel::raise(RefError, "Invalid Reference - probably recycled", Kernel::caller(2))
return @store unless @weak
@store[self] or ::Kernel::raise(RefError, "Invalid Reference - probably recycled", ::Kernel::caller(2))
end

def __setobj__(obj) # :nodoc:
Expand All @@ -54,6 +54,6 @@ def __setobj__(obj) # :nodoc:
# Returns true if the referenced object is still alive.

def weakref_alive?
@@__map.key?(self) or defined?(@delegate_sd_obj)
!@weak || @store.key?(self)
end
end
14 changes: 14 additions & 0 deletions test/test_weakref.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,4 +69,18 @@ def test_repeated_object_memory_leak
150_000.times { WeakRef.new(a) }
end;
end

if defined?(Ractor)
def test_weakref_ractor_creatable
warning = Warning[:experimental]
Warning[:experimental] = false
bug22105 = '[ruby-core:125705]'
assert_nothing_raised(Ractor::RemoteError, bug22105) do
ractor = Ractor.new { WeakRef.new(Object.new) }
Ractor.select(ractor)
end
ensure
Warning[:experimental] = warning
end
end
end