ko1	2013-06-19 04:50:44 +0900 (Wed, 19 Jun 2013)

  New Revision: 41396

  http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=rev&revision=41396

  Log:
    * variable.c (rb_const_set): fix WB miss.
    
      WBs had located before creating reference between a klass
      and constant value. It causes GC bug.
    
        # pseudo code:
        WB(klass, value); # WB and remember klass
        st_insert(klass->const_table, const_id, value);
    
      `st_insert()' can cause GC before inserting `value' and
      forget `klass' from the remember set. After that, relationship
      between `klass' and `value' are created with constant table.
      Now, `value' can be young (shady) object and `klass' can be old
      object, without remembering `klass' object.
      At the next GC, old `klass' object will be skipped and
      young (shady) `value' will be miss-collected. -> GC bug
    
      Lesson: The place of a WB is important.

  Modified files:
    trunk/ChangeLog
    trunk/variable.c

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 41395)
+++ ChangeLog	(revision 41396)
@@ -1,3 +1,24 @@ https://github.com/ruby/ruby/blob/trunk/ChangeLog#L1
+Wed Jun 19 04:33:22 2013  Koichi Sasada  <ko1@a...>
+
+	* variable.c (rb_const_set): fix WB miss.
+
+	  WBs had located before creating reference between a klass
+	  and constant value. It causes GC bug.
+
+	    # pseudo code:
+	    WB(klass, value); # WB and remember klass
+	    st_insert(klass->const_table, const_id, value);
+
+	  `st_insert()' can cause GC before inserting `value' and
+	  forget `klass' from the remember set. After that, relationship
+	  between `klass' and `value' are created with constant table.
+	  Now, `value' can be young (shady) object and `klass' can be old
+	  object, without remembering `klass' object.
+	  At the next GC, old `klass' object will be skipped and
+	  young (shady) `value' will be miss-collected. -> GC bug
+
+	  Lesson: The place of a WB is important.
+
 Tue Jun 18 22:01:00 2013  Charlie Somerville  <charliesome@r...>
 
 	* vm_insnhelper.c (vm_call_method): ensure methods of type
Index: variable.c
===================================================================
--- variable.c	(revision 41395)
+++ variable.c	(revision 41396)
@@ -2155,7 +2155,7 @@ rb_const_set(VALUE klass, ID id, VALUE v https://github.com/ruby/ruby/blob/trunk/variable.c#L2155
 		/* for autoloading thread, keep the defined value to autoloading storage */
 		if (load && (ele = check_autoload_data(load)) && (ele->thread == rb_thread_current())) {
 		    rb_vm_change_state();
-		    ele->value = val;
+		    ele->value = val; /* autoload_i is shady */
 		    return;
 		}
 		/* otherwise, allow to override */
@@ -2180,12 +2180,12 @@ rb_const_set(VALUE klass, ID id, VALUE v https://github.com/ruby/ruby/blob/trunk/variable.c#L2180
     rb_vm_change_state();
 
     ce = ALLOC(rb_const_entry_t);
+    MEMZERO(ce, rb_const_entry_t, 1);
     ce->flag = visibility;
-    OBJ_WRITE(klass, (VALUE *)&ce->value, val);
-    OBJ_WRITE(klass, (VALUE *)&ce->file, rb_sourcefilename());
     ce->line = rb_sourceline();
-
     st_insert(RCLASS_CONST_TBL(klass), (st_data_t)id, (st_data_t)ce);
+    OBJ_WRITE(klass, (VALUE *)&ce->value, val);
+    OBJ_WRITE(klass, (VALUE *)&ce->file, rb_sourcefilename());
 }
 
 void

--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/