Bug #19584
closedCrash in rb_gc_register_address
Description
GitHub PR: https://github.com/ruby/ruby/pull/7670
Some C extensions pass a pointer to a global variable to rb_gc_register_address. However, if a GC is triggered inside of rb_gc_register_address, then the object could get swept since it does not exist on the stack.
Updated by peterzhu2118 (Peter Zhu) over 1 year ago
- Status changed from Open to Closed
Applied in changeset git|bccec7fb468ad977be75e7e4c2644b4ea845ab0c.
Fix crash in rb_gc_register_address
[Bug #19584]
Some C extensions pass a pointer to a global variable to
rb_gc_register_address. However, if a GC is triggered inside of
rb_gc_register_address, then the object could get swept since it does
not exist on the stack.
Updated by nobu (Nobuyoshi Nakada) over 1 year ago
- Status changed from Closed to Open
It sounds a bug of such extension library.
rb_gc_register_address
must be called before assigning any GC-able object to that variable.
Updated by nobu (Nobuyoshi Nakada) over 1 year ago
- Backport changed from 3.0: REQUIRED, 3.1: REQUIRED, 3.2: REQUIRED to 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN
Updated by peterzhu2118 (Peter Zhu) over 1 year ago
Well, unfortunately it looks like that requirement is not being followed by gems and even in parts of the Ruby codebase.
Ruby:
- https://github.com/ruby/ruby/blob/fb822076d79339427648cb9eacf76528f827427e/string.c#L12064
- https://github.com/ruby/ruby/blob/fb822076d79339427648cb9eacf76528f827427e/ruby.c#L645
Gems:
- https://github.com/google/mysql-protobuf/blob/467cda676afaa49e762c5c9164a43f6ad31a1fbf/protobuf/ruby/ext/google/protobuf_c/defs.c#L130
- https://github.com/ged/ruby-pg/blob/a99322a7111064aec6e430abb8787fa10fce37dd/ext/pg_type_map_all_strings.c#L129
- https://github.com/rgeo/rgeo/blob/3358e982d277965cfa039fa06a73dd5e2ddcdbdc/ext/geos_c_impl/factory.c#L629
Updated by nobu (Nobuyoshi Nakada) over 1 year ago
Thanks.
Since Qnil
is never GC-ed, rb_fs
and rgeo cases are OK.
Other 3 cases are, even it would be very rare, can cause a crash.
Updated by nobu (Nobuyoshi Nakada) over 1 year ago
- Status changed from Open to Closed
Applied in changeset git|5f8ebcada099351acbc22db264e7cd3773c2bdc4.
[Bug #19584] Register global variable address before assignment
Updated by peterzhu2118 (Peter Zhu) over 1 year ago
- Backport changed from 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN to 3.0: REQUIRED, 3.1: REQUIRED, 3.2: REQUIRED
Updated by Eregon (Benoit Daloze) over 1 year ago
FYI this function is quite difficult to implement on TruffleRuby/other Rubies with the semantics of being called before the assignment.
If it's done during Init_myextension
, we can simulate it, but if it's not we have to read the value immediately for TruffleRuby: https://github.com/oracle/truffleruby/issues/2721
There is no way to hook into the GC to make it read such variables during GC (on JVM at least).
This requirement seems documented but one need to read it very carefully to notice it:
https://github.com/ruby/ruby/blob/671cfc20000db024f2aeaf602b1a77895c819abc/include/ruby/internal/gc.h#L401-L412
Updated by peterzhu2118 (Peter Zhu) over 1 year ago
This requirement seems documented but one need to read it very carefully to notice it
The documentation was changed a few days ago in 4adcfc8cd7a17593a6590025da2b03eebf4fd63c. The old documentation did not mention this requirement at all.
Inform the garbage collector that
valptr
points to a live Ruby object that
should not be moved. Note that extensions should use this API on global
constants instead of assuming constants defined in Ruby are always alive.
Ruby code can remove global constants.