From 0c5583c02b16ae4d133706fc2660a2a60f492c35 Mon Sep 17 00:00:00 2001 From: Hartley McGuire Date: Thu, 6 Aug 2026 02:02:58 -0400 Subject: [PATCH] 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`). --- ext/java/org/msgpack/jruby/Factory.java | 1 + ext/msgpack/factory_class.c | 4 ++++ spec/factory_spec.rb | 4 ++-- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/ext/java/org/msgpack/jruby/Factory.java b/ext/java/org/msgpack/jruby/Factory.java index 5973f8fb..dac2666d 100644 --- a/ext/java/org/msgpack/jruby/Factory.java +++ b/ext/java/org/msgpack/jruby/Factory.java @@ -59,6 +59,7 @@ public IRubyObject dup() { Factory clone = (Factory)super.dup(); clone.extensionRegistry = extensionRegistry(); clone.hasSymbolExtType = hasSymbolExtType; + clone.hasBigIntExtType = hasBigIntExtType; return clone; } diff --git a/ext/msgpack/factory_class.c b/ext/msgpack/factory_class.c index 1eeb0081..c4683a2a 100644 --- a/ext/msgpack/factory_class.c +++ b/ext/msgpack/factory_class.c @@ -121,7 +121,10 @@ static VALUE Factory_dup(VALUE self) msgpack_factory_t *fc = Factory_get(self); msgpack_factory_t *cloned_fc = Factory_get(clone); + cloned_fc->has_bigint_ext_type = fc->has_bigint_ext_type; cloned_fc->has_symbol_ext_type = fc->has_symbol_ext_type; + cloned_fc->optimized_symbol_ext_type = fc->optimized_symbol_ext_type; + cloned_fc->symbol_ext_type = fc->symbol_ext_type; cloned_fc->pkrg = fc->pkrg; msgpack_unpacker_ext_registry_borrow(fc->ukrg, &cloned_fc->ukrg); 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 } if(ext_module == rb_cSymbol) { + fc->symbol_ext_type = ext_type; if(NIL_P(options) || RTEST(rb_hash_aref(options, ID2SYM(rb_intern("packer"))))) { fc->has_symbol_ext_type = true; } diff --git a/spec/factory_spec.rb b/spec/factory_spec.rb index b8fefee7..d86cac4c 100644 --- a/spec/factory_spec.rb +++ b/spec/factory_spec.rb @@ -512,10 +512,10 @@ def roundtrip(object, options = nil) before do skip if IS_JRUBY # JRuby implementation doesn't support the optimized symbols unpacker for now subject.register_type( - 0x00, + 0x01, ::Symbol, packer: :to_msgpack_ext, - unpacker: :from_msgpack_ext, + unpacker: ->(_) { raise "symbol unpacking not optimized" }, optimized_symbols_parsing: true, ) end