From f96cef56940c44846012ebbdb2f22e472b41fa30 Mon Sep 17 00:00:00 2001 From: Koichi Sasada Date: Wed, 9 Sep 2026 02:11:47 +0000 Subject: [PATCH] Give each Ractor its own WeakMap WeakRef held one process-wide `@@__map`, so creating a WeakRef in a non-main Ractor raised Ractor::IsolationError: require "weakref" Ractor.new { WeakRef.new("asdf") }.join #=> can not read non-shareable class variable @@__map from # non-main Ractors (WeakRef) (Ractor::IsolationError) The map cannot simply be made shareable: its values are the referenced objects, which are usually not shareable, and ObjectSpace::WeakMap is neither shareable nor thread safe. The map has to follow the Ractor that owns the referent, so each Ractor now gets its own. Each WeakRef keeps its Ractor's map in @map. That makes lookups a single ivar read instead of a Ractor-local storage lookup, and it makes the WeakRef itself non-shareable and non-movable, which is what we want: its entry only exists in the map of the Ractor that created it. Ractor.store_if_absent is Ruby 3.4 and later, so 3.0 to 3.3 use Ractor.current[] instead. Two threads of one Ractor can race there and each build a map, but that only leaves one empty map behind, since every WeakRef keeps the map it registered itself in. Rubies without Ractor keep the single map they have today. Fixes https://bugs.ruby-lang.org/issues/22105 Co-Authored-By: Claude Opus 5 (1M context) --- lib/weakref.rb | 39 +++++++++++++++++++++++++++++++++++---- test/test_weakref.rb | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 4 deletions(-) diff --git a/lib/weakref.rb b/lib/weakref.rb index c7274f9..65b0239 100644 --- a/lib/weakref.rb +++ b/lib/weakref.rb @@ -15,6 +15,9 @@ # GC.start # start the garbage collector # p foo.to_s # should raise exception (recycled) # +# A WeakRef belongs to the Ractor that created it: the referenced object is +# usually not shareable, so it may not be reached from another Ractor. +# Consequently a WeakRef is neither shareable nor movable. class WeakRef < Delegator # The version string @@ -27,7 +30,28 @@ class WeakRef < Delegator class RefError < StandardError end - @@__map = ::ObjectSpace::WeakMap.new + # ObjectSpace::WeakMap is neither shareable nor thread safe, and its values + # are the referenced objects, which are not shareable either. So each Ractor + # gets its own map instead of one map shared by the whole process. + if defined?(::Ractor) && ::Ractor.respond_to?(:store_if_absent) + def self.__map__ # :nodoc: + ::Ractor.store_if_absent(:__weakref_map__) { ::ObjectSpace::WeakMap.new } + end + elsif defined?(::Ractor) + # Ractor.store_if_absent is Ruby 3.4 and later. Two threads of the same + # Ractor can race here and each build a map, but that only leaves one empty + # map behind: every WeakRef keeps the map it registered itself in. + def self.__map__ # :nodoc: + current = ::Ractor.current + current[:__weakref_map__] ||= ::ObjectSpace::WeakMap.new + end + else + @__map = ::ObjectSpace::WeakMap.new + + def self.__map__ # :nodoc: + @__map + end + end ## # Creates a weak reference to +orig+ @@ -35,15 +59,22 @@ class RefError < StandardError def initialize(orig) case orig when true, false, nil + @map = nil @delegate_sd_obj = orig else - @@__map[self] = orig + # Holding the map in an instance variable keeps lookups to a single ivar + # read, and makes the WeakRef itself non-shareable and non-movable, which + # is what we want: its entry lives in this Ractor's map only. + # Delegator does not inherit from Object, so a bare constant here would go + # through Delegator.const_missing on every call. + @map = ::WeakRef.__map__ + @map[self] = orig end super end def __getobj__(&_block) # :nodoc: - @@__map[self] or defined?(@delegate_sd_obj) ? @delegate_sd_obj : + @map && @map[self] or defined?(@delegate_sd_obj) ? @delegate_sd_obj : Kernel::raise(RefError, "Invalid Reference - probably recycled", Kernel::caller(2)) end @@ -54,6 +85,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) + (@map ? @map.key?(self) : false) or defined?(@delegate_sd_obj) end end diff --git a/test/test_weakref.rb b/test/test_weakref.rb index f2308db..ebd9ebc 100644 --- a/test/test_weakref.rb +++ b/test/test_weakref.rb @@ -69,4 +69,41 @@ def test_repeated_object_memory_leak 150_000.times { WeakRef.new(a) } end; end + + if defined?(Ractor) + def test_weakref_in_ractor + bug22105 = '[ruby-core:125705]' + assert_separately(%w(-rweakref), <<-'end;', ignore_stderr: true) + Warning[:experimental] = false + ractor = Ractor.new do + str = "referenced" + ref = WeakRef.new(str) + [ref.__getobj__, ref.weakref_alive?] + end + # Ractor#value is 4.0 and later, Ractor#take before that. + obj, alive = ractor.respond_to?(:value) ? ractor.value : ractor.take + assert_equal("referenced", obj) + assert_equal(true, alive) + end; + end + + def test_map_is_per_ractor + assert_separately(%w(-rweakref), <<-'end;', ignore_stderr: true) + Warning[:experimental] = false + ractor = Ractor.new { WeakRef.__map__.object_id } + other = ractor.respond_to?(:value) ? ractor.value : ractor.take + assert_not_equal(WeakRef.__map__.object_id, other) + end; + end + + def test_weakref_is_not_shareable + # The referenced object belongs to the Ractor that created the WeakRef, + # so the WeakRef must not escape it. + assert_separately(%w(-rweakref), <<-'end;', ignore_stderr: true) + Warning[:experimental] = false + ref = WeakRef.new(Object.new) + assert_raise(Ractor::Error) { Ractor.make_shareable(ref) } + end; + end + end end