Skip to content

Commit 8b07057

Browse files
committed
Expand to shareable_proc and shareable_lambda
1 parent 9dacbfd commit 8b07057

7 files changed

Lines changed: 304 additions & 41 deletions

File tree

error.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -228,8 +228,8 @@ rb_warning_category_enabled_p(rb_warning_category_t category)
228228
* +:ractor_isolation+ ::
229229
* Ractor isolation violations reported by Ractor.check_isolation
230230
* (downgraded from Ractor::IsolationError exceptions to warnings), and
231-
* FrozenError warnings reported for objects passed to Ractor.make_shareable
232-
* while Ractor.warn_frozen_error is enabled.
231+
* FrozenError warnings reported for objects that would be frozen for Ractor
232+
* shareability while Ractor.warn_frozen_error is enabled.
233233
*/
234234

235235
static VALUE

include/ruby/internal/intern/error.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -202,16 +202,16 @@ RBIMPL_ATTR_NORETURN()
202202
void rb_error_frozen_object(VALUE what);
203203

204204
/**
205-
* True once Ractor.warn_frozen_error has marked any objects whose would-be
205+
* True once Ractor.warn_frozen_error has recorded any objects whose would-be
206206
* FrozenError should be reported as a warning instead.
207207
*
208208
* @internal
209209
*/
210210
RUBY_EXTERN bool ruby_ractor_warn_frozen_error_objects_enabled;
211211

212212
/**
213-
* Emits a warning and returns true if +obj+ was recorded by Ractor.make_shareable
214-
* while Ractor.warn_frozen_error was enabled.
213+
* Emits a warning and returns true if +obj+ was recorded for Ractor
214+
* shareability while Ractor.warn_frozen_error was enabled.
215215
*
216216
* @internal
217217
*/

ractor.c

Lines changed: 34 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1270,13 +1270,13 @@ rb_ractor_warn_frozen_error_warn(VALUE obj)
12701270
}
12711271

12721272
rb_category_warn(RB_WARN_CATEGORY_RACTOR_ISOLATION,
1273-
"would raise FrozenError: can't modify object passed to Ractor.make_shareable with Ractor.warn_frozen_error=true: %"PRIsVALUE,
1273+
"would raise FrozenError: can't modify object that would be frozen for Ractor shareability with Ractor.warn_frozen_error=true: %"PRIsVALUE,
12741274
rb_obj_class(obj));
12751275
return true;
12761276
}
12771277

1278-
static void
1279-
ractor_warn_frozen_error_mark(VALUE obj)
1278+
void
1279+
rb_ractor_warn_frozen_error_mark(VALUE obj)
12801280
{
12811281
if (RB_SPECIAL_CONST_P(obj)) {
12821282
return;
@@ -1708,19 +1708,22 @@ make_shareable_warn_check_shareable(VALUE obj, struct obj_traverse_data *data)
17081708
if (rb_ractor_shareable_p(obj)) {
17091709
return traverse_skip;
17101710
}
1711-
else if (!allow_frozen_shareable_p(obj)) {
1711+
else if (rb_ractor_warn_frozen_error_marked_p(obj)) {
1712+
return traverse_skip;
1713+
}
1714+
1715+
/* Mark on entry, not as a finalizer, so recursive warning-mode
1716+
* make_shareable calls that come from Proc self / outer-variable checks
1717+
* can detect cycles and stop instead of re-walking the same object graph
1718+
* until SystemStackError. */
1719+
rb_ractor_warn_frozen_error_mark(obj);
1720+
1721+
if (!allow_frozen_shareable_p(obj)) {
17121722
VM_ASSERT(RB_TYPE_P(obj, T_DATA));
17131723
const rb_data_type_t *type = RTYPEDDATA_TYPE(obj);
17141724

17151725
if (type->flags & RUBY_TYPED_FROZEN_SHAREABLE_NO_REC) {
1716-
if (obj_refer_only_shareables_p(obj)) {
1717-
ractor_warn_frozen_error_mark(obj);
1718-
return traverse_skip;
1719-
}
1720-
else {
1721-
rb_raise(rb_eRactorError,
1722-
"can not make shareable object for %+"PRIsVALUE" because it refers unshareable objects", obj);
1723-
}
1726+
return obj_refer_only_shareables_p(obj) ? traverse_skip : traverse_cont;
17241727
}
17251728
else if (rb_obj_is_proc(obj)) {
17261729
if (!rb_proc_ractor_make_shareable_continue(obj, Qundef, data->chain)) {
@@ -1735,7 +1738,7 @@ make_shareable_warn_check_shareable(VALUE obj, struct obj_traverse_data *data)
17351738
return traverse_cont;
17361739
}
17371740
else {
1738-
return traverse_stop;
1741+
return traverse_cont;
17391742
}
17401743
}
17411744

@@ -1754,7 +1757,7 @@ make_shareable_warn_check_shareable(VALUE obj, struct obj_traverse_data *data)
17541757
static enum obj_traverse_iterator_result
17551758
mark_warn_shareable(VALUE obj)
17561759
{
1757-
ractor_warn_frozen_error_mark(obj);
1760+
rb_ractor_warn_frozen_error_mark(obj);
17581761
return traverse_cont;
17591762
}
17601763

@@ -1781,13 +1784,20 @@ rb_ractor_make_shareable(VALUE obj)
17811784

17821785
if (rb_obj_traverse(obj, enter_func, null_leave, final_func, &chain, &exception)) {
17831786
if (!exception) {
1784-
exception = rb_exc_new3(rb_eRactorError, rb_sprintf("can not make shareable object for %+"PRIsVALUE, obj));
1787+
if (ruby_ractor_warn_frozen_error) {
1788+
exception = rb_exc_new3(rb_eRactorError,
1789+
rb_sprintf("can not make shareable object for an instance of %"PRIsVALUE,
1790+
rb_class_real(CLASS_OF(obj))));
1791+
}
1792+
else {
1793+
exception = rb_exc_new3(rb_eRactorError, rb_sprintf("can not make shareable object for %+"PRIsVALUE, obj));
1794+
}
17851795
}
17861796
// In Ractor.check_isolation mode downgrade to a :ractor_isolation
17871797
// warning (with the chain inlined) so the sweep can keep going.
17881798
// Outside that mode, attach the chain to @reference_chain and raise
17891799
// exactly as before.
1790-
if (rb_thread_ractor_isolation_check_p()) {
1800+
if (rb_thread_ractor_isolation_check_p() || ruby_ractor_warn_frozen_error) {
17911801
VALUE message = rb_obj_as_string(rb_funcall(exception, rb_intern("message"), 0));
17921802
if (!NIL_P(chain)) {
17931803
rb_str_append(message, chain);
@@ -2699,9 +2709,14 @@ static VALUE
26992709
ractor_shareable_proc(rb_execution_context_t *ec, VALUE replace_self, bool is_lambda)
27002710
{
27012711
if (!rb_ractor_shareable_p(replace_self)) {
2702-
// In check_isolation mode this only warns; fall through and try to
2703-
// make the proc shareable anyway so the sweep can keep going.
2704-
rb_ractor_isolation_violation("self should be shareable: %" PRIsVALUE, replace_self);
2712+
if (ruby_ractor_warn_frozen_error) {
2713+
rb_ractor_make_shareable(replace_self);
2714+
}
2715+
else {
2716+
// In check_isolation mode this only warns; fall through and try to
2717+
// make the proc shareable anyway so the sweep can keep going.
2718+
rb_ractor_isolation_violation("self should be shareable: %" PRIsVALUE, replace_self);
2719+
}
27052720
}
27062721
VALUE proc = is_lambda ? rb_block_lambda() : rb_block_proc();
27072722
return rb_proc_ractor_make_shareable(rb_proc_dup(proc), replace_self);

ractor.rb

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -735,6 +735,11 @@ def unmonitor port
735735
# Ractor.shareable_proc(self: self){}
736736
# #=> self should be shareable: main (Ractor::IsolationError)
737737
#
738+
# If +Ractor.warn_frozen_error+ is enabled, the returned Proc (and any
739+
# unshareable replacement +self+) is recorded for mutation warnings instead
740+
# of being frozen or made actually shareable. This mode is intended only for
741+
# development sweeps.
742+
#
738743
def self.shareable_proc self: nil
739744
Primitive.attr! :use_block
740745

@@ -748,6 +753,7 @@ def self.shareable_proc self: nil
748753
# Ractor.shareable_lambda(self: nil){} -> shareable lambda
749754
#
750755
# Same as Ractor.shareable_proc, but returns a lambda Proc.
756+
# +Ractor.warn_frozen_error+ has the same development-mode behavior here.
751757
#
752758
def self.shareable_lambda self: nil
753759
Primitive.attr! :use_block

ractor_core.h

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,15 +167,24 @@ st_table *rb_ractor_targeted_hooks(rb_ractor_t *cr);
167167
* gate site -- they just pay one extra LIKELY-false byte load. */
168168
RUBY_EXTERN bool ruby_ractor_isolation_check_enabled;
169169

170+
/* Global opt-in used by Ractor.make_shareable, Ractor.shareable_proc, and
171+
* Ractor.shareable_lambda to record would-be frozen objects and warn on later
172+
* mutation instead of freezing them. */
173+
RUBY_EXTERN bool ruby_ractor_warn_frozen_error;
174+
170175
/* True if the current thread has Ractor.check_isolation enabled.
171176
* Hot-path callers should NOT call this directly; go through
172177
* rb_ractor_isolation_check_active(). */
173178
bool rb_thread_ractor_isolation_check_p(void);
174179

175-
/* True if obj was made warning-shareable by Ractor.make_shareable while
176-
* Ractor.warn_frozen_error was enabled. */
180+
/* True if obj was recorded by Ractor.make_shareable, Ractor.shareable_proc, or
181+
* Ractor.shareable_lambda while Ractor.warn_frozen_error was enabled. */
177182
bool rb_ractor_warn_frozen_error_marked_p(VALUE obj);
178183

184+
/* Record obj so future mutation attempts emit :ractor_isolation warnings
185+
* instead of raising FrozenError. */
186+
void rb_ractor_warn_frozen_error_mark(VALUE obj);
187+
179188
/* Report a Ractor isolation violation:
180189
* - if Ractor.check_isolation is active on the current thread, emit a
181190
* :ractor_isolation category warning and return;

test/ruby/test_ractor.rb

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -637,6 +637,152 @@ def test_warn_frozen_error_warns_on_instance_variable_writes
637637
RUBY
638638
end
639639

640+
def test_warn_frozen_error_make_shareable_tolerates_unshareable_typed_data
641+
assert_ractor(<<~'RUBY')
642+
old = Ractor.warn_frozen_error
643+
begin
644+
Ractor.warn_frozen_error = true
645+
h = Hash.new(Mutex.new)
646+
assert_nothing_raised { Ractor.make_shareable(h) }
647+
assert_equal false, h.frozen?
648+
ensure
649+
Ractor.warn_frozen_error = old
650+
end
651+
RUBY
652+
end
653+
654+
def test_warn_frozen_error_make_shareable_handles_proc_cycles
655+
assert_ractor(<<~'RUBY')
656+
old = Ractor.warn_frozen_error
657+
begin
658+
Ractor.warn_frozen_error = true
659+
obj = Object.new
660+
pr = proc { obj }
661+
obj.instance_variable_set(:@proc, pr)
662+
663+
assert_nothing_raised { Ractor.make_shareable(obj) }
664+
assert_same obj, pr.call
665+
ensure
666+
Ractor.warn_frozen_error = old
667+
end
668+
RUBY
669+
end
670+
671+
def test_warn_frozen_error_marks_shareable_proc_without_freezing
672+
assert_ractor(<<~'RUBY')
673+
old = Ractor.warn_frozen_error
674+
begin
675+
Ractor.warn_frozen_error = true
676+
pr = Ractor.shareable_proc { :ok }
677+
678+
assert_equal false, Ractor.shareable?(pr)
679+
assert_equal false, pr.frozen?
680+
assert_equal :ok, pr.call
681+
682+
assert_warning(/would raise FrozenError.*Proc/) do
683+
pr.instance_variable_set(:@mutated, true)
684+
end
685+
assert_equal true, pr.instance_variable_get(:@mutated)
686+
ensure
687+
Ractor.warn_frozen_error = old
688+
end
689+
RUBY
690+
end
691+
692+
def test_warn_frozen_error_marks_shareable_lambda_without_freezing
693+
assert_ractor(<<~'RUBY')
694+
old = Ractor.warn_frozen_error
695+
begin
696+
Ractor.warn_frozen_error = true
697+
pr = Ractor.shareable_lambda { :ok }
698+
699+
assert_equal false, Ractor.shareable?(pr)
700+
assert_equal false, pr.frozen?
701+
assert_equal true, pr.lambda?
702+
assert_equal :ok, pr.call
703+
704+
assert_warning(/would raise FrozenError.*Proc/) do
705+
pr.instance_variable_set(:@mutated, true)
706+
end
707+
assert_equal true, pr.instance_variable_get(:@mutated)
708+
ensure
709+
Ractor.warn_frozen_error = old
710+
end
711+
RUBY
712+
end
713+
714+
def test_warn_frozen_error_preserves_shareable_proc_outer_variables
715+
assert_ractor(<<~'RUBY')
716+
old = Ractor.warn_frozen_error
717+
begin
718+
Ractor.warn_frozen_error = true
719+
ary = []
720+
pr = nil
721+
722+
assert_warning(/cannot make a shareable Proc because it can refer unshareable object.*variable 'ary'/) do
723+
pr = Ractor.shareable_proc { ary << :called; ary }
724+
end
725+
726+
assert_equal [:called], pr.call
727+
assert_equal false, ary.frozen?
728+
assert_warning(/would raise FrozenError.*Array/) do
729+
ary << :mutated
730+
end
731+
assert_equal [:called, :mutated], ary
732+
ensure
733+
Ractor.warn_frozen_error = old
734+
end
735+
RUBY
736+
end
737+
738+
def test_warn_frozen_error_warns_but_preserves_reassigned_outer_variables
739+
assert_ractor(<<~'RUBY')
740+
old = Ractor.warn_frozen_error
741+
begin
742+
Ractor.warn_frozen_error = true
743+
counter = 0
744+
pr = nil
745+
746+
assert_warning(/can not make a Proc shareable because it accesses outer variables \(counter\)/) do
747+
pr = Ractor.shareable_proc { counter += 1 }
748+
end
749+
750+
assert_equal 1, pr.call
751+
assert_equal 2, pr.call
752+
ensure
753+
Ractor.warn_frozen_error = old
754+
end
755+
RUBY
756+
end
757+
758+
def test_warn_frozen_error_marks_shareable_proc_self_without_freezing
759+
assert_ractor(<<~'RUBY')
760+
old = Ractor.warn_frozen_error
761+
begin
762+
Ractor.warn_frozen_error = true
763+
replacement_self = Object.new
764+
nested = []
765+
replacement_self.instance_variable_set(:@nested, nested)
766+
pr = Ractor.shareable_proc(self: replacement_self) { self }
767+
768+
assert_same replacement_self, pr.call
769+
assert_equal false, replacement_self.frozen?
770+
assert_equal false, nested.frozen?
771+
772+
assert_warning(/would raise FrozenError.*Object/) do
773+
replacement_self.instance_variable_set(:@mutated, true)
774+
end
775+
assert_equal true, replacement_self.instance_variable_get(:@mutated)
776+
assert_warning(/would raise FrozenError.*Array/) do
777+
nested << :mutated
778+
end
779+
assert_equal [:mutated], nested
780+
ensure
781+
Ractor.warn_frozen_error = old
782+
end
783+
RUBY
784+
end
785+
640786
def test_warn_frozen_error_off_uses_normal_make_shareable_freezing
641787
assert_ractor(<<~'RUBY')
642788
old = Ractor.warn_frozen_error
@@ -646,6 +792,10 @@ def test_warn_frozen_error_off_uses_normal_make_shareable_freezing
646792
Ractor.make_shareable(ary)
647793
assert_equal true, ary.frozen?
648794
assert_raise(FrozenError) { ary << :mutated }
795+
796+
pr = Ractor.shareable_proc { :ok }
797+
assert_equal true, pr.frozen?
798+
assert_raise(FrozenError) { pr.instance_variable_set(:@mutated, true) }
649799
ensure
650800
Ractor.warn_frozen_error = old
651801
end

0 commit comments

Comments
 (0)