ruby-changes:73408
From: nagachika <ko1@a...>
Date: Sun, 4 Sep 2022 16:25:34 +0900 (JST)
Subject: [ruby-changes:73408] 882d4203f1 (ruby_3_1): merge revision(s) ef525b012a709077ea2797e8642fae0b61234063,dc9a13abeef5a2b936fbb55edc112b8b382a05e7: [Backport #18775]
https://git.ruby-lang.org/ruby.git/commit/?id=882d4203f1 From 882d4203f184ac93ac5c2c02c1e9f4b26cfad1db Mon Sep 17 00:00:00 2001 From: nagachika <nagachika@r...> Date: Sun, 4 Sep 2022 16:24:57 +0900 Subject: merge revision(s) ef525b012a709077ea2797e8642fae0b61234063,dc9a13abeef5a2b936fbb55edc112b8b382a05e7: [Backport #18775] Explicit handling of frozen strings in `IO::Buffer#for`. (#5892) --- io_buffer.c | 122 +++++++++++++++++++++++++++++++++++--------- test/ruby/test_io_buffer.rb | 36 +++++++------ 2 files changed, 117 insertions(+), 41 deletions(-) Fix rdoc of IO::Buffer [ci skip] --- io_buffer.c | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) --- io_buffer.c | 107 ++++++++++++++++++++++++++++++++++---------- test/ruby/test_io_buffer.rb | 36 ++++++++------- version.h | 2 +- 3 files changed, 104 insertions(+), 41 deletions(-) diff --git a/io_buffer.c b/io_buffer.c index 5227a2a5b8..0ca6ec5632 100644 --- a/io_buffer.c +++ b/io_buffer.c @@ -208,9 +208,11 @@ io_buffer_free(struct rb_io_buffer *data) https://github.com/ruby/ruby/blob/trunk/io_buffer.c#L208 io_buffer_unmap(data->base, data->size); } - if (RB_TYPE_P(data->source, T_STRING)) { - rb_str_unlocktmp(data->source); - } + // Previously we had this, but we found out due to the way GC works, we + // can't refer to any other Ruby objects here. + // if (RB_TYPE_P(data->source, T_STRING)) { + // rb_str_unlocktmp(data->source); + // } data->base = NULL; @@ -282,12 +284,65 @@ rb_io_buffer_type_allocate(VALUE self) https://github.com/ruby/ruby/blob/trunk/io_buffer.c#L284 return instance; } +static VALUE +io_buffer_for_make_instance(VALUE klass, VALUE string) +{ + VALUE instance = rb_io_buffer_type_allocate(klass); + + struct rb_io_buffer *data = NULL; + TypedData_Get_Struct(instance, struct rb_io_buffer, &rb_io_buffer_type, data); + + enum rb_io_buffer_flags flags = RB_IO_BUFFER_EXTERNAL; + + if (RB_OBJ_FROZEN(string)) + flags |= RB_IO_BUFFER_READONLY; + + io_buffer_initialize(data, RSTRING_PTR(string), RSTRING_LEN(string), flags, string); + + return instance; +} + +struct io_buffer_for_yield_instance_arguments { + VALUE klass; + VALUE string; + VALUE instance; +}; + +static VALUE +io_buffer_for_yield_instance(VALUE _arguments) { + struct io_buffer_for_yield_instance_arguments *arguments = (struct io_buffer_for_yield_instance_arguments *)_arguments; + + rb_str_locktmp(arguments->string); + + arguments->instance = io_buffer_for_make_instance(arguments->klass, arguments->string); + + return rb_yield(arguments->instance); +} + +static VALUE +io_buffer_for_yield_instance_ensure(VALUE _arguments) +{ + struct io_buffer_for_yield_instance_arguments *arguments = (struct io_buffer_for_yield_instance_arguments *)_arguments; + + if (arguments->instance != Qnil) { + rb_io_buffer_free(arguments->instance); + } + + rb_str_unlocktmp(arguments->string); + + return Qnil; +} + /* - * call-seq: IO::Buffer.for(string) -> io_buffer + * call-seq: + * IO::Buffer.for(string) -> readonly io_buffer + * IO::Buffer.for(string) {|io_buffer| ... read/write io_buffer ...} * - * Creates a IO::Buffer from the given string's memory. The buffer remains - * associated with the string, and writing to a buffer will update the string's - * contents. + * Creates a IO::Buffer from the given string's memory. Without a block a + * frozen internal copy of the string is created efficiently and used as the + * buffer source. When a block is provided, the buffer is associated directly + * with the string's internal data and updating the buffer will update the + * string. * * Until #free is invoked on the buffer, either explicitly or via the garbage * collector, the source string will be locked and cannot be modified. @@ -296,7 +351,7 @@ rb_io_buffer_type_allocate(VALUE self) https://github.com/ruby/ruby/blob/trunk/io_buffer.c#L351 * modified. * * string = 'test' - * buffer = IO::Buffer.for(str) + * buffer = IO::Buffer.for(string) * buffer.external? #=> true * * buffer.get_string(0, 1) @@ -306,29 +361,33 @@ rb_io_buffer_type_allocate(VALUE self) https://github.com/ruby/ruby/blob/trunk/io_buffer.c#L361 * * buffer.resize(100) * # in `resize': Cannot resize external buffer! (IO::Buffer::AccessError) + * + * IO::Buffer.for(string) do |buffer| + * buffer.set_string("T") + * string + * # => "Test" + * end */ VALUE rb_io_buffer_type_for(VALUE klass, VALUE string) { - io_buffer_experimental(); - StringValue(string); - VALUE instance = rb_io_buffer_type_allocate(klass); + // If the string is frozen, both code paths are okay. + // If the string is not frozen, if a block is not given, it must be frozen. + if (rb_block_given_p()) { + struct io_buffer_for_yield_instance_arguments arguments = { + .klass = klass, + .string = string, + .instance = Qnil, + }; - struct rb_io_buffer *data = NULL; - TypedData_Get_Struct(instance, struct rb_io_buffer, &rb_io_buffer_type, data); - - rb_str_locktmp(string); - - enum rb_io_buffer_flags flags = RB_IO_BUFFER_EXTERNAL; - - if (RB_OBJ_FROZEN(string)) - flags |= RB_IO_BUFFER_READONLY; - - io_buffer_initialize(data, RSTRING_PTR(string), RSTRING_LEN(string), flags, string); - - return instance; + return rb_ensure(io_buffer_for_yield_instance, (VALUE)&arguments, io_buffer_for_yield_instance_ensure, (VALUE)&arguments); + } else { + // This internally returns the source string if it's already frozen. + string = rb_str_tmp_frozen_acquire(string); + return io_buffer_for_make_instance(klass, string); + } } VALUE diff --git a/test/ruby/test_io_buffer.rb b/test/ruby/test_io_buffer.rb index 7e3b467ed5..e3f7021b26 100644 --- a/test/ruby/test_io_buffer.rb +++ b/test/ruby/test_io_buffer.rb @@ -88,30 +88,34 @@ class TestIOBuffer < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_io_buffer.rb#L88 def test_string_mapped string = "Hello World" buffer = IO::Buffer.for(string) - refute buffer.readonly? - - # Cannot modify string as it's locked by the buffer: - assert_raise RuntimeError do - string[0] = "h" - end - - buffer.set_value(:U8, 0, "h".ord) - - # Buffer releases it's ownership of the string: - buffer.free - - assert_equal "hello World", string - string[0] = "H" - assert_equal "Hello World", string + assert buffer.readonly? end def test_string_mapped_frozen string = "Hello World".freeze buffer = IO::Buffer.for(string) - assert buffer.readonly? end + def test_string_mapped_mutable + string = "Hello World" + IO::Buffer.for(string) do |buffer| + refute buffer.readonly? + + # Cannot modify string as it's locked by the buffer: + assert_raise RuntimeError do + string[0] = "h" + end + + buffer.set_value(:U8, 0, "h".ord) + + # Buffer releases it's ownership of the string: + buffer.free + + assert_equal "hello World", string + end + end + def test_non_string not_string = Object.new diff --git a/version.h b/version.h index f7dbdf5382..4d700a1885 100644 --- a/version.h +++ b/version.h @@ -11,7 +11,7 @@ https://github.com/ruby/ruby/blob/trunk/version.h#L11 # define RUBY_VERSION_MINOR RUBY_API_VERSION_MINOR #define RUBY_VERSION_TEENY 3 #define RUBY_RELEASE_DATE RUBY_RELEASE_YEAR_STR"-"RUBY_RELEASE_MONTH_STR"-"RUBY_RELEASE_DAY_STR -#define RUBY_PATCHLEVEL 46 +#define RUBY_PATCHLEVEL 47 #define RUBY_RELEASE_YEAR 2022 #define RUBY_RELEASE_MONTH 9 -- cgit v1.2.1 -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/