Issue #14070 has been updated by Aaroncgray (Aaronc Gray).


I think that its clever to read your blogging. Keep the great work. Frequently my own plan to make my own site and also my own particular pleasure is becoming because of this page. I do trust you may well be more convincing. You can use this [best essay writing service](https://www.clazwork.com) for any kind of academic writing work.

----------------------------------------
Bug #14070: Refining a module dumps core
https://bugs.ruby-lang.org/issues/14070#change-68073

* Author: mame (Yusuke Endoh)
* Status: Assigned
* Priority: Normal
* Assignee: matz (Yukihiro Matsumoto)
* Target version: 2.5
* ruby -v: ruby 2.5.0dev (2017-10-30 trunk 60565) [x86_64-linux]
* Backport: 2.3: UNKNOWN, 2.4: UNKNOWN
----------------------------------------
Including and refining one module at a time, seems to cause "double free" bug.  Here is a short example (ko1 found):

~~~
loop do
  Class.new do
    include Enumerable
  end

  Module.new do
    refine Enumerable do
      def foo
      end
    end
  end
end
~~~

`make test-spec` fails with SEGV very infrequently.

The refinement module (the return value of `refine Enumerable`) seems to be included to Enumerable, and simultaneously, to have Enumerable as a superclass.  Is this intended?  The current VM assumes that a module has no superclass, and that a class is not included to other class.  This discrepancy causes SEGV.

If this is really intended, we need to extend VM to support a module that has a superclass and is included.  The following patch fixes the issue by keeping a included module list in addition to a subclass list.  I'm unsure if this is enough or not.

~~~
diff --git a/class.c b/class.c
index 364f258333..a092fd7cc9 100644
--- a/class.c
+++ b/class.c
@@ -62,14 +62,14 @@ rb_module_add_to_subclasses_list(VALUE module, VALUE iclass)
     entry->klass = iclass;
     entry->next = NULL;
 
-    head = RCLASS_EXT(module)->subclasses;
+    head = RCLASS_EXT(module)->submodules;
     if (head) {
 	entry->next = head;
 	RCLASS_EXT(head->klass)->module_subclasses = &entry->next;
     }
 
-    RCLASS_EXT(module)->subclasses = entry;
-    RCLASS_EXT(iclass)->module_subclasses = &RCLASS_EXT(module)->subclasses;
+    RCLASS_EXT(module)->submodules = entry;
+    RCLASS_EXT(iclass)->module_subclasses = &RCLASS_EXT(module)->submodules;
 }
 
 void
@@ -110,10 +110,8 @@ rb_class_remove_from_module_subclasses(VALUE klass)
 }
 
 void
-rb_class_foreach_subclass(VALUE klass, void (*f)(VALUE, VALUE), VALUE arg)
+rb_class_foreach_subclass(rb_subclass_entry_t *cur, void (*f)(VALUE, VALUE), VALUE arg)
 {
-    rb_subclass_entry_t *cur = RCLASS_EXT(klass)->subclasses;
-
     /* do not be tempted to simplify this loop into a for loop, the order of
        operations is important here if `f` modifies the linked list */
     while (cur) {
@@ -132,7 +130,7 @@ class_detach_subclasses(VALUE klass, VALUE arg)
 void
 rb_class_detach_subclasses(VALUE klass)
 {
-    rb_class_foreach_subclass(klass, class_detach_subclasses, Qnil);
+    rb_class_foreach_subclass(RCLASS_EXT(klass)->subclasses, class_detach_subclasses, Qnil);
 }
 
 static void
@@ -144,7 +142,7 @@ class_detach_module_subclasses(VALUE klass, VALUE arg)
 void
 rb_class_detach_module_subclasses(VALUE klass)
 {
-    rb_class_foreach_subclass(klass, class_detach_module_subclasses, Qnil);
+    rb_class_foreach_subclass(RCLASS_EXT(klass)->submodules, class_detach_module_subclasses, Qnil);
 }
 
 /**
diff --git a/gc.c b/gc.c
index 0d6163ea98..a85375ffd7 100644
--- a/gc.c
+++ b/gc.c
@@ -2220,14 +2220,12 @@ obj_free(rb_objspace_t *objspace, VALUE obj)
 	if (RCLASS_IV_INDEX_TBL(obj)) {
 	    st_free_table(RCLASS_IV_INDEX_TBL(obj));
 	}
+	if (RCLASS_EXT(obj)->submodules) {
+	    rb_class_detach_module_subclasses(obj);
+	    RCLASS_EXT(obj)->submodules = NULL;
+	}
 	if (RCLASS_EXT(obj)->subclasses) {
-	    if (BUILTIN_TYPE(obj) == T_MODULE) {
-		rb_class_detach_module_subclasses(obj);
-	    }
-	    else {
-		rb_class_detach_subclasses(obj);
-	    }
-	    RCLASS_EXT(obj)->subclasses = NULL;
+	    rb_class_detach_subclasses(obj);
 	}
 	rb_class_remove_from_module_subclasses(obj);
 	rb_class_remove_from_super_subclasses(obj);
diff --git a/internal.h b/internal.h
index ad29434c7c..10b8051748 100644
--- a/internal.h
+++ b/internal.h
@@ -764,6 +764,7 @@ struct rb_classext_struct {
      * in the module's `subclasses` list that indicates that the klass has been
      * included. Hopefully that makes sense.
      */
+    rb_subclass_entry_t *submodules;
     rb_subclass_entry_t **module_subclasses;
     rb_serial_t class_serial;
     const VALUE origin_;
@@ -1077,7 +1078,7 @@ VALUE rb_class_boot(VALUE);
 VALUE rb_class_inherited(VALUE, VALUE);
 VALUE rb_make_metaclass(VALUE, VALUE);
 VALUE rb_include_class_new(VALUE, VALUE);
-void rb_class_foreach_subclass(VALUE klass, void (*f)(VALUE, VALUE), VALUE);
+void rb_class_foreach_subclass(rb_subclass_entry_t *cur, void (*f)(VALUE, VALUE), VALUE);
 void rb_class_detach_subclasses(VALUE);
 void rb_class_detach_module_subclasses(VALUE);
 void rb_class_remove_from_module_subclasses(VALUE);
diff --git a/vm_method.c b/vm_method.c
index 3378f10bd5..817ae09fcb 100644
--- a/vm_method.c
+++ b/vm_method.c
@@ -77,7 +77,8 @@ rb_class_clear_method_cache(VALUE klass, VALUE arg)
 	}
     }
 
-    rb_class_foreach_subclass(klass, rb_class_clear_method_cache, arg);
+    rb_class_foreach_subclass(RCLASS_EXT(klass)->subclasses, rb_class_clear_method_cache, arg);
+    rb_class_foreach_subclass(RCLASS_EXT(klass)->submodules, rb_class_clear_method_cache, arg);
 }
 
 void
@@ -103,7 +104,14 @@ rb_clear_method_cache_by_class(VALUE klass)
     }
 
     if (klass == rb_mKernel) {
-	rb_subclass_entry_t *entry = RCLASS_EXT(klass)->subclasses;
+	rb_subclass_entry_t *entry = RCLASS_EXT(klass)->submodules;
+
+	for (; entry != NULL; entry = entry->next) {
+	    struct rb_id_table *table = RCLASS_CALLABLE_M_TBL(entry->klass);
+	    if (table)rb_id_table_clear(table);
+	}
+
+	entry = RCLASS_EXT(klass)->subclasses;
 
 	for (; entry != NULL; entry = entry->next) {
 	    struct rb_id_table *table = RCLASS_CALLABLE_M_TBL(entry->klass);
@@ -479,7 +487,8 @@ check_override_opt_method(VALUE klass, VALUE arg)
 	    if (newme != me) rb_vm_check_redefinition_opt_method(me, me->owner);
 	}
     }
-    rb_class_foreach_subclass(klass, check_override_opt_method, (VALUE)mid);
+    rb_class_foreach_subclass(RCLASS_EXT(klass)->subclasses, check_override_opt_method, (VALUE)mid);
+    rb_class_foreach_subclass(RCLASS_EXT(klass)->submodules, check_override_opt_method, (VALUE)mid);
 }
 
 /*
~~~



-- 
https://bugs.ruby-lang.org/

Unsubscribe: <mailto:ruby-core-request / ruby-lang.org?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>