Skip to content

Commit d74dbc4

Browse files
Fix some Factory/ext_type optimization edge cases
Previously, optimized_symbol_parsing would enable a fast path for symbols but it never actually registered the Symbol's ext_type (so it was always 0x0). This resulted in the optimization not working if Symbol is registered with a different type and could additionally cause data corruption if some other type is registered as 0x0. Additionally, many of these Factory fields were not copied over on dup, meaning pooled Factories would have optimized_symbol_parsing disabled if they don't explicitly `freeze` before calling `pool` (which would do `dup.freeze`).
1 parent 42378b0 commit d74dbc4

3 files changed

Lines changed: 7 additions & 2 deletions

File tree

ext/java/org/msgpack/jruby/Factory.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ public IRubyObject dup() {
5959
Factory clone = (Factory)super.dup();
6060
clone.extensionRegistry = extensionRegistry();
6161
clone.hasSymbolExtType = hasSymbolExtType;
62+
clone.hasBigIntExtType = hasBigIntExtType;
6263
return clone;
6364
}
6465

ext/msgpack/factory_class.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,10 @@ static VALUE Factory_dup(VALUE self)
121121
msgpack_factory_t *fc = Factory_get(self);
122122
msgpack_factory_t *cloned_fc = Factory_get(clone);
123123

124+
cloned_fc->has_bigint_ext_type = fc->has_bigint_ext_type;
124125
cloned_fc->has_symbol_ext_type = fc->has_symbol_ext_type;
126+
cloned_fc->optimized_symbol_ext_type = fc->optimized_symbol_ext_type;
127+
cloned_fc->symbol_ext_type = fc->symbol_ext_type;
125128
cloned_fc->pkrg = fc->pkrg;
126129
msgpack_unpacker_ext_registry_borrow(fc->ukrg, &cloned_fc->ukrg);
127130
msgpack_packer_ext_registry_dup(clone, &fc->pkrg, &cloned_fc->pkrg);
@@ -230,6 +233,7 @@ static VALUE Factory_register_type_internal(VALUE self, VALUE rb_ext_type, VALUE
230233
}
231234

232235
if(ext_module == rb_cSymbol) {
236+
fc->symbol_ext_type = ext_type;
233237
if(NIL_P(options) || RTEST(rb_hash_aref(options, ID2SYM(rb_intern("packer"))))) {
234238
fc->has_symbol_ext_type = true;
235239
}

spec/factory_spec.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -512,10 +512,10 @@ def roundtrip(object, options = nil)
512512
before do
513513
skip if IS_JRUBY # JRuby implementation doesn't support the optimized symbols unpacker for now
514514
subject.register_type(
515-
0x00,
515+
0x01,
516516
::Symbol,
517517
packer: :to_msgpack_ext,
518-
unpacker: :from_msgpack_ext,
518+
unpacker: ->(_) { raise "symbol unpacking not optimized" },
519519
optimized_symbols_parsing: true,
520520
)
521521
end

0 commit comments

Comments
 (0)