Skip to content

Commit aefa671

Browse files
committed
Fix concurrent usage of JSON::Coder#dump
Fix: rails/rails@9061627#r168784389 Because the `depth` counter is inside `JSON::State` it can't be used concurrently, and in case of a circular reference the counter may be left at the max value. The depth counter should be moved outside `JSON_Generator_State` and into `struct generate_json_data`, but it's a larger refactor. In the meantime, `JSON::Coder` calls `State#generate_new` so I changed that method so that it first copy the state on the stack.
1 parent 9e6067b commit aefa671

File tree

5 files changed

+73
-9
lines changed

5 files changed

+73
-9
lines changed

CHANGES.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22

33
### Unreleased
44

5+
* Fix `JSON::Coder` to have one dedicated depth counter per invocation.
6+
After encountering a circular reference in `JSON::Coder#dump`, any further `#dump` call would raise `JSON::NestingError`.
7+
58
### 2025-10-07 (2.15.1)
69

710
* Fix incorrect escaping in the JRuby extension when encoding shared strings.

ext/json/ext/generator/generator.c

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1124,7 +1124,7 @@ static inline long increase_depth(struct generate_json_data *data)
11241124
JSON_Generator_State *state = data->state;
11251125
long depth = ++state->depth;
11261126
if (RB_UNLIKELY(depth > state->max_nesting && state->max_nesting)) {
1127-
rb_raise(eNestingError, "nesting of %ld is too deep", --state->depth);
1127+
rb_raise(eNestingError, "nesting of %ld is too deep. Did you try to serialize objects with circular references?", --state->depth);
11281128
}
11291129
return depth;
11301130
}
@@ -1491,10 +1491,39 @@ static VALUE cState_generate(int argc, VALUE *argv, VALUE self)
14911491
rb_check_arity(argc, 1, 2);
14921492
VALUE obj = argv[0];
14931493
VALUE io = argc > 1 ? argv[1] : Qnil;
1494-
VALUE result = cState_partial_generate(self, obj, generate_json, io);
1494+
return cState_partial_generate(self, obj, generate_json, io);
1495+
}
1496+
1497+
static VALUE cState_generate_new(int argc, VALUE *argv, VALUE self)
1498+
{
1499+
rb_check_arity(argc, 1, 2);
1500+
VALUE obj = argv[0];
1501+
VALUE io = argc > 1 ? argv[1] : Qnil;
1502+
14951503
GET_STATE(self);
1496-
(void)state;
1497-
return result;
1504+
1505+
JSON_Generator_State new_state;
1506+
MEMCPY(&new_state, state, JSON_Generator_State, 1);
1507+
1508+
// FIXME: depth shouldn't be part of JSON_Generator_State, as that prevents it from being used concurrently.
1509+
new_state.depth = 0;
1510+
1511+
char stack_buffer[FBUFFER_STACK_SIZE];
1512+
FBuffer buffer = {
1513+
.io = RTEST(io) ? io : Qfalse,
1514+
};
1515+
fbuffer_stack_init(&buffer, state->buffer_initial_length, stack_buffer, FBUFFER_STACK_SIZE);
1516+
1517+
struct generate_json_data data = {
1518+
.buffer = &buffer,
1519+
.vstate = Qfalse,
1520+
.state = &new_state,
1521+
.obj = obj,
1522+
.func = generate_json
1523+
};
1524+
rb_rescue(generate_json_try, (VALUE)&data, generate_json_rescue, (VALUE)&data);
1525+
1526+
return fbuffer_finalize(&buffer);
14981527
}
14991528

15001529
static VALUE cState_initialize(int argc, VALUE *argv, VALUE self)
@@ -2072,7 +2101,7 @@ void Init_generator(void)
20722101
rb_define_method(cState, "buffer_initial_length", cState_buffer_initial_length, 0);
20732102
rb_define_method(cState, "buffer_initial_length=", cState_buffer_initial_length_set, 1);
20742103
rb_define_method(cState, "generate", cState_generate, -1);
2075-
rb_define_alias(cState, "generate_new", "generate"); // :nodoc:
2104+
rb_define_method(cState, "generate_new", cState_generate_new, -1); // :nodoc:
20762105

20772106
rb_define_private_method(cState, "allow_duplicate_key?", cState_allow_duplicate_key_p, 0);
20782107

java/src/json/ext/GeneratorState.java

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ public IRubyObject initialize_copy(ThreadContext context, IRubyObject vOrig) {
231231
* the result. If no valid JSON document can be created this method raises
232232
* a GeneratorError exception.
233233
*/
234-
@JRubyMethod(alias="generate_new")
234+
@JRubyMethod
235235
public IRubyObject generate(ThreadContext context, IRubyObject obj, IRubyObject io) {
236236
IRubyObject result = Generator.generateJson(context, obj, this, io);
237237
RuntimeInfo info = RuntimeInfo.forRuntime(context.runtime);
@@ -251,11 +251,25 @@ public IRubyObject generate(ThreadContext context, IRubyObject obj, IRubyObject
251251
return resultString;
252252
}
253253

254-
@JRubyMethod(alias="generate_new")
254+
@JRubyMethod
255255
public IRubyObject generate(ThreadContext context, IRubyObject obj) {
256256
return generate(context, obj, context.nil);
257257
}
258258

259+
@JRubyMethod
260+
public IRubyObject generate_new(ThreadContext context, IRubyObject obj, IRubyObject io) {
261+
GeneratorState newState = (GeneratorState)dup();
262+
newState.resetDepth();
263+
return newState.generate(context, obj, io);
264+
}
265+
266+
@JRubyMethod
267+
public IRubyObject generate_new(ThreadContext context, IRubyObject obj) {
268+
GeneratorState newState = (GeneratorState)dup();
269+
newState.resetDepth();
270+
return newState.generate(context, obj, context.nil);
271+
}
272+
259273
@JRubyMethod(name="[]")
260274
public IRubyObject op_aref(ThreadContext context, IRubyObject vName) {
261275
String name = vName.asJavaString();
@@ -478,6 +492,10 @@ public IRubyObject depth_set(IRubyObject vDepth) {
478492
return vDepth;
479493
}
480494

495+
public void resetDepth() {
496+
depth = 0;
497+
}
498+
481499
private ByteList prepareByteList(ThreadContext context, IRubyObject value) {
482500
RubyString str = value.convertToString();
483501
if (str.getEncoding() != UTF8Encoding.INSTANCE) {
@@ -610,7 +628,7 @@ public int decreaseDepth() {
610628
private void checkMaxNesting(ThreadContext context) {
611629
if (maxNesting != 0 && depth > maxNesting) {
612630
depth--;
613-
throw Utils.newException(context, Utils.M_NESTING_ERROR, "nesting of " + depth + " is too deep");
631+
throw Utils.newException(context, Utils.M_NESTING_ERROR, "nesting of " + depth + " is too deep. Did you try to serialize objects with circular references?");
614632
}
615633
}
616634
}

lib/json/truffle_ruby/generator.rb

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ def check_max_nesting # :nodoc:
212212
return if @max_nesting.zero?
213213
current_nesting = depth + 1
214214
current_nesting > @max_nesting and
215-
raise NestingError, "nesting of #{current_nesting} is too deep"
215+
raise NestingError, "nesting of #{current_nesting} is too deep. Did you try to serialize objects with circular references?"
216216
end
217217

218218
# Returns true, if circular data structures are checked,
@@ -347,6 +347,10 @@ def generate_new(obj, anIO = nil) # :nodoc:
347347
dup.generate(obj, anIO)
348348
end
349349

350+
private def initialize_copy(_orig)
351+
@depth = 0
352+
end
353+
350354
# Handles @allow_nan, @buffer_initial_length, other ivars must be the default value (see above)
351355
private def generate_json(obj, buf)
352356
case obj

test/json/json_coder_test.rb

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,4 +66,14 @@ def test_json_coder_dump_NaN_or_Infinity_loop
6666
end
6767
assert_include error.message, "NaN not allowed in JSON"
6868
end
69+
70+
def test_nesting_recovery
71+
coder = JSON::Coder.new
72+
ary = []
73+
ary << ary
74+
assert_raise JSON::NestingError do
75+
coder.dump(ary)
76+
end
77+
assert_equal '{"a":1}', coder.dump({ a: 1 })
78+
end
6979
end

0 commit comments

Comments
 (0)