diff --git a/ruby/ext/google/protobuf_c/map.c b/ruby/ext/google/protobuf_c/map.c index 7b78f87fc0..99de072a6a 100644 --- a/ruby/ext/google/protobuf_c/map.c +++ b/ruby/ext/google/protobuf_c/map.c @@ -563,22 +563,13 @@ VALUE Map_eq(VALUE _self, VALUE _other) { * Freezes the message object. We have to intercept this so we can pin the * Ruby object into memory so we don't forget it's frozen. */ -static VALUE Map_freeze(VALUE _self) { +VALUE Map_freeze(VALUE _self) { Map* self = ruby_to_Map(_self); - if (!RB_OBJ_FROZEN(_self)) { - Arena_Pin(self->arena, _self); - RB_OBJ_FREEZE(_self); - } - return _self; -} -/* - * Deep freezes the map and values recursively. - * Internal use only. - */ -VALUE Map_internal_deep_freeze(VALUE _self) { - Map* self = ruby_to_Map(_self); - Map_freeze(_self); + if (RB_OBJ_FROZEN(_self)) return _self; + Arena_Pin(self->arena, _self); + RB_OBJ_FREEZE(_self); + if (self->value_type_info.type == kUpb_CType_Message) { size_t iter = kUpb_Map_Begin; upb_MessageValue key, val; @@ -586,7 +577,7 @@ VALUE Map_internal_deep_freeze(VALUE _self) { while (upb_Map_Next(self->map, &key, &val, &iter)) { VALUE val_val = Convert_UpbToRuby(val, self->value_type_info, self->arena); - Message_internal_deep_freeze(val_val); + Message_freeze(val_val); } } return _self; diff --git a/ruby/ext/google/protobuf_c/map.h b/ruby/ext/google/protobuf_c/map.h index d3cebc6a69..cb4041d5a0 100644 --- a/ruby/ext/google/protobuf_c/map.h +++ b/ruby/ext/google/protobuf_c/map.h @@ -39,6 +39,6 @@ extern VALUE cMap; void Map_register(VALUE module); // Recursively freeze map -VALUE Map_internal_deep_freeze(VALUE _self); +VALUE Map_freeze(VALUE _self); #endif // RUBY_PROTOBUF_MAP_H_ diff --git a/ruby/ext/google/protobuf_c/message.c b/ruby/ext/google/protobuf_c/message.c index 3b12c93f3d..d2bc27062c 100644 --- a/ruby/ext/google/protobuf_c/message.c +++ b/ruby/ext/google/protobuf_c/message.c @@ -826,22 +826,12 @@ static VALUE Message_to_h(VALUE _self) { * Freezes the message object. We have to intercept this so we can pin the * Ruby object into memory so we don't forget it's frozen. */ -static VALUE Message_freeze(VALUE _self) { +VALUE Message_freeze(VALUE _self) { Message* self = ruby_to_Message(_self); - if (!RB_OBJ_FROZEN(_self)) { - Arena_Pin(self->arena, _self); - RB_OBJ_FREEZE(_self); - } - return _self; -} -/* - * Deep freezes the message object recursively. - * Internal use only. - */ -VALUE Message_internal_deep_freeze(VALUE _self) { - Message* self = ruby_to_Message(_self); - Message_freeze(_self); + if (RB_OBJ_FROZEN(_self)) return _self; + Arena_Pin(self->arena, _self); + RB_OBJ_FREEZE(_self); int n = upb_MessageDef_FieldCount(self->msgdef); for (int i = 0; i < n; i++) { @@ -850,11 +840,11 @@ VALUE Message_internal_deep_freeze(VALUE _self) { if (field != Qnil) { if (upb_FieldDef_IsMap(f)) { - Map_internal_deep_freeze(field); + Map_freeze(field); } else if (upb_FieldDef_IsRepeated(f)) { - RepeatedField_internal_deep_freeze(field); + RepeatedField_freeze(field); } else if (upb_FieldDef_IsSubMessage(f)) { - Message_internal_deep_freeze(field); + Message_freeze(field); } } } @@ -963,7 +953,7 @@ VALUE Message_decode_bytes(int size, const char* bytes, int options, rb_raise(cParseError, "Error occurred during parsing"); } if (freeze) { - Message_internal_deep_freeze(msg_rb); + Message_freeze(msg_rb); } return msg_rb; } diff --git a/ruby/ext/google/protobuf_c/message.h b/ruby/ext/google/protobuf_c/message.h index cb6897f0d4..ab872be43b 100644 --- a/ruby/ext/google/protobuf_c/message.h +++ b/ruby/ext/google/protobuf_c/message.h @@ -78,7 +78,7 @@ VALUE Message_decode_bytes(int size, const char* bytes, int options, VALUE klass, bool freeze); // Recursively freeze message -VALUE Message_internal_deep_freeze(VALUE _self); +VALUE Message_freeze(VALUE _self); // Call at startup to register all types in this module. void Message_register(VALUE protobuf); diff --git a/ruby/ext/google/protobuf_c/repeated_field.c b/ruby/ext/google/protobuf_c/repeated_field.c index 1960126749..15b8abfd2a 100644 --- a/ruby/ext/google/protobuf_c/repeated_field.c +++ b/ruby/ext/google/protobuf_c/repeated_field.c @@ -478,29 +478,20 @@ VALUE RepeatedField_eq(VALUE _self, VALUE _other) { * Freezes the repeated field. We have to intercept this so we can pin the Ruby * object into memory so we don't forget it's frozen. */ -static VALUE RepeatedField_freeze(VALUE _self) { +VALUE RepeatedField_freeze(VALUE _self) { RepeatedField* self = ruby_to_RepeatedField(_self); - if (!RB_OBJ_FROZEN(_self)) { - Arena_Pin(self->arena, _self); - RB_OBJ_FREEZE(_self); - } - return _self; -} -/* - * Deep freezes the repeated field and values recursively. - * Internal use only. - */ -VALUE RepeatedField_internal_deep_freeze(VALUE _self) { - RepeatedField* self = ruby_to_RepeatedField(_self); - RepeatedField_freeze(_self); + if (RB_OBJ_FROZEN(_self)) return _self; + Arena_Pin(self->arena, _self); + RB_OBJ_FREEZE(_self); + if (self->type_info.type == kUpb_CType_Message) { int size = upb_Array_Size(self->array); int i; for (i = 0; i < size; i++) { upb_MessageValue msgval = upb_Array_Get(self->array, i); VALUE val = Convert_UpbToRuby(msgval, self->type_info, self->arena); - Message_internal_deep_freeze(val); + Message_freeze(val); } } return _self; diff --git a/ruby/ext/google/protobuf_c/repeated_field.h b/ruby/ext/google/protobuf_c/repeated_field.h index f3f7a50cd5..f5d5e223cb 100644 --- a/ruby/ext/google/protobuf_c/repeated_field.h +++ b/ruby/ext/google/protobuf_c/repeated_field.h @@ -36,6 +36,6 @@ extern VALUE cRepeatedField; void RepeatedField_register(VALUE module); // Recursively freeze RepeatedField. -VALUE RepeatedField_internal_deep_freeze(VALUE _self); +VALUE RepeatedField_freeze(VALUE _self); #endif // RUBY_PROTOBUF_REPEATED_FIELD_H_ diff --git a/ruby/lib/google/protobuf/ffi/descriptor.rb b/ruby/lib/google/protobuf/ffi/descriptor.rb index 25d226abd1..3eb108663a 100644 --- a/ruby/lib/google/protobuf/ffi/descriptor.rb +++ b/ruby/lib/google/protobuf/ffi/descriptor.rb @@ -100,7 +100,7 @@ module Google size_ptr = ::FFI::MemoryPointer.new(:size_t, 1) temporary_arena = Google::Protobuf::FFI.create_arena buffer = Google::Protobuf::FFI.message_options(self, size_ptr, temporary_arena) - Google::Protobuf::MessageOptions.decode(buffer.read_string_length(size_ptr.read(:size_t)).force_encoding("ASCII-8BIT").freeze).send(:internal_deep_freeze) + Google::Protobuf::MessageOptions.decode(buffer.read_string_length(size_ptr.read(:size_t)).force_encoding("ASCII-8BIT").freeze).freeze end end @@ -138,7 +138,7 @@ module Google message = OBJECT_CACHE.get(msg.address) if message.nil? message = descriptor.msgclass.send(:private_constructor, arena, msg: msg) - message.send :internal_deep_freeze if frozen? + message.freeze if frozen? end message end diff --git a/ruby/lib/google/protobuf/ffi/enum_descriptor.rb b/ruby/lib/google/protobuf/ffi/enum_descriptor.rb index a1f4fefcd5..e280e393a1 100644 --- a/ruby/lib/google/protobuf/ffi/enum_descriptor.rb +++ b/ruby/lib/google/protobuf/ffi/enum_descriptor.rb @@ -84,7 +84,7 @@ module Google size_ptr = ::FFI::MemoryPointer.new(:size_t, 1) temporary_arena = Google::Protobuf::FFI.create_arena buffer = Google::Protobuf::FFI.enum_options(self, size_ptr, temporary_arena) - Google::Protobuf::EnumOptions.decode(buffer.read_string_length(size_ptr.read(:size_t)).force_encoding("ASCII-8BIT").freeze).send(:internal_deep_freeze) + Google::Protobuf::EnumOptions.decode(buffer.read_string_length(size_ptr.read(:size_t)).force_encoding("ASCII-8BIT").freeze).freeze end end diff --git a/ruby/lib/google/protobuf/ffi/field_descriptor.rb b/ruby/lib/google/protobuf/ffi/field_descriptor.rb index d7c45da193..b15c910667 100644 --- a/ruby/lib/google/protobuf/ffi/field_descriptor.rb +++ b/ruby/lib/google/protobuf/ffi/field_descriptor.rb @@ -211,7 +211,7 @@ module Google size_ptr = ::FFI::MemoryPointer.new(:size_t, 1) temporary_arena = Google::Protobuf::FFI.create_arena buffer = Google::Protobuf::FFI.field_options(self, size_ptr, temporary_arena) - Google::Protobuf::FieldOptions.decode(buffer.read_string_length(size_ptr.read(:size_t)).force_encoding("ASCII-8BIT").freeze).send(:internal_deep_freeze) + Google::Protobuf::FieldOptions.decode(buffer.read_string_length(size_ptr.read(:size_t)).force_encoding("ASCII-8BIT").freeze).freeze end end diff --git a/ruby/lib/google/protobuf/ffi/file_descriptor.rb b/ruby/lib/google/protobuf/ffi/file_descriptor.rb index 291ac4f3e4..f1da9f7385 100644 --- a/ruby/lib/google/protobuf/ffi/file_descriptor.rb +++ b/ruby/lib/google/protobuf/ffi/file_descriptor.rb @@ -51,7 +51,7 @@ module Google size_ptr = ::FFI::MemoryPointer.new(:size_t, 1) temporary_arena = Google::Protobuf::FFI.create_arena buffer = Google::Protobuf::FFI.file_options(@file_def, size_ptr, temporary_arena) - Google::Protobuf::FileOptions.decode(buffer.read_string_length(size_ptr.read(:size_t)).force_encoding("ASCII-8BIT").freeze).send(:internal_deep_freeze) + Google::Protobuf::FileOptions.decode(buffer.read_string_length(size_ptr.read(:size_t)).force_encoding("ASCII-8BIT").freeze).freeze end end end diff --git a/ruby/lib/google/protobuf/ffi/map.rb b/ruby/lib/google/protobuf/ffi/map.rb index 61abbe53b0..90898b66a9 100644 --- a/ruby/lib/google/protobuf/ffi/map.rb +++ b/ruby/lib/google/protobuf/ffi/map.rb @@ -155,6 +155,19 @@ module Google end alias size length + def freeze + return self if frozen? + super + @arena.pin self + if value_type == :message + internal_iterator do |iterator| + value_message_value = Google::Protobuf::FFI.map_value(@map_ptr, iterator) + convert_upb_to_ruby(value_message_value, value_type, descriptor, arena).freeze + end + end + self + end + ## # call-seq: # Map.dup => new_map @@ -269,17 +282,6 @@ module Google include Google::Protobuf::Internal::Convert - def internal_deep_freeze - freeze - if value_type == :message - internal_iterator do |iterator| - value_message_value = Google::Protobuf::FFI.map_value(@map_ptr, iterator) - convert_upb_to_ruby(value_message_value, value_type, descriptor, arena).send :internal_deep_freeze - end - end - self - end - def internal_iterator iter = ::FFI::MemoryPointer.new(:size_t, 1) iter.write(:size_t, Google::Protobuf::FFI::Upb_Map_Begin) diff --git a/ruby/lib/google/protobuf/ffi/message.rb b/ruby/lib/google/protobuf/ffi/message.rb index 5d9a365927..c1b9b4318e 100644 --- a/ruby/lib/google/protobuf/ffi/message.rb +++ b/ruby/lib/google/protobuf/ffi/message.rb @@ -59,8 +59,15 @@ module Google end def freeze + return self if frozen? super @arena.pin self + self.class.descriptor.each do |field_descriptor| + next if field_descriptor.has_presence? && !Google::Protobuf::FFI.get_message_has(@msg, field_descriptor) + if field_descriptor.map? or field_descriptor.repeated? or field_descriptor.sub_message? + get_field(field_descriptor).freeze + end + end self end @@ -302,17 +309,6 @@ module Google include Google::Protobuf::Internal::Convert - def internal_deep_freeze - freeze - self.class.descriptor.each do |field_descriptor| - next if field_descriptor.has_presence? && !Google::Protobuf::FFI.get_message_has(@msg, field_descriptor) - if field_descriptor.map? or field_descriptor.repeated? or field_descriptor.sub_message? - get_field(field_descriptor).send :internal_deep_freeze - end - end - self - end - def self.setup_accessors! @descriptor.each do |field_descriptor| field_name = field_descriptor.name @@ -639,7 +635,7 @@ module Google repeated_field = OBJECT_CACHE.get(array.address) if repeated_field.nil? repeated_field = RepeatedField.send(:construct_for_field, field, @arena, array: array) - repeated_field.send :internal_deep_freeze if frozen? + repeated_field.freeze if frozen? end repeated_field end @@ -652,7 +648,7 @@ module Google map_field = OBJECT_CACHE.get(map.address) if map_field.nil? map_field = Google::Protobuf::Map.send(:construct_for_field, field, @arena, map: map) - map_field.send :internal_deep_freeze if frozen? + map_field.freeze if frozen? end map_field end diff --git a/ruby/lib/google/protobuf/ffi/oneof_descriptor.rb b/ruby/lib/google/protobuf/ffi/oneof_descriptor.rb index 00acc995c1..934f8f0ac4 100644 --- a/ruby/lib/google/protobuf/ffi/oneof_descriptor.rb +++ b/ruby/lib/google/protobuf/ffi/oneof_descriptor.rb @@ -58,7 +58,7 @@ module Google size_ptr = ::FFI::MemoryPointer.new(:size_t, 1) temporary_arena = Google::Protobuf::FFI.create_arena buffer = Google::Protobuf::FFI.oneof_options(self, size_ptr, temporary_arena) - Google::Protobuf::OneofOptions.decode(buffer.read_string_length(size_ptr.read(:size_t)).force_encoding("ASCII-8BIT").freeze).send(:internal_deep_freeze) + Google::Protobuf::OneofOptions.decode(buffer.read_string_length(size_ptr.read(:size_t)).force_encoding("ASCII-8BIT").freeze).freeze end end diff --git a/ruby/lib/google/protobuf/ffi/repeated_field.rb b/ruby/lib/google/protobuf/ffi/repeated_field.rb index ccc95ad6f4..25d2ab3563 100644 --- a/ruby/lib/google/protobuf/ffi/repeated_field.rb +++ b/ruby/lib/google/protobuf/ffi/repeated_field.rb @@ -174,6 +174,18 @@ module Google end alias size :length + def freeze + return self if frozen? + super + @arena.pin self + if type == :message + each do |element| + element.freeze + end + end + self + end + def dup instance = self.class.allocate instance.send(:initialize, type, descriptor: descriptor, arena: arena) @@ -252,16 +264,6 @@ module Google attr :name, :arena, :array, :type, :descriptor - def internal_deep_freeze - freeze - if type == :message - each do |element| - element.send :internal_deep_freeze - end - end - self - end - def internal_push(*elements) elements.each do |element| append_msg_val convert_ruby_to_upb(element, arena, type, descriptor) diff --git a/ruby/src/main/java/com/google/protobuf/jruby/RubyMap.java b/ruby/src/main/java/com/google/protobuf/jruby/RubyMap.java index f3849b1b2a..204608d026 100644 --- a/ruby/src/main/java/com/google/protobuf/jruby/RubyMap.java +++ b/ruby/src/main/java/com/google/protobuf/jruby/RubyMap.java @@ -372,11 +372,15 @@ public class RubyMap extends RubyObject { return RubyHash.newHash(context.runtime, mapForHash, context.nil); } - protected IRubyObject deepFreeze(ThreadContext context) { + @JRubyMethod + public IRubyObject freeze(ThreadContext context) { + if (isFrozen()) { + return this; + } setFrozen(true); if (valueType == FieldDescriptor.Type.MESSAGE) { for (IRubyObject key : table.keySet()) { - ((RubyMessage) table.get(key)).deepFreeze(context); + ((RubyMessage) table.get(key)).freeze(context); } } return this; diff --git a/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java b/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java index 2ac9fc912e..365f23c7b2 100644 --- a/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java +++ b/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java @@ -664,7 +664,7 @@ public class RubyMessage extends RubyObject { }); } if (freeze) { - ret.deepFreeze(context); + ret.freeze(context); } return ret; } @@ -814,16 +814,20 @@ public class RubyMessage extends RubyObject { return ret; } - protected IRubyObject deepFreeze(ThreadContext context) { + @JRubyMethod + public IRubyObject freeze(ThreadContext context) { + if (isFrozen()) { + return this; + } setFrozen(true); for (FieldDescriptor fdef : descriptor.getFields()) { if (fdef.isMapField()) { - ((RubyMap) fields.get(fdef)).deepFreeze(context); + ((RubyMap) fields.get(fdef)).freeze(context); } else if (fdef.isRepeated()) { - this.getRepeatedField(context, fdef).deepFreeze(context); + this.getRepeatedField(context, fdef).freeze(context); } else if (fields.containsKey(fdef)) { if (fdef.getType() == FieldDescriptor.Type.MESSAGE) { - ((RubyMessage) fields.get(fdef)).deepFreeze(context); + ((RubyMessage) fields.get(fdef)).freeze(context); } } } diff --git a/ruby/src/main/java/com/google/protobuf/jruby/RubyRepeatedField.java b/ruby/src/main/java/com/google/protobuf/jruby/RubyRepeatedField.java index 085dd1cc0f..595afbc246 100644 --- a/ruby/src/main/java/com/google/protobuf/jruby/RubyRepeatedField.java +++ b/ruby/src/main/java/com/google/protobuf/jruby/RubyRepeatedField.java @@ -358,11 +358,15 @@ public class RubyRepeatedField extends RubyObject { return storage.inspect(); } - protected IRubyObject deepFreeze(ThreadContext context) { + @JRubyMethod + public IRubyObject freeze(ThreadContext context) { + if (isFrozen()) { + return this; + } setFrozen(true); if (fieldType == FieldDescriptor.Type.MESSAGE) { for (int i = 0; i < size(); i++) { - ((RubyMessage) storage.eltInternal(i)).deepFreeze(context); + ((RubyMessage) storage.eltInternal(i)).freeze(context); } } return this; diff --git a/ruby/tests/basic.rb b/ruby/tests/basic.rb index b71fdac4ea..46e3fec49e 100755 --- a/ruby/tests/basic.rb +++ b/ruby/tests/basic.rb @@ -696,15 +696,14 @@ module BasicTest end end - def test_message_deep_freeze + def test_message_freeze message = TestDeprecatedMessage.new - omit(":internal_deep_freeze only exists under FFI") unless message.respond_to? :internal_deep_freeze, true nested_message_2 = TestMessage2.new message.map_string_msg["message"] = TestMessage2.new message.repeated_msg.push(TestMessage2.new) - message.send(:internal_deep_freeze) + message.freeze assert_raise FrozenError do message.map_string_msg["message"].foo = "bar"