Skip to content

Ractor-shareable JSON::Coder #897

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 21, 2025
+117 −7
Merged

Ractor-shareable JSON::Coder #897

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1df7523
Ractor-shareable JSON::Coder
etiennebarrie Nov 18, 2025
b7e1734
Skip test failing with JRuby in CI
etiennebarrie Nov 21, 2025
1219ce0
Move RUBY_TYPED_FROZEN_SHAREABLE macro to json.h
etiennebarrie Nov 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions ext/json/ext/generator/generator.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -926,11 +926,6 @@ static size_t State_memsize(const void *ptr)
return sizeof(JSON_Generator_State);
}

#ifndef HAVE_RB_EXT_RACTOR_SAFE
# undef RUBY_TYPED_FROZEN_SHAREABLE
# define RUBY_TYPED_FROZEN_SHAREABLE 0
#endif

static const rb_data_type_t JSON_Generator_State_type = {
"JSON/Generator/State",
{
Expand Down Expand Up @@ -1630,6 +1625,7 @@ static VALUE string_config(VALUE config)
*/
static VALUE cState_indent_set(VALUE self, VALUE indent)
{
rb_check_frozen(self);
GET_STATE(self);
RB_OBJ_WRITE(self, &state->indent, string_config(indent));
return Qnil;
Expand All @@ -1655,6 +1651,7 @@ static VALUE cState_space(VALUE self)
*/
static VALUE cState_space_set(VALUE self, VALUE space)
{
rb_check_frozen(self);
GET_STATE(self);
RB_OBJ_WRITE(self, &state->space, string_config(space));
return Qnil;
Expand All @@ -1678,6 +1675,7 @@ static VALUE cState_space_before(VALUE self)
*/
static VALUE cState_space_before_set(VALUE self, VALUE space_before)
{
rb_check_frozen(self);
GET_STATE(self);
RB_OBJ_WRITE(self, &state->space_before, string_config(space_before));
return Qnil;
Expand All @@ -1703,6 +1701,7 @@ static VALUE cState_object_nl(VALUE self)
*/
static VALUE cState_object_nl_set(VALUE self, VALUE object_nl)
{
rb_check_frozen(self);
GET_STATE(self);
RB_OBJ_WRITE(self, &state->object_nl, string_config(object_nl));
return Qnil;
Expand All @@ -1726,6 +1725,7 @@ static VALUE cState_array_nl(VALUE self)
*/
static VALUE cState_array_nl_set(VALUE self, VALUE array_nl)
{
rb_check_frozen(self);
GET_STATE(self);
RB_OBJ_WRITE(self, &state->array_nl, string_config(array_nl));
return Qnil;
Expand All @@ -1749,6 +1749,7 @@ static VALUE cState_as_json(VALUE self)
*/
static VALUE cState_as_json_set(VALUE self, VALUE as_json)
{
rb_check_frozen(self);
GET_STATE(self);
RB_OBJ_WRITE(self, &state->as_json, rb_convert_type(as_json, T_DATA, "Proc", "to_proc"));
return Qnil;
Expand Down Expand Up @@ -1791,6 +1792,7 @@ static long long_config(VALUE num)
*/
static VALUE cState_max_nesting_set(VALUE self, VALUE depth)
{
rb_check_frozen(self);
GET_STATE(self);
state->max_nesting = long_config(depth);
return Qnil;
Expand All @@ -1816,6 +1818,7 @@ static VALUE cState_script_safe(VALUE self)
*/
static VALUE cState_script_safe_set(VALUE self, VALUE enable)
{
rb_check_frozen(self);
GET_STATE(self);
state->script_safe = RTEST(enable);
return Qnil;
Expand Down Expand Up @@ -1847,6 +1850,7 @@ static VALUE cState_strict(VALUE self)
*/
static VALUE cState_strict_set(VALUE self, VALUE enable)
{
rb_check_frozen(self);
GET_STATE(self);
state->strict = RTEST(enable);
return Qnil;
Expand All @@ -1871,6 +1875,7 @@ static VALUE cState_allow_nan_p(VALUE self)
*/
static VALUE cState_allow_nan_set(VALUE self, VALUE enable)
{
rb_check_frozen(self);
GET_STATE(self);
state->allow_nan = RTEST(enable);
return Qnil;
Expand All @@ -1895,6 +1900,7 @@ static VALUE cState_ascii_only_p(VALUE self)
*/
static VALUE cState_ascii_only_set(VALUE self, VALUE enable)
{
rb_check_frozen(self);
GET_STATE(self);
state->ascii_only = RTEST(enable);
return Qnil;
Expand Down Expand Up @@ -1932,6 +1938,7 @@ static VALUE cState_depth(VALUE self)
*/
static VALUE cState_depth_set(VALUE self, VALUE depth)
{
rb_check_frozen(self);
GET_STATE(self);
state->depth = long_config(depth);
return Qnil;
Expand Down Expand Up @@ -1965,6 +1972,7 @@ static void buffer_initial_length_set(JSON_Generator_State *state, VALUE buffer_
*/
static VALUE cState_buffer_initial_length_set(VALUE self, VALUE buffer_initial_length)
{
rb_check_frozen(self);
GET_STATE(self);
buffer_initial_length_set(state, buffer_initial_length);
return Qnil;
Expand Down Expand Up @@ -2031,6 +2039,7 @@ static void configure_state(JSON_Generator_State *state, VALUE vstate, VALUE con

static VALUE cState_configure(VALUE self, VALUE opts)
{
rb_check_frozen(self);
GET_STATE(self);
configure_state(state, self, opts);
return self;
Expand Down
5 changes: 5 additions & 0 deletions ext/json/ext/json.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ typedef unsigned char _Bool;
#endif
#endif

#ifndef HAVE_RB_EXT_RACTOR_SAFE
# undef RUBY_TYPED_FROZEN_SHAREABLE
# define RUBY_TYPED_FROZEN_SHAREABLE 0
#endif

#ifndef NORETURN
#define NORETURN(x) x
#endif
Expand Down
3 changes: 2 additions & 1 deletion ext/json/ext/parser/parser.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1467,6 +1467,7 @@ static void parser_config_init(JSON_ParserConfig *config, VALUE opts)
*/
static VALUE cParserConfig_initialize(VALUE self, VALUE opts)
{
rb_check_frozen(self);
GET_PARSER_CONFIG;

parser_config_init(config, opts);
Expand Down Expand Up @@ -1562,7 +1563,7 @@ static const rb_data_type_t JSON_ParserConfig_type = {
JSON_ParserConfig_memsize,
},
0, 0,
RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED,
RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED | RUBY_TYPED_FROZEN_SHAREABLE,
Copy link
Member

@eregon eregon Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems correct because all the fields in

json/ext/json/ext/parser/parser.c

Lines 331 to 342 in 7b1fb27

typedef struct JSON_ParserStruct {
VALUE on_load_proc;
VALUE decimal_class;
ID decimal_method_id;
enum duplicate_key_action on_duplicate_key;
int max_nesting;
bool allow_nan;
bool allow_trailing_comma;
bool parsing_name;
bool symbolize_names;
bool freeze;
} JSON_ParserConfig;

are only written in parser_config_init().
However we need to make sure that's not called multiple times, so there should be a rb_check_frozen() at the beginning of parser_config_init().

BTW bool parsing_name; seems unused.

Copy link
Member

@byroot byroot Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW bool parsing_name; seems unused.

Good catch: ab5efca

Copy link
Contributor Author

@etiennebarrie etiennebarrie Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eregon Thanks for the review! I added a call to rb_check_frozen() in cParserConfig_initialize.

>> JSON::Coder.new.freeze.instance_variable_get(:@parser_config).send(:initialize, {})
(irb):1:in 'JSON::Ext::ParserConfig#initialize': can't modify frozen JSON::Ext::ParserConfig: #<JSON::Ext::ParserConfig:0x0000000125000d40> (FrozenError)

I wasn't sure whether to add a test for that, it looked like this:

    coder = JSON::Coder.new.freeze
    parser_config = coder.instance_variable_get(:@parser_config)
    assert_instance_of JSON::Parser::Config, parser_config
    assert_raise FrozenError do
      parser_config.send(:initialize, {})
    end

BTW I just realized that the Generator::State object is already frozen in initialize, I'll actually do the same for the Parser::Config.

Copy link
Contributor Author

@etiennebarrie etiennebarrie Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll also need to make sure a frozen Ext::Generator::State is not modified:

>> state = JSON::Coder.new.instance_variable_get(:@state)
=> #<JSON::Ext::Generator::State:0x000000012557c7d8>
>> state.frozen?
=> true
>> state.max_nesting
=> 100
>> state.max_nesting=1
=> 1
>> state.max_nesting
=> 1

Copy link
Member

@eregon eregon Nov 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, that's already marked as RUBY_TYPED_FROZEN_SHAREABLE but it's not correct since it can still be mutated. So I guess we need rb_check_frozen() in all methods that can mutate a Ext::Generator::State then.
For Ext::Generator::State fields which don't contain a shareable value (e.g. strings) it would break the Ractor invariant (unshareable objects must always stay in a single Ractor) and probably segfault. And it shouldn't be visible as mutable anyway if marked as RUBY_TYPED_FROZEN_SHAREABLE.

};

static VALUE cJSON_parser_s_allocate(VALUE klass)
Expand Down
13 changes: 13 additions & 0 deletions java/src/json/ext/GeneratorState.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,7 @@ public IRubyObject op_aref(ThreadContext context, IRubyObject vName) {

@JRubyMethod(name="[]=")
public IRubyObject op_aset(ThreadContext context, IRubyObject vName, IRubyObject value) {
checkFrozen();
String name = vName.asJavaString();
String nameWriter = name + "=";
if (getMetaClass().isMethodBound(nameWriter, true)) {
Expand All @@ -304,6 +305,7 @@ public RubyString indent_get(ThreadContext context) {

@JRubyMethod(name="indent=")
public IRubyObject indent_set(ThreadContext context, IRubyObject indent) {
checkFrozen();
this.indent = prepareByteList(context, indent);
return indent;
}
Expand All @@ -319,6 +321,7 @@ public RubyString space_get(ThreadContext context) {

@JRubyMethod(name="space=")
public IRubyObject space_set(ThreadContext context, IRubyObject space) {
checkFrozen();
this.space = prepareByteList(context, space);
return space;
}
Expand All @@ -335,6 +338,7 @@ public RubyString space_before_get(ThreadContext context) {
@JRubyMethod(name="space_before=")
public IRubyObject space_before_set(ThreadContext context,
IRubyObject spaceBefore) {
checkFrozen();
this.spaceBefore = prepareByteList(context, spaceBefore);
return spaceBefore;
}
Expand All @@ -351,6 +355,7 @@ public RubyString object_nl_get(ThreadContext context) {
@JRubyMethod(name="object_nl=")
public IRubyObject object_nl_set(ThreadContext context,
IRubyObject objectNl) {
checkFrozen();
this.objectNl = prepareByteList(context, objectNl);
return objectNl;
}
Expand All @@ -367,6 +372,7 @@ public RubyString array_nl_get(ThreadContext context) {
@JRubyMethod(name="array_nl=")
public IRubyObject array_nl_set(ThreadContext context,
IRubyObject arrayNl) {
checkFrozen();
this.arrayNl = prepareByteList(context, arrayNl);
return arrayNl;
}
Expand All @@ -382,6 +388,7 @@ public IRubyObject as_json_get(ThreadContext context) {

@JRubyMethod(name="as_json=")
public IRubyObject as_json_set(ThreadContext context, IRubyObject asJSON) {
checkFrozen();
if (asJSON.isNil() || asJSON == context.getRuntime().getFalse()) {
this.asJSON = null;
} else {
Expand All @@ -402,6 +409,7 @@ public RubyInteger max_nesting_get(ThreadContext context) {

@JRubyMethod(name="max_nesting=")
public IRubyObject max_nesting_set(IRubyObject max_nesting) {
checkFrozen();
maxNesting = RubyNumeric.fix2int(max_nesting);
return max_nesting;
}
Expand All @@ -420,6 +428,7 @@ public RubyBoolean script_safe_get(ThreadContext context) {

@JRubyMethod(name="script_safe=", alias="escape_slash=")
public IRubyObject script_safe_set(IRubyObject script_safe) {
checkFrozen();
scriptSafe = script_safe.isTrue();
return script_safe.getRuntime().newBoolean(scriptSafe);
}
Expand All @@ -443,6 +452,7 @@ public RubyBoolean strict_get(ThreadContext context) {

@JRubyMethod(name="strict=")
public IRubyObject strict_set(IRubyObject isStrict) {
checkFrozen();
strict = isStrict.isTrue();
return isStrict.getRuntime().newBoolean(strict);
}
Expand Down Expand Up @@ -472,6 +482,7 @@ public RubyInteger buffer_initial_length_get(ThreadContext context) {

@JRubyMethod(name="buffer_initial_length=")
public IRubyObject buffer_initial_length_set(IRubyObject buffer_initial_length) {
checkFrozen();
int newLength = RubyNumeric.fix2int(buffer_initial_length);
if (newLength > 0) bufferInitialLength = newLength;
return buffer_initial_length;
Expand All @@ -488,6 +499,7 @@ public RubyInteger depth_get(ThreadContext context) {

@JRubyMethod(name="depth=")
public IRubyObject depth_set(IRubyObject vDepth) {
checkFrozen();
depth = RubyNumeric.fix2int(vDepth);
return vDepth;
}
Expand Down Expand Up @@ -532,6 +544,7 @@ public boolean getDeprecateDuplicateKey() {
*/
@JRubyMethod(visibility=Visibility.PRIVATE)
public IRubyObject _configure(ThreadContext context, IRubyObject vOpts) {
checkFrozen();
OptionsReader opts = new OptionsReader(context, vOpts);

ByteList indent = opts.getString("indent");
Expand Down
1 change: 1 addition & 0 deletions java/src/json/ext/ParserConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ public static IRubyObject newInstance(IRubyObject clazz, IRubyObject arg0, IRuby

@JRubyMethod(visibility = Visibility.PRIVATE)
public IRubyObject initialize(ThreadContext context, IRubyObject options) {
checkFrozen();
Ruby runtime = context.runtime;

OptionsReader opts = new OptionsReader(context, options);
Expand Down
2 changes: 1 addition & 1 deletion lib/json/common.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1048,7 +1048,7 @@ def initialize(options = nil, &as_json)
options[:as_json] = as_json if as_json

@state = State.new(options).freeze
@parser_config = Ext::Parser::Config.new(ParserOptions.prepare(options))
@parser_config = Ext::Parser::Config.new(ParserOptions.prepare(options)).freeze
end

# call-seq:
Expand Down
14 changes: 14 additions & 0 deletions test/json/json_generator_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -901,4 +901,18 @@ def test_generate_duplicate_keys_disallowed
end
assert_equal %(detected duplicate key "foo" in #{hash.inspect}), error.message
end

def test_frozen
state = JSON::State.new.freeze
assert_raise(FrozenError) do
state.configure(max_nesting: 1)
end
setters = state.methods.grep(/\w=$/)
assert_not_empty setters
setters.each do |setter|
assert_raise(FrozenError) do
state.send(setter, 1)
end
end
end
end
8 changes: 8 additions & 0 deletions test/json/json_parser_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -820,6 +820,14 @@ def test_parse_whitespace_after_newline
assert_equal [], JSON.parse("[\n#{' ' * (8 + 8 + 4 + 3)}]")
end

def test_frozen
parser_config = JSON::Parser::Config.new({}).freeze
omit "JRuby failure in CI" if RUBY_ENGINE == "jruby"
assert_raise FrozenError do
parser_config.send(:initialize, {})
end
end

private

def assert_equal_float(expected, actual, delta = 1e-2)
Expand Down
59 changes: 59 additions & 0 deletions test/json/ractor_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,63 @@ def test_generate
_, status = Process.waitpid2(pid)
assert_predicate status, :success?
end

def test_coder
coder = JSON::Coder.new.freeze
assert Ractor.shareable?(coder)
pid = fork do
Warning[:experimental] = false
r = Ractor.new(coder) do |coder|
json = coder.dump({
'a' => 2,
'b' => 3.141,
'c' => 'c',
'd' => [ 1, "b", 3.14 ],
'e' => { 'foo' => 'bar' },
'g' => "\"\0\037",
'h' => 1000.0,
'i' => 0.001
})
coder.load(json)
end
expected_json = JSON.parse('{"a":2,"b":3.141,"c":"c","d":[1,"b",3.14],"e":{"foo":"bar"},' +
'"g":"\\"\\u0000\\u001f","h":1000.0,"i":0.001}')
actual_json = r.value

if expected_json == actual_json
exit 0
else
puts "Expected:"
puts expected_json
puts "Actual:"
puts actual_json
puts
exit 1
end
end
_, status = Process.waitpid2(pid)
assert_predicate status, :success?
end

class NonNative
def initialize(value)
@value = value
end
end

def test_coder_proc
block = Ractor.shareable_proc { |value| value.as_json }
coder = JSON::Coder.new(&block).freeze
assert Ractor.shareable?(coder)

pid = fork do
Warning[:experimental] = false
assert_equal [{}], Ractor.new(coder) { |coder|
coder.load('[{}]')
}.value
end

_, status = Process.waitpid2(pid)
assert_predicate status, :success?
end if Ractor.respond_to?(:shareable_proc)
end if defined?(Ractor) && Process.respond_to?(:fork)
Loading