diff options
author | Yusuke Endoh <mame@ruby-lang.org> | 2021-11-09 17:06:01 +0900 |
---|---|---|
committer | Yusuke Endoh <mame@ruby-lang.org> | 2021-11-10 10:08:30 +0900 |
commit | 5c892da7d7974aeed8e7dd97bb31d2394cc19356 (patch) | |
tree | 4bbfbe05b8c6704ce4b8333b7c53a437e89eea17 /class.c | |
parent | 0d3898ec7b94b737fd9e0a9df1d0a944a9709564 (diff) |
class.c: descendants must not cause GC until the result array is created
Follow up of 428227472fc6563046d8138aab17f07bef6af753. The previous fix
uses `rb_ary_new_from_values` to create the result array, but it may
trigger the GC.
This second try is to create the result array by `rb_ary_new_capa`
before the second iteration, and assume that `rb_ary_push` does not
trigger GC. This assumption is very fragile, so should be improved in
future.
[Bug #18282] [Feature #14394]
Notes
Notes:
Merged: https://github.com/ruby/ruby/pull/5097
Diffstat (limited to 'class.c')
-rw-r--r-- | class.c | 22 |
1 files changed, 13 insertions, 9 deletions
@@ -29,6 +29,7 @@ #include "internal/variable.h" #include "ruby/st.h" #include "vm_core.h" +#include "gc.h" #define id_attached id__attached__ @@ -1338,7 +1339,7 @@ rb_mod_ancestors(VALUE mod) struct subclass_traverse_data { - VALUE *buffer; + VALUE buffer; long count; long maxcount; }; @@ -1349,8 +1350,9 @@ class_descendants_recursive(VALUE klass, VALUE v) struct subclass_traverse_data *data = (struct subclass_traverse_data *) v; if (BUILTIN_TYPE(klass) == T_CLASS && !FL_TEST(klass, FL_SINGLETON)) { - if (data->buffer && data->count < data->maxcount) { - data->buffer[data->count] = klass; + if (data->buffer && data->count < data->maxcount && !rb_objspace_garbage_object_p(klass)) { + // assumes that this does not cause GC as long as the length does not exceed the capacity + rb_ary_push(data->buffer, klass); } data->count++; } @@ -1378,24 +1380,26 @@ class_descendants_recursive(VALUE klass, VALUE v) VALUE rb_class_descendants(VALUE klass) { - struct subclass_traverse_data data = { NULL, 0, -1 }; + struct subclass_traverse_data data = { Qfalse, 0, -1 }; // estimate the count of subclasses rb_class_foreach_subclass(klass, class_descendants_recursive, (VALUE) &data); // the following allocation may cause GC which may change the number of subclasses - data.buffer = ALLOC_N(VALUE, data.count); + data.buffer = rb_ary_new_capa(data.count); data.maxcount = data.count; data.count = 0; + size_t gc_count = rb_gc_count(); + // enumerate subclasses rb_class_foreach_subclass(klass, class_descendants_recursive, (VALUE) &data); - VALUE ary = rb_ary_new_from_values(data.count, data.buffer); - - free(data.buffer); + if (gc_count != rb_gc_count()) { + rb_bug("GC must not occur during the subclass iteration of Class#descendants"); + } - return ary; + return data.buffer; } static void |