Skip to content

Commit 783dde3

Browse files
json: reject non-shareable default sort_keys proc from non-main Ractors
JSON::State.default_sort_keys_proc is registered at extension load time so it's owned by the main Ractor. When we implement the registration table on each Ractor storing another Ractor's unshareable Proc into that slot leaves a dangling reference when the owning Ractor runs a local GC. This commit ensures that we won't use a shareable proc by raising a Ractor::IsolationError.
1 parent 099340c commit 783dde3

3 files changed

Lines changed: 59 additions & 0 deletions

File tree

‎lib/json/common.rb‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,24 @@ def parser=(parser) # :nodoc:
8080
def generator=(generator) # :nodoc:
8181
old, $VERBOSE = $VERBOSE, nil
8282

83+
unless generator::State.respond_to?(:default_sort_keys_proc_unchecked=, true)
84+
generator::State.singleton_class.class_eval do
85+
alias_method :default_sort_keys_proc_unchecked=, :default_sort_keys_proc=
86+
private :default_sort_keys_proc_unchecked=
87+
88+
def default_sort_keys_proc=(proc)
89+
unless ::Proc === proc
90+
raise ::TypeError, "sort_key_proc must be a Proc"
91+
end
92+
if defined?(::Ractor) && !::Ractor.shareable?(proc) && !::Ractor.current.equal?(::Ractor.main)
93+
raise ::Ractor::IsolationError,
94+
"can not set a non-shareable Proc as the default sort_keys proc from a non-main Ractor"
95+
end
96+
self.default_sort_keys_proc_unchecked = proc
97+
end
98+
end
99+
end
100+
83101
# The default proc used when the +sort_keys+ generation option is +true+.
84102
# It returns a new hash with the entries sorted by their keys.
85103
sort_keys_proc = ->(hash) { hash.sort.to_h }

‎test/json/json_generator_test.rb‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,18 @@ def test_generate_sort_keys_with_proc
248248
assert_instance_of Proc, state.sort_keys
249249
end
250250

251+
def test_default_sort_keys_proc_setter_survives_generator_reassignment
252+
omit "fork not supported" unless Process.respond_to?(:fork)
253+
pid = fork do
254+
JSON.generator = JSON::Ext::Generator
255+
JSON.generator = JSON::Ext::Generator
256+
JSON::State.default_sort_keys_proc = ->(hash) { hash.sort.to_h }
257+
exit!(JSON.generate({b: 1, a: 2}, sort_keys: true) == '{"a":2,"b":1}' ? 0 : 1)
258+
end
259+
_, status = Process.wait2(pid)
260+
assert_predicate status, :success?
261+
end
262+
251263
def test_generate_custom
252264
state = State.new(space_before: " ", space: " ", indent: "<i>", object_nl: "\n", array_nl: "<a_nl>")
253265
json = generate({1=>{2=>3,4=>[5,6]}}, state)

‎test/json/ractor_test.rb‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,4 +123,33 @@ def test_coder_proc
123123
_, status = Process.waitpid2(pid)
124124
assert_predicate status, :success?
125125
end if Ractor.respond_to?(:shareable_proc)
126+
127+
def test_default_sort_keys_proc_ractor_safety
128+
pid = fork do
129+
Warning[:experimental] = false
130+
results = Ractor.new do
131+
outcomes = []
132+
133+
begin
134+
JSON::State.default_sort_keys_proc = ->(hash) { hash.sort.to_h }
135+
outcomes << :accepted
136+
rescue Ractor::IsolationError
137+
outcomes << :rejected
138+
end
139+
140+
# A shareable Proc is allowed from any Ractor.
141+
JSON::State.default_sort_keys_proc = Ractor.shareable_lambda { |hash| hash.sort.reverse.to_h }
142+
outcomes << JSON.generate({b: 1, a: 2}, sort_keys: true)
143+
outcomes
144+
end.value
145+
146+
# The main Ractor owns the slot, so it may install any Proc.
147+
JSON::State.default_sort_keys_proc = ->(hash) { hash.sort.to_h }
148+
main_result = JSON.generate({b: 1, a: 2}, sort_keys: true)
149+
150+
exit(results == [:rejected, '{"b":1,"a":2}'] && main_result == '{"a":2,"b":1}' ? 0 : 1)
151+
end
152+
_, status = Process.waitpid2(pid)
153+
assert_predicate status, :success?
154+
end if Ractor.respond_to?(:shareable_lambda)
126155
end if defined?(Ractor) && Process.respond_to?(:fork)

0 commit comments

Comments
 (0)