Bug #19584: Crash in rb_gc_register_address

• Status: Closed
• Assignee: -
• Target version: -
• Backport: 3.0: REQUIRED, 3.1: REQUIRED, 3.2: REQUIRED

[peterzhu2118 (Peter Zhu)]

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.

[peterzhu2118 (Peter Zhu)]

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

[nobu (Nobuyoshi Nakada)]

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

[nobu (Nobuyoshi Nakada)]

• Backport changed from 3.0: REQUIRED, 3.1: REQUIRED, 3.2: REQUIRED to 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN

[peterzhu2118 (Peter Zhu)]

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

[nobu (Nobuyoshi Nakada)]

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.

[nobu (Nobuyoshi Nakada)]

• Status changed from Open to Closed

Applied in changeset git|5f8ebcada099351acbc22db264e7cd3773c2bdc4.

---

[Bug #19584] Register global variable address before assignment

[peterzhu2118 (Peter Zhu)]

• Backport changed from 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN to 3.0: REQUIRED, 3.1: REQUIRED, 3.2: REQUIRED

[Eregon (Benoit Daloze)]

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

[peterzhu2118 (Peter Zhu)]

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.