Issue #20998 has been updated by nobu (Nobuyoshi Nakada). There isn’t a case that locking a string just to avoid being modified or moved, but not to mutate it? ---------------------------------------- Bug #20998: rb_str_locktmp() changes flags of frozen strings and string literals https://bugs.ruby-lang.org/issues/20998#change-111278 * Author: Eregon (Benoit Daloze) * Status: Open * ruby -v: ruby 3.4.1 (2024-12-25 revision 48d4efcb85) +PRISM [x86_64-linux] * Backport: 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN, 3.4: UNKNOWN ---------------------------------------- ```ruby # frozen_string_literal: true # BOILERPLATE START require 'tmpdir' require 'rbconfig' def inline_c_extension(c_code) Dir.mktmpdir('inline_c_extension') do |dir| File.write("#{dir}/cext.c", c_code) File.write("#{dir}/extconf.rb", <<~RUBY) require 'mkmf' create_makefile('cext') RUBY out = IO.popen([RbConfig.ruby, 'extconf.rb'], chdir: dir, &:read) raise "ruby extconf.rb failed: #{$?.inspect}\n#{out}" unless $?.success? out = IO.popen(['make'], chdir: dir, &:read) raise "make failed: #{$?.inspect}\n#{out}" unless $?.success? require "#{dir}/cext.#{RbConfig::CONFIG['DLEXT']}" end end inline_c_extension <<~C #include "ruby.h" static VALUE foo(VALUE self, VALUE str) { rb_str_locktmp(str); return str; } void Init_cext(void) { VALUE c = rb_define_class("Foo", rb_cObject); rb_define_singleton_method(c, "foo", foo, 1); } C # BOILERPLATE END a = "str" Foo.foo(a) # imagine a million lines of code in between b = "str" b << "." # can't modify string; temporarily locked (RuntimeError) # What? Who "locked" that immutable frozen string literal? # It should be: can't modify frozen String: "str" (FrozenError) ``` Same problem with: ```ruby Foo.foo("abc") # imagine a million lines of code in between Foo.foo("abc") # temporal locking already locked string (RuntimeError) ``` Related: https://github.com/oracle/truffleruby/issues/3752 It seems a clear bug to mutate a frozen string (with visible side effects), even more so for shared frozen string literals. I think rb_str_locktmp() should raise (a FrozenError, as it's effectively attempting to mutate it, same as calling `rb_str_modify()`) if called on a frozen string, because it makes little sense, I think "locking a string" only makes sense for mutable strings. The alternative would be to noop on frozen strings for rb_str_locktmp() and rb_str_unlocktmp(), I think that's susprising, and potentially hiding more bugs, e.g. if one by mistake tries to mutate the RSTRING_PTR() or so, and also rb_str_locktmp() wouldn't imply the flag is set after it returns. Any attempt to mutate a frozen string should fail, so might as well fail early. -- https://bugs.ruby-lang.org/

Issue #20998 has been updated by byroot (Jean Boussier). Actually yes, I think it's not an uncommon use case? e.g. if you need to write a given string into a socket or something like that, you may want to lock it to ensure a concurrent thread won't modify it. I think `zlic.c` uses it this way? ---------------------------------------- Bug #20998: rb_str_locktmp() changes flags of frozen strings and string literals https://bugs.ruby-lang.org/issues/20998#change-111289 * Author: Eregon (Benoit Daloze) * Status: Open * ruby -v: ruby 3.4.1 (2024-12-25 revision 48d4efcb85) +PRISM [x86_64-linux] * Backport: 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN, 3.4: UNKNOWN ---------------------------------------- ```ruby # frozen_string_literal: true # BOILERPLATE START require 'tmpdir' require 'rbconfig' def inline_c_extension(c_code) Dir.mktmpdir('inline_c_extension') do |dir| File.write("#{dir}/cext.c", c_code) File.write("#{dir}/extconf.rb", <<~RUBY) require 'mkmf' create_makefile('cext') RUBY out = IO.popen([RbConfig.ruby, 'extconf.rb'], chdir: dir, &:read) raise "ruby extconf.rb failed: #{$?.inspect}\n#{out}" unless $?.success? out = IO.popen(['make'], chdir: dir, &:read) raise "make failed: #{$?.inspect}\n#{out}" unless $?.success? require "#{dir}/cext.#{RbConfig::CONFIG['DLEXT']}" end end inline_c_extension <<~C #include "ruby.h" static VALUE foo(VALUE self, VALUE str) { rb_str_locktmp(str); return str; } void Init_cext(void) { VALUE c = rb_define_class("Foo", rb_cObject); rb_define_singleton_method(c, "foo", foo, 1); } C # BOILERPLATE END a = "str" Foo.foo(a) # imagine a million lines of code in between b = "str" b << "." # can't modify string; temporarily locked (RuntimeError) # What? Who "locked" that immutable frozen string literal? # It should be: can't modify frozen String: "str" (FrozenError) ``` Same problem with: ```ruby Foo.foo("abc") # imagine a million lines of code in between Foo.foo("abc") # temporal locking already locked string (RuntimeError) ``` Related: https://github.com/oracle/truffleruby/issues/3752 It seems a clear bug to mutate a frozen string (with visible side effects), even more so for shared frozen string literals. I think rb_str_locktmp() should raise (a FrozenError, as it's effectively attempting to mutate it, same as calling `rb_str_modify()`) if called on a frozen string, because it makes little sense, I think "locking a string" only makes sense for mutable strings. The alternative would be to noop on frozen strings for rb_str_locktmp() and rb_str_unlocktmp(), I think that's susprising, and potentially hiding more bugs, e.g. if one by mistake tries to mutate the RSTRING_PTR() or so, and also rb_str_locktmp() wouldn't imply the flag is set after it returns. Any attempt to mutate a frozen string should fail, so might as well fail early. -- https://bugs.ruby-lang.org/

Issue #20998 has been updated by Eregon (Benoit Daloze). https://github.com/ruby/ruby/blob/975461bf885b8fb791cf048ad39c79924d28e4e9/e... but I'm not quite sure how that String is used. It seems clearly wrong that two threads using Zlib with a frozen String literal (which happens to have the same contents/characters for both threads) would fail with `temporal locking already locked string (RuntimeError)` though. So what should we do then when rb_str_locktmp() is called on a frozen string? ---------------------------------------- Bug #20998: rb_str_locktmp() changes flags of frozen strings and string literals https://bugs.ruby-lang.org/issues/20998#change-111293 * Author: Eregon (Benoit Daloze) * Status: Open * ruby -v: ruby 3.4.1 (2024-12-25 revision 48d4efcb85) +PRISM [x86_64-linux] * Backport: 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN, 3.4: UNKNOWN ---------------------------------------- ```ruby # frozen_string_literal: true # BOILERPLATE START require 'tmpdir' require 'rbconfig' def inline_c_extension(c_code) Dir.mktmpdir('inline_c_extension') do |dir| File.write("#{dir}/cext.c", c_code) File.write("#{dir}/extconf.rb", <<~RUBY) require 'mkmf' create_makefile('cext') RUBY out = IO.popen([RbConfig.ruby, 'extconf.rb'], chdir: dir, &:read) raise "ruby extconf.rb failed: #{$?.inspect}\n#{out}" unless $?.success? out = IO.popen(['make'], chdir: dir, &:read) raise "make failed: #{$?.inspect}\n#{out}" unless $?.success? require "#{dir}/cext.#{RbConfig::CONFIG['DLEXT']}" end end inline_c_extension <<~C #include "ruby.h" static VALUE foo(VALUE self, VALUE str) { rb_str_locktmp(str); return str; } void Init_cext(void) { VALUE c = rb_define_class("Foo", rb_cObject); rb_define_singleton_method(c, "foo", foo, 1); } C # BOILERPLATE END a = "str" Foo.foo(a) # imagine a million lines of code in between b = "str" b << "." # can't modify string; temporarily locked (RuntimeError) # What? Who "locked" that immutable frozen string literal? # It should be: can't modify frozen String: "str" (FrozenError) ``` Same problem with: ```ruby Foo.foo("abc") # imagine a million lines of code in between Foo.foo("abc") # temporal locking already locked string (RuntimeError) ``` Related: https://github.com/oracle/truffleruby/issues/3752 It seems a clear bug to mutate a frozen string (with visible side effects), even more so for shared frozen string literals. I think rb_str_locktmp() should raise (a FrozenError, as it's effectively attempting to mutate it, same as calling `rb_str_modify()`) if called on a frozen string, because it makes little sense, I think "locking a string" only makes sense for mutable strings. The alternative would be to noop on frozen strings for rb_str_locktmp() and rb_str_unlocktmp(), I think that's susprising, and potentially hiding more bugs, e.g. if one by mistake tries to mutate the RSTRING_PTR() or so, and also rb_str_locktmp() wouldn't imply the flag is set after it returns. Any attempt to mutate a frozen string should fail, so might as well fail early. -- https://bugs.ruby-lang.org/

Issue #20998 has been updated by Eregon (Benoit Daloze). @nobu @byroot So do you think rb_str_locktmp() should noop on frozen strings (since they can't change anyway), or it should raise because the usage seems somewhat dubious? I'm fine with either, I'd just like a decision so we can do the same in TruffleRuby. (the current status is broken so that's worse than either fix) ---------------------------------------- Bug #20998: rb_str_locktmp() changes flags of frozen strings and string literals https://bugs.ruby-lang.org/issues/20998#change-111408 * Author: Eregon (Benoit Daloze) * Status: Open * ruby -v: ruby 3.4.1 (2024-12-25 revision 48d4efcb85) +PRISM [x86_64-linux] * Backport: 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN, 3.4: UNKNOWN ---------------------------------------- ```ruby # frozen_string_literal: true # BOILERPLATE START require 'tmpdir' require 'rbconfig' def inline_c_extension(c_code) Dir.mktmpdir('inline_c_extension') do |dir| File.write("#{dir}/cext.c", c_code) File.write("#{dir}/extconf.rb", <<~RUBY) require 'mkmf' create_makefile('cext') RUBY out = IO.popen([RbConfig.ruby, 'extconf.rb'], chdir: dir, &:read) raise "ruby extconf.rb failed: #{$?.inspect}\n#{out}" unless $?.success? out = IO.popen(['make'], chdir: dir, &:read) raise "make failed: #{$?.inspect}\n#{out}" unless $?.success? require "#{dir}/cext.#{RbConfig::CONFIG['DLEXT']}" end end inline_c_extension <<~C #include "ruby.h" static VALUE foo(VALUE self, VALUE str) { rb_str_locktmp(str); return str; } void Init_cext(void) { VALUE c = rb_define_class("Foo", rb_cObject); rb_define_singleton_method(c, "foo", foo, 1); } C # BOILERPLATE END a = "str" Foo.foo(a) # imagine a million lines of code in between b = "str" b << "." # can't modify string; temporarily locked (RuntimeError) # What? Who "locked" that immutable frozen string literal? # It should be: can't modify frozen String: "str" (FrozenError) ``` Same problem with: ```ruby Foo.foo("abc") # imagine a million lines of code in between Foo.foo("abc") # temporal locking already locked string (RuntimeError) ``` Related: https://github.com/oracle/truffleruby/issues/3752 It seems a clear bug to mutate a frozen string (with visible side effects), even more so for shared frozen string literals. I think rb_str_locktmp() should raise (a FrozenError, as it's effectively attempting to mutate it, same as calling `rb_str_modify()`) if called on a frozen string, because it makes little sense, I think "locking a string" only makes sense for mutable strings. The alternative would be to noop on frozen strings for rb_str_locktmp() and rb_str_unlocktmp(), I think that's susprising, and potentially hiding more bugs, e.g. if one by mistake tries to mutate the RSTRING_PTR() or so, and also rb_str_locktmp() wouldn't imply the flag is set after it returns. Any attempt to mutate a frozen string should fail, so might as well fail early. -- https://bugs.ruby-lang.org/

Issue #20998 has been updated by mame (Yusuke Endoh). Discussed at the dev meeting. @akr preferred raising an Exception. The current purpose of `rb_str_locktmp` is to prevent the `RSTRING_PTR` from being moved (by realloc, compaction, etc.) when passing the pointer to a C function that blocks and writes to the buffer (typically, read(2) syscall). In other words, `rb_str_locktmp` should be performed on a String that is about to be written. We couldn't find a use case for rb_str_locktmp on a frozen String. Unless there is a valid use case for that, it would be good to raise an Exception, instead of no-op. ---------------------------------------- Bug #20998: rb_str_locktmp() changes flags of frozen strings and string literals https://bugs.ruby-lang.org/issues/20998#change-111883 * Author: Eregon (Benoit Daloze) * Status: Open * ruby -v: ruby 3.4.1 (2024-12-25 revision 48d4efcb85) +PRISM [x86_64-linux] * Backport: 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN, 3.4: UNKNOWN ---------------------------------------- ```ruby # frozen_string_literal: true # BOILERPLATE START require 'tmpdir' require 'rbconfig' def inline_c_extension(c_code) Dir.mktmpdir('inline_c_extension') do |dir| File.write("#{dir}/cext.c", c_code) File.write("#{dir}/extconf.rb", <<~RUBY) require 'mkmf' create_makefile('cext') RUBY out = IO.popen([RbConfig.ruby, 'extconf.rb'], chdir: dir, &:read) raise "ruby extconf.rb failed: #{$?.inspect}\n#{out}" unless $?.success? out = IO.popen(['make'], chdir: dir, &:read) raise "make failed: #{$?.inspect}\n#{out}" unless $?.success? require "#{dir}/cext.#{RbConfig::CONFIG['DLEXT']}" end end inline_c_extension <<~C #include "ruby.h" static VALUE foo(VALUE self, VALUE str) { rb_str_locktmp(str); return str; } void Init_cext(void) { VALUE c = rb_define_class("Foo", rb_cObject); rb_define_singleton_method(c, "foo", foo, 1); } C # BOILERPLATE END a = "str" Foo.foo(a) # imagine a million lines of code in between b = "str" b << "." # can't modify string; temporarily locked (RuntimeError) # What? Who "locked" that immutable frozen string literal? # It should be: can't modify frozen String: "str" (FrozenError) ``` Same problem with: ```ruby Foo.foo("abc") # imagine a million lines of code in between Foo.foo("abc") # temporal locking already locked string (RuntimeError) ``` Related: https://github.com/oracle/truffleruby/issues/3752 It seems a clear bug to mutate a frozen string (with visible side effects), even more so for shared frozen string literals. I think rb_str_locktmp() should raise (a FrozenError, as it's effectively attempting to mutate it, same as calling `rb_str_modify()`) if called on a frozen string, because it makes little sense, I think "locking a string" only makes sense for mutable strings. The alternative would be to noop on frozen strings for rb_str_locktmp() and rb_str_unlocktmp(), I think that's susprising, and potentially hiding more bugs, e.g. if one by mistake tries to mutate the RSTRING_PTR() or so, and also rb_str_locktmp() wouldn't imply the flag is set after it returns. Any attempt to mutate a frozen string should fail, so might as well fail early. -- https://bugs.ruby-lang.org/

Issue #20998 has been updated by Eregon (Benoit Daloze). Assignee set to Eregon (Benoit Daloze) PR at https://github.com/ruby/ruby/pull/13615 ---------------------------------------- Bug #20998: rb_str_locktmp() changes flags of frozen strings and string literals https://bugs.ruby-lang.org/issues/20998#change-113758 * Author: Eregon (Benoit Daloze) * Status: Open * Assignee: Eregon (Benoit Daloze) * ruby -v: ruby 3.4.1 (2024-12-25 revision 48d4efcb85) +PRISM [x86_64-linux] * Backport: 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN, 3.4: UNKNOWN ---------------------------------------- ```ruby # frozen_string_literal: true # BOILERPLATE START require 'tmpdir' require 'rbconfig' def inline_c_extension(c_code) Dir.mktmpdir('inline_c_extension') do |dir| File.write("#{dir}/cext.c", c_code) File.write("#{dir}/extconf.rb", <<~RUBY) require 'mkmf' create_makefile('cext') RUBY out = IO.popen([RbConfig.ruby, 'extconf.rb'], chdir: dir, &:read) raise "ruby extconf.rb failed: #{$?.inspect}\n#{out}" unless $?.success? out = IO.popen(['make'], chdir: dir, &:read) raise "make failed: #{$?.inspect}\n#{out}" unless $?.success? require "#{dir}/cext.#{RbConfig::CONFIG['DLEXT']}" end end inline_c_extension <<~C #include "ruby.h" static VALUE foo(VALUE self, VALUE str) { rb_str_locktmp(str); return str; } void Init_cext(void) { VALUE c = rb_define_class("Foo", rb_cObject); rb_define_singleton_method(c, "foo", foo, 1); } C # BOILERPLATE END a = "str" Foo.foo(a) # imagine a million lines of code in between b = "str" b << "." # can't modify string; temporarily locked (RuntimeError) # What? Who "locked" that immutable frozen string literal? # It should be: can't modify frozen String: "str" (FrozenError) ``` Same problem with: ```ruby Foo.foo("abc") # imagine a million lines of code in between Foo.foo("abc") # temporal locking already locked string (RuntimeError) ``` Related: https://github.com/oracle/truffleruby/issues/3752 It seems a clear bug to mutate a frozen string (with visible side effects), even more so for shared frozen string literals. I think rb_str_locktmp() should raise (a FrozenError, as it's effectively attempting to mutate it, same as calling `rb_str_modify()`) if called on a frozen string, because it makes little sense, I think "locking a string" only makes sense for mutable strings. The alternative would be to noop on frozen strings for rb_str_locktmp() and rb_str_unlocktmp(), I think that's susprising, and potentially hiding more bugs, e.g. if one by mistake tries to mutate the RSTRING_PTR() or so, and also rb_str_locktmp() wouldn't imply the flag is set after it returns. Any attempt to mutate a frozen string should fail, so might as well fail early. -- https://bugs.ruby-lang.org/

Issue #20998 has been updated by Eregon (Benoit Daloze). ioquatix (Samuel Williams) wrote in #note-16:

My usage of `rb_str_locktmp` isn't about mutation, it's about making sure the pointer doesn't change. Could compacting GC cause embedded frozen strings to be moved?

rb_str_locktmp() doesn't prevent GC compaction to move the String, more details about that on the PR. ---------------------------------------- Bug #20998: rb_str_locktmp() changes flags of frozen strings and string literals https://bugs.ruby-lang.org/issues/20998#change-113770 * Author: Eregon (Benoit Daloze) * Status: Closed * Assignee: Eregon (Benoit Daloze) * ruby -v: ruby 3.4.1 (2024-12-25 revision 48d4efcb85) +PRISM [x86_64-linux] * Backport: 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN, 3.4: UNKNOWN ---------------------------------------- ```ruby # frozen_string_literal: true # BOILERPLATE START require 'tmpdir' require 'rbconfig' def inline_c_extension(c_code) Dir.mktmpdir('inline_c_extension') do |dir| File.write("#{dir}/cext.c", c_code) File.write("#{dir}/extconf.rb", <<~RUBY) require 'mkmf' create_makefile('cext') RUBY out = IO.popen([RbConfig.ruby, 'extconf.rb'], chdir: dir, &:read) raise "ruby extconf.rb failed: #{$?.inspect}\n#{out}" unless $?.success? out = IO.popen(['make'], chdir: dir, &:read) raise "make failed: #{$?.inspect}\n#{out}" unless $?.success? require "#{dir}/cext.#{RbConfig::CONFIG['DLEXT']}" end end inline_c_extension <<~C #include "ruby.h" static VALUE foo(VALUE self, VALUE str) { rb_str_locktmp(str); return str; } void Init_cext(void) { VALUE c = rb_define_class("Foo", rb_cObject); rb_define_singleton_method(c, "foo", foo, 1); } C # BOILERPLATE END a = "str" Foo.foo(a) # imagine a million lines of code in between b = "str" b << "." # can't modify string; temporarily locked (RuntimeError) # What? Who "locked" that immutable frozen string literal? # It should be: can't modify frozen String: "str" (FrozenError) ``` Same problem with: ```ruby Foo.foo("abc") # imagine a million lines of code in between Foo.foo("abc") # temporal locking already locked string (RuntimeError) ``` Related: https://github.com/oracle/truffleruby/issues/3752 It seems a clear bug to mutate a frozen string (with visible side effects), even more so for shared frozen string literals. I think rb_str_locktmp() should raise (a FrozenError, as it's effectively attempting to mutate it, same as calling `rb_str_modify()`) if called on a frozen string, because it makes little sense, I think "locking a string" only makes sense for mutable strings. The alternative would be to noop on frozen strings for rb_str_locktmp() and rb_str_unlocktmp(), I think that's susprising, and potentially hiding more bugs, e.g. if one by mistake tries to mutate the RSTRING_PTR() or so, and also rb_str_locktmp() wouldn't imply the flag is set after it returns. Any attempt to mutate a frozen string should fail, so might as well fail early. -- https://bugs.ruby-lang.org/

Issue #20998 has been updated by Eregon (Benoit Daloze). @hanazuki Thanks for the link, I wish I found that earlier. It confirms that it was already broken which is the same conclusion I came to. hanazuki (Kasumi Hanazuki) wrote in #note-20:

FYI: IO::Buffer is now broken (by a recent change)

Which change do you mean? 6012145299cfa4ab561360c78710c7f2941a7e9d ? ---------------------------------------- Bug #20998: rb_str_locktmp() changes flags of frozen strings and string literals https://bugs.ruby-lang.org/issues/20998#change-113775 * Author: Eregon (Benoit Daloze) * Status: Closed * Assignee: Eregon (Benoit Daloze) * Target version: 3.5 * ruby -v: ruby 3.4.1 (2024-12-25 revision 48d4efcb85) +PRISM [x86_64-linux] * Backport: 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN, 3.4: UNKNOWN ---------------------------------------- ```ruby # frozen_string_literal: true # BOILERPLATE START require 'tmpdir' require 'rbconfig' def inline_c_extension(c_code) Dir.mktmpdir('inline_c_extension') do |dir| File.write("#{dir}/cext.c", c_code) File.write("#{dir}/extconf.rb", <<~RUBY) require 'mkmf' create_makefile('cext') RUBY out = IO.popen([RbConfig.ruby, 'extconf.rb'], chdir: dir, &:read) raise "ruby extconf.rb failed: #{$?.inspect}\n#{out}" unless $?.success? out = IO.popen(['make'], chdir: dir, &:read) raise "make failed: #{$?.inspect}\n#{out}" unless $?.success? require "#{dir}/cext.#{RbConfig::CONFIG['DLEXT']}" end end inline_c_extension <<~C #include "ruby.h" static VALUE foo(VALUE self, VALUE str) { rb_str_locktmp(str); return str; } void Init_cext(void) { VALUE c = rb_define_class("Foo", rb_cObject); rb_define_singleton_method(c, "foo", foo, 1); } C # BOILERPLATE END a = "str" Foo.foo(a) # imagine a million lines of code in between b = "str" b << "." # can't modify string; temporarily locked (RuntimeError) # What? Who "locked" that immutable frozen string literal? # It should be: can't modify frozen String: "str" (FrozenError) ``` Same problem with: ```ruby Foo.foo("abc") # imagine a million lines of code in between Foo.foo("abc") # temporal locking already locked string (RuntimeError) ``` Related: https://github.com/oracle/truffleruby/issues/3752 It seems a clear bug to mutate a frozen string (with visible side effects), even more so for shared frozen string literals. I think rb_str_locktmp() should raise (a FrozenError, as it's effectively attempting to mutate it, same as calling `rb_str_modify()`) if called on a frozen string, because it makes little sense, I think "locking a string" only makes sense for mutable strings. The alternative would be to noop on frozen strings for rb_str_locktmp() and rb_str_unlocktmp(), I think that's susprising, and potentially hiding more bugs, e.g. if one by mistake tries to mutate the RSTRING_PTR() or so, and also rb_str_locktmp() wouldn't imply the flag is set after it returns. Any attempt to mutate a frozen string should fail, so might as well fail early. -- https://bugs.ruby-lang.org/