Bug #21115: Etc.getgrgid is not Ractor-safe but is marked as such

• Status: Open
• Assignee: -
• Target version: -
• 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

[Eregon (Benoit Daloze)]

require 'etc'

20.times.map do
  Ractor.new do
    1000.times do
      raise unless Etc.getgrgid(Process.gid).gid == Process.gid
    end
  end
end.each(&:take)

Running it a few times gives a segfault:

$ ruby ractor_getgrgid.rb
ractor_getgrgid.rb:4: warning: Ractor is experimental, and the behavior may change in future versions of Ruby! Also there are many implementation issues.
#<Thread:0x00007f251b7d0478 run> terminated with exception (report_on_exception is true):
ractor_getgrgid.rb:6:in 'block (3 levels) in <main>': unhandled exception
	from <internal:numeric>:257:in 'Integer#times'
	from ractor_getgrgid.rb:5:in 'block (2 levels) in <main>'
#<Thread:0x00007f251b7dde70 run> terminated with exception (report_on_exception is true):
ractor_getgrgid.rb:6:in 'block (3 levels) in <main>': unhandled exception
	from <internal:numeric>:257:in 'Integer#times'
	from ractor_getgrgid.rb:5:in 'block (2 levels) in <main>'
ractor_getgrgid.rb:6: [BUG] Segmentation fault at 0x00000000706d7564
ruby 3.4.1 (2024-12-25 revision 48d4efcb85) +PRISM [x86_64-linux]

-- Control frame information -----------------------------------------------
c:0005 p:---- s:0019 e:000018 CFUNC  :getgrgid
c:0004 p:0009 s:0014 e:000013 BLOCK  ractor_getgrgid.rb:6
c:0003 p:0024 s:0011 e:000010 METHOD <internal:numeric>:257
c:0002 p:0005 s:0006 e:000005 BLOCK  ractor_getgrgid.rb:5 [FINISH]
c:0001 p:---- s:0003 e:000002 DUMMY  [FINISH]

Etc is marked as Ractor-safe since https://github.com/ruby/ruby/pull/3954.
But that is not correct, etc.c uses many C functions which are not thread-safe, such as getgrgid().

[Eregon (Benoit Daloze)]

I think most methods of Etc have the same problem, so it's probably best to mark none of them as thread-safe, or only some known as thread-safe and frequently-used like Etc.nprocessor and Etc.uname (but should still be reviewed if they are indeed thread-safe).

[nobu (Nobuyoshi Nakada)]

MT-Safe variants such as getgrgid_r may be available, but we don't provide per-method ractor-safe declaration API, for now.
Also ractor_unsafe_check() is not exposed.

[Eregon (Benoit Daloze)]

nobu (Nobuyoshi Nakada) wrote in #note-2:

MT-Safe variants such as getgrgid_r may be available, but we don't provide per-method ractor-safe declaration API, for now.

I think it's possible, like:

void Init_foo() {
    rb_ext_ractor_safe(true);

    #ifdef HAVE_GETGRGID_R
    rb_define_method(cls, "foo", ractor_safe_function, 0);
    #else
    rb_ext_ractor_safe(false);
    rb_define_method(cls, "foo", ractor_unsafe_function, 0);
    rb_ext_ractor_safe(true); // restore
    #endif
}

or

void Init_foo() {
    rb_ext_ractor_safe(false);

    #ifdef HAVE_GETGRGID_R
    rb_ext_ractor_safe(true);
    rb_define_method(cls, "foo", ractor_safe_function, 0);
    rb_ext_ractor_safe(false); // restore
    #else
    rb_define_method(cls, "foo", ractor_unsafe_function, 0);
    #endif
}

But I am not sure it's worth the effort to use getgrgid_r and other _r methods (which are often difficult to use, e.g. to properly size the buffer).
I think we should mark Etc.getgrgid and other Etc methods as always Ractor-unsafe.
IOW, use rb_ext_ractor_safe(true);/rb_ext_ractor_safe(false); around actually Ractor-safe Etc methods, and let the rest be Ractor-unsafe.

Alternatively Etc could lock explicitly around these thread-unsafe library calls.

[luke-gru (Luke Gruber)]

I think having a mutex per unsafe C extension (that you want to make safe) to lock around those library calls would be a good idea. The alternative would
be confusing to users imo, the ractor safety would be system dependant (OS version dependent and stdlib version dependent).