Skip to content

Commit 243b326

Browse files
committed
Run user #freeze side effects under Ractor.warn_frozen_error
Real Ractor.make_shareable freezes objects, which runs any user-defined #freeze. Some classes rely on that to migrate mutable state into Ractor-local storage on freeze (e.g. ActiveSupport::CachingKeyGenerator, which moves its cache into Ractor[] and nils the ivar in #freeze). Ractor.warn_frozen_error mode deliberately does not freeze objects so a program can keep running and report every would-be FrozenError instead of crashing. But because it never froze, it also never ran those #freeze methods, so such objects never set themselves up and emitted spurious "would raise FrozenError" warnings even though they are Ractor-safe. Resolve the chicken-and-egg by invoking #freeze for its side effects while intercepting the actual freeze so the object is only marked, never frozen: - make_shareable_warn_check_shareable now invokes a non-default #freeze on T_OBJECTs before marking/descending, so the method's own setup writes are not reported and any state it moves out of the object graph is neither walked nor marked. An in-flight guard breaks recursion when #freeze calls Ractor.make_shareable(self). - rb_obj_freeze records the object for mutation warnings instead of setting FL_FREEZE while ruby_ractor_warn_freeze_as_mark is set (scoped to the traversal), so super inside a user #freeze marks instead of freezes. Genuine unsafe mutations still warn; the object stays unfrozen. Assisted-By: devx/e2120fdf-5247-4bd8-86e5-49513fcb7cda
1 parent 4c610f7 commit 243b326

4 files changed

Lines changed: 176 additions & 0 deletions

File tree

include/ruby/internal/intern/error.h

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,16 @@ void rb_error_frozen_object(VALUE what);
209209
*/
210210
RUBY_EXTERN bool ruby_ractor_warn_frozen_error_objects_enabled;
211211

212+
/**
213+
* True while Ractor.make_shareable (under Ractor.warn_frozen_error) is running
214+
* an object's user-defined #freeze for its side effects. While set, rb_obj_freeze
215+
* records the object for mutation warnings instead of actually freezing it, so
216+
* the program can keep running and reporting warnings end to end.
217+
*
218+
* @internal
219+
*/
220+
RUBY_EXTERN bool ruby_ractor_warn_freeze_as_mark;
221+
212222
/**
213223
* Emits a warning and returns true if +obj+ was recorded for Ractor
214224
* shareability while Ractor.warn_frozen_error was enabled.
@@ -217,6 +227,14 @@ RUBY_EXTERN bool ruby_ractor_warn_frozen_error_objects_enabled;
217227
*/
218228
bool rb_ractor_warn_frozen_error_warn(VALUE obj);
219229

230+
/**
231+
* Records +obj+ so that subsequent mutations are reported as would-be
232+
* FrozenError warnings under Ractor.warn_frozen_error, without freezing it.
233+
*
234+
* @internal
235+
*/
236+
void rb_ractor_warn_frozen_error_mark(VALUE obj);
237+
220238
/**
221239
* Queries if the passed object is frozen.
222240
*

object.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1375,6 +1375,14 @@ rb_obj_dummy1(VALUE _x, VALUE _y)
13751375
VALUE
13761376
rb_obj_freeze(VALUE obj)
13771377
{
1378+
if (RB_UNLIKELY(ruby_ractor_warn_freeze_as_mark) && !SPECIAL_CONST_P(obj)) {
1379+
/* Ractor.warn_frozen_error mode is invoking this object's #freeze for
1380+
* its Ractor setup side effects. We must not actually freeze it, so a
1381+
* full request can keep running and reporting warnings; just record it
1382+
* so later mutations are reported as would-be FrozenError. */
1383+
rb_ractor_warn_frozen_error_mark(obj);
1384+
return obj;
1385+
}
13781386
if (!OBJ_FROZEN(obj)) {
13791387
OBJ_FREEZE(obj);
13801388
if (SPECIAL_CONST_P(obj)) {

ractor.c

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include "internal/struct.h"
1919
#include "internal/st.h"
2020
#include "internal/thread.h"
21+
#include "internal/vm.h"
2122
#include "variable.h"
2223
#include "yjit.h"
2324
#include "zjit.h"
@@ -1290,6 +1291,37 @@ rb_ractor_warn_frozen_error_mark(VALUE obj)
12901291
ruby_ractor_warn_frozen_error_objects_enabled = true;
12911292
}
12921293

1294+
/* Set while we invoke a user-defined #freeze during warn-mode make_shareable.
1295+
* rb_obj_freeze() checks this and records the object for mutation warnings
1296+
* instead of actually freezing it (see object.c). */
1297+
bool ruby_ractor_warn_freeze_as_mark = false;
1298+
1299+
/* Objects whose #freeze is currently being invoked by warn-mode make_shareable.
1300+
* Used to break recursion when an object's #freeze calls Ractor.make_shareable
1301+
* on itself (in real mode the object would already be frozen/shareable and be
1302+
* skipped; in warn mode it never freezes, so we need an explicit guard). */
1303+
static VALUE ractor_warn_freeze_inflight = Qnil;
1304+
1305+
static VALUE
1306+
ractor_warn_freeze_inflight_hash(void)
1307+
{
1308+
if (NIL_P(ractor_warn_freeze_inflight)) {
1309+
ractor_warn_freeze_inflight = rb_ident_hash_new();
1310+
rb_obj_hide(ractor_warn_freeze_inflight);
1311+
rb_gc_register_mark_object(ractor_warn_freeze_inflight);
1312+
}
1313+
return ractor_warn_freeze_inflight;
1314+
}
1315+
1316+
static bool
1317+
ractor_warn_freeze_inflight_p(VALUE obj)
1318+
{
1319+
if (NIL_P(ractor_warn_freeze_inflight)) {
1320+
return false;
1321+
}
1322+
return RTEST(rb_hash_lookup2(ractor_warn_freeze_inflight, obj, Qfalse));
1323+
}
1324+
12931325
/// traverse function
12941326

12951327
// 2: stop search
@@ -1711,6 +1743,34 @@ make_shareable_warn_check_shareable(VALUE obj, struct obj_traverse_data *data)
17111743
else if (rb_ractor_warn_frozen_error_marked_p(obj)) {
17121744
return traverse_skip;
17131745
}
1746+
else if (ractor_warn_freeze_inflight_p(obj)) {
1747+
/* We are already inside this object's #freeze (reached again via a
1748+
* re-entrant Ractor.make_shareable). Skip to break the recursion; the
1749+
* outer traversal will finish walking its children. */
1750+
return traverse_skip;
1751+
}
1752+
1753+
/* Real Ractor.make_shareable freezes objects, which runs any user-defined
1754+
* #freeze and lets classes migrate mutable state into Ractor-local storage
1755+
* (e.g. ActiveSupport::CachingKeyGenerator). Warn mode must not freeze, so
1756+
* we invoke #freeze here for its side effects while rb_obj_freeze is
1757+
* intercepted (ruby_ractor_warn_freeze_as_mark) to only record the object.
1758+
* Running #freeze *before* marking obj means #freeze's own setup writes are
1759+
* not reported, and running it *before* descending means state it moves out
1760+
* of the object graph is neither walked nor marked -- so it stops the
1761+
* spurious warnings without ever freezing anything. */
1762+
if (BUILTIN_TYPE(obj) == T_OBJECT &&
1763+
!RB_OBJ_FROZEN_RAW(obj) &&
1764+
!rb_method_basic_definition_p(CLASS_OF(obj), idFreeze)) {
1765+
struct rescue_freeze_data rescue_freeze_data = { 0 };
1766+
bool prev = ruby_ractor_warn_freeze_as_mark;
1767+
1768+
rb_hash_aset(ractor_warn_freeze_inflight_hash(), obj, Qtrue);
1769+
ruby_ractor_warn_freeze_as_mark = true;
1770+
rb_rescue(try_freeze, obj, rescue_freeze, (VALUE)&rescue_freeze_data);
1771+
ruby_ractor_warn_freeze_as_mark = prev;
1772+
rb_hash_delete(ractor_warn_freeze_inflight_hash(), obj);
1773+
}
17141774

17151775
/* Mark on entry, not as a finalizer, so recursive warning-mode
17161776
* make_shareable calls that come from Proc self / outer-variable checks

test/ruby/test_ractor.rb

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -783,6 +783,96 @@ def test_warn_frozen_error_marks_shareable_proc_self_without_freezing
783783
RUBY
784784
end
785785

786+
def test_warn_frozen_error_runs_custom_freeze_side_effects_without_freezing
787+
# Real Ractor.make_shareable freezes objects, which runs any user-defined
788+
# #freeze. Some classes rely on that to migrate mutable state into
789+
# Ractor-local storage on freeze (e.g. ActiveSupport::CachingKeyGenerator).
790+
# Warn mode must not freeze, but it must still run #freeze for its side
791+
# effects, otherwise those objects never set themselves up and emit
792+
# spurious FrozenError warnings even though they are Ractor-safe.
793+
assert_ractor(<<~'RUBY')
794+
old = Ractor.warn_frozen_error
795+
begin
796+
Ractor.warn_frozen_error = true
797+
798+
klass = Class.new do
799+
def initialize
800+
@cache = {}
801+
@ractor_key = nil
802+
end
803+
804+
def freeze
805+
@ractor_key = "_klass_cache_#{object_id}".to_sym
806+
Ractor[@ractor_key] = @cache
807+
@cache = nil
808+
super
809+
end
810+
811+
def store(k, v)
812+
cache[k] = v
813+
end
814+
815+
private
816+
817+
def cache
818+
@cache || (Ractor[@ractor_key] ||= {})
819+
end
820+
end
821+
822+
obj = klass.new
823+
Ractor.make_shareable(obj)
824+
825+
# #freeze ran (state migrated) but the object was not actually frozen.
826+
assert_equal false, obj.frozen?
827+
assert_nil obj.instance_variable_get(:@cache)
828+
refute_nil obj.instance_variable_get(:@ractor_key)
829+
830+
# The migrated-away cache is not part of the object graph any more, so
831+
# it was never marked: writing to it must NOT warn.
832+
assert_warning("") do
833+
obj.store(:a, 1)
834+
end
835+
ensure
836+
Ractor.warn_frozen_error = old
837+
end
838+
RUBY
839+
end
840+
841+
def test_warn_frozen_error_custom_freeze_calling_make_shareable_on_self
842+
# A #freeze that re-enters Ractor.make_shareable(self) must not recurse
843+
# forever: in real mode the object would already be frozen/shareable and be
844+
# skipped, but in warn mode it never freezes, so make_shareable relies on an
845+
# explicit in-flight guard to break the recursion.
846+
assert_ractor(<<~'RUBY')
847+
old = Ractor.warn_frozen_error
848+
begin
849+
Ractor.warn_frozen_error = true
850+
851+
klass = Class.new do
852+
attr_reader :froze
853+
def freeze
854+
@froze = true
855+
Ractor.make_shareable(self)
856+
super
857+
end
858+
end
859+
860+
obj = klass.new
861+
assert_nothing_raised do
862+
Ractor.make_shareable(obj)
863+
end
864+
assert_equal true, obj.froze
865+
assert_equal false, obj.frozen?
866+
867+
assert_warning(/would raise FrozenError/) do
868+
obj.instance_variable_set(:@mutated, true)
869+
end
870+
ensure
871+
Ractor.warn_frozen_error = old
872+
end
873+
RUBY
874+
end
875+
786876
def test_warn_frozen_error_off_uses_normal_make_shareable_freezing
787877
assert_ractor(<<~'RUBY')
788878
old = Ractor.warn_frozen_error

0 commit comments

Comments
 (0)