Ruby » Ruby master

Bug #20998

Updated by Eregon (Benoit Daloze) 5 days ago

```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. so. 
 Any attempt to mutate a frozen string should fail, so might as well fail early.

Back