[ruby-core:120465] [Ruby master Bug#20998] rb_str_locktmp() changes flags of frozen strings and string literals

[ruby-core:120465] [Ruby master Bug#20998] rb_str_locktmp() changes flags of frozen strings and string literals

[Eregon (Benoit Daloze) via ruby-core]

Issue #20998 has been reported by Eregon (Benoit Daloze).

----------------------------------------
Bug #20998: rb_str_locktmp() changes flags of frozen strings and string literals
https://bugs.ruby-lang.org/issues/20998

* 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"

# imagine a million line of codes in between

b = "str"

Foo.foo(a)

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 line of codes in between

Foo.foo("abc") # emporal 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.
Any attempt to mutate a frozen string should fail, so might as well fail early.



-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:120493] [Ruby master Bug#20998] rb_str_locktmp() changes flags of frozen strings and string literals

[nobu (Nobuyoshi Nakada) via ruby-core]

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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:120495] [Ruby master Bug#20998] rb_str_locktmp() changes flags of frozen strings and string literals

[Eregon (Benoit Daloze) via ruby-core]

Issue #20998 has been updated by Eregon (Benoit Daloze).


I'm not aware of any such case, and `.freeze` seems much better for that use case.

----------------------------------------
Bug #20998: rb_str_locktmp() changes flags of frozen strings and string literals
https://bugs.ruby-lang.org/issues/20998#change-111282

* 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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:120497] [Ruby master Bug#20998] rb_str_locktmp() changes flags of frozen strings and string literals

[byroot (Jean Boussier) via ruby-core]

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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:120499] [Ruby master Bug#20998] rb_str_locktmp() changes flags of frozen strings and string literals

[Eregon (Benoit Daloze) via ruby-core]

Issue #20998 has been updated by Eregon (Benoit Daloze).


@byroot but that's mutating the string, @nobu is asking for a case not mutating the string, which I think there is none.

----------------------------------------
Bug #20998: rb_str_locktmp() changes flags of frozen strings and string literals
https://bugs.ruby-lang.org/issues/20998#change-111292

* 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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:120500] [Ruby master Bug#20998] rb_str_locktmp() changes flags of frozen strings and string literals

[Eregon (Benoit Daloze) via ruby-core]

Issue #20998 has been updated by Eregon (Benoit Daloze).


https://github.com/ruby/ruby/blob/975461bf885b8fb791cf048ad39c79924d28e4e9/ext/zlib/zlib.c#L1056 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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:120520] [Ruby master Bug#20998] rb_str_locktmp() changes flags of frozen strings and string literals

[byroot (Jean Boussier) via ruby-core]

Issue #20998 has been updated by byroot (Jean Boussier).


> 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.

That I totally agree.


----------------------------------------
Bug #20998: rb_str_locktmp() changes flags of frozen strings and string literals
https://bugs.ruby-lang.org/issues/20998#change-111325

* 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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:120583] [Ruby master Bug#20998] rb_str_locktmp() changes flags of frozen strings and string literals

[Eregon (Benoit Daloze) via ruby-core]

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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:120585] [Ruby master Bug#20998] rb_str_locktmp() changes flags of frozen strings and string literals

[byroot (Jean Boussier) via ruby-core]

Issue #20998 has been updated by byroot (Jean Boussier).


I prefer the noop solution, as it makes it a much easier to use API. But no strong opinion.

----------------------------------------
Bug #20998: rb_str_locktmp() changes flags of frozen strings and string literals
https://bugs.ruby-lang.org/issues/20998#change-111410

* 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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:120981] [Ruby master Bug#20998] rb_str_locktmp() changes flags of frozen strings and string literals

[mame (Yusuke Endoh) via ruby-core]

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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:122142] [Ruby Bug#20998] rb_str_locktmp() changes flags of frozen strings and string literals

[Eregon (Benoit Daloze) via ruby-core]

Issue #20998 has been updated by Eregon (Benoit Daloze).


mame (Yusuke Endoh) wrote in #note-13:
> Unless there is a valid use case for that, it would be good to raise an Exception, instead of no-op.

FYI TruffleRuby implemented FrozenError when `rb_str_locktmp()` is used on a frozen String in https://github.com/oracle/truffleruby/pull/3867.

It would be great if CRuby can do the same for 3.5.

----------------------------------------
Bug #20998: rb_str_locktmp() changes flags of frozen strings and string literals
https://bugs.ruby-lang.org/issues/20998#change-113293

* 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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:122532] [Ruby Bug#20998] rb_str_locktmp() changes flags of frozen strings and string literals

[Eregon (Benoit Daloze) via ruby-core]

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/
______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:122535] [Ruby Bug#20998] rb_str_locktmp() changes flags of frozen strings and string literals

[ioquatix (Samuel Williams) via ruby-core]

Issue #20998 has been updated by ioquatix (Samuel Williams).


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?

----------------------------------------
Bug #20998: rb_str_locktmp() changes flags of frozen strings and string literals
https://bugs.ruby-lang.org/issues/20998#change-113761

* 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/
______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:122541] [Ruby Bug#20998] rb_str_locktmp() changes flags of frozen strings and string literals

[Eregon (Benoit Daloze) via ruby-core]

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/
______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:122542] [Ruby Bug#20998] rb_str_locktmp() changes flags of frozen strings and string literals

[hanazuki (Kasumi Hanazuki) via ruby-core]

Issue #20998 has been updated by hanazuki (Kasumi Hanazuki).


FYI: IO::Buffer is now broken (by a recent change) because the embedded Strings are not pinned, and I'm proposing a fix at #21210

----------------------------------------
Bug #20998: rb_str_locktmp() changes flags of frozen strings and string literals
https://bugs.ruby-lang.org/issues/20998#change-113772

* 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/
______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:122544] [Ruby Bug#20998] rb_str_locktmp() changes flags of frozen strings and string literals

[Eregon (Benoit Daloze) via ruby-core]

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/
______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:122547] Re: [Ruby Bug#20998] rb_str_locktmp() changes flags of frozen strings and string literals

[Kasumi Hanazuki via ruby-core]

[-- Attachment #1.1: Type: text/plain, Size: 353 bytes --]

On Tue, 17 Jun 2025, 16:16 Eregon (Benoit Daloze) via ruby-core, <
ruby-core@ml.ruby-lang.org> wrote:

>
> hanazuki (Kasumi Hanazuki) wrote in #note-20:
> > FYI: IO::Buffer is now broken (by a recent change)
>
> Which change do you mean? 6012145299cfa4ab561360c78710c7f2941a7e9d ?
>

Yes. the commit "Mark rb_io_buffer_type references declaratively"

>

[-- Attachment #1.2: Type: text/html, Size: 916 bytes --]

[-- Attachment #2: Type: text/plain, Size: 254 bytes --]

______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/lists/ruby-core.ml.ruby-lang.org/