nagachika	2017-08-05 15:35:02 +0900 (Sat, 05 Aug 2017)

  New Revision: 59516

  https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=59516

  Log:
    merge revision(s) 59462,59474: [Backport #13772]
    
    release VM stack properly.
    
    * cont.c: r55766 change the handling method of Fiber's VM stack.
      Resumed Fiber points NULL as VM stack and running Thread has
      responsibility to manage it (marking and releasing).
    
      However, thread_start_func_2()@thread.c and thread_free()@vm.c
      doesn't free the VM stack if corresponding root Fiber is exist.
      This causes memory leak. [Bug #13772]
    
    * cont.c (root_fiber_alloc): fib->cont.saved_thread.ec.stack should be NULL
      because running thread has responsibility to manage this stack.
    
    * vm.c (rb_thread_recycle_stack_release): assert given stack is not NULL
      (callers should care it).
    
    fix stack storing for root fibers.
    
    * cont.c (root_fiber_alloc): this function is called by fiber_current()
      and fiber_store(). fiber_current() should clear VM stack information
      in a fiber data because runnning thread knows stack information and has
      responsibility to manage it. However fiber_store() requires to remain
      VM stack information in a fiber data because the responsibility to manage
      VM stack is moved to the Fiber from the Thread (and switch to another
      fiber).
    
    * cont.c (root_fiber_alloc): save thread's fiber and root_fiber information.

  Modified directories:
    branches/ruby_2_4/
  Modified files:
    branches/ruby_2_4/cont.c
    branches/ruby_2_4/thread.c
    branches/ruby_2_4/vm.c
Index: ruby_2_4/cont.c
===================================================================
--- ruby_2_4/cont.c	(revision 59515)
+++ ruby_2_4/cont.c	(revision 59516)
@@ -575,6 +575,8 @@ cont_restore_thread(rb_context_t *cont) https://github.com/ruby/ruby/blob/trunk/ruby_2_4/cont.c#L575
     th->root_lep = sth->root_lep;
     th->root_svar = sth->root_svar;
     th->ensure_list = sth->ensure_list;
+    VM_ASSERT(th->stack != NULL);
+    VM_ASSERT(sth->status == THREAD_RUNNABLE);
 }
 
 #if FIBER_USE_NATIVE
@@ -1316,6 +1318,7 @@ root_fiber_alloc(rb_thread_t *th) https://github.com/ruby/ruby/blob/trunk/ruby_2_4/cont.c#L1318
 #endif
     fib->status = RUNNING;
 
+    th->root_fiber = th->fiber = fib;
     return fib;
 }
 
@@ -1324,9 +1327,9 @@ fiber_current(void) https://github.com/ruby/ruby/blob/trunk/ruby_2_4/cont.c#L1327
 {
     rb_thread_t *th = GET_THREAD();
     if (th->fiber == 0) {
-	/* save root */
 	rb_fiber_t *fib = root_fiber_alloc(th);
-	th->root_fiber = th->fiber = fib;
+	/* Running thread object has stack management responsibility */
+	fib->cont.saved_thread.stack = NULL;
     }
     return th->fiber;
 }
@@ -1367,9 +1370,8 @@ fiber_store(rb_fiber_t *next_fib, rb_thr https://github.com/ruby/ruby/blob/trunk/ruby_2_4/cont.c#L1370
 	cont_save_thread(&fib->cont, th);
     }
     else {
-	/* create current fiber */
+	/* create root fiber */
 	fib = root_fiber_alloc(th);
-	th->root_fiber = th->fiber = fib;
     }
 
 #if FIBER_USE_NATIVE
Index: ruby_2_4/thread.c
===================================================================
--- ruby_2_4/thread.c	(revision 59515)
+++ ruby_2_4/thread.c	(revision 59516)
@@ -688,10 +688,8 @@ thread_start_func_2(rb_thread_t *th, VAL https://github.com/ruby/ruby/blob/trunk/ruby_2_4/thread.c#L688
 	rb_threadptr_unlock_all_locking_mutexes(th);
 	rb_check_deadlock(th->vm);
 
-	if (!th->root_fiber) {
-	    rb_thread_recycle_stack_release(th->stack);
-	    th->stack = 0;
-	}
+	rb_thread_recycle_stack_release(th->stack);
+	th->stack = NULL;
     }
     native_mutex_lock(&th->vm->thread_destruct_lock);
     /* make sure vm->running_thread never point me after this point.*/
Index: ruby_2_4/vm.c
===================================================================
--- ruby_2_4/vm.c	(revision 59515)
+++ ruby_2_4/vm.c	(revision 59516)
@@ -90,6 +90,8 @@ VM_CFP_IN_HEAP_P(const rb_thread_t *th, https://github.com/ruby/ruby/blob/trunk/ruby_2_4/vm.c#L90
 {
     const VALUE *start = th->stack;
     const VALUE *end = (VALUE *)th->stack + th->stack_size;
+    VM_ASSERT(start != NULL);
+
     if (start <= (VALUE *)cfp && (VALUE *)cfp < end) {
 	return FALSE;
     }
@@ -103,6 +105,8 @@ VM_EP_IN_HEAP_P(const rb_thread_t *th, c https://github.com/ruby/ruby/blob/trunk/ruby_2_4/vm.c#L105
 {
     const VALUE *start = th->stack;
     const VALUE *end = (VALUE *)th->cfp;
+    VM_ASSERT(start != NULL);
+
     if (start <= ep && ep < end) {
 	return FALSE;
     }
@@ -2315,7 +2319,7 @@ static int thread_recycle_stack_count = https://github.com/ruby/ruby/blob/trunk/ruby_2_4/vm.c#L2319
 static VALUE *
 thread_recycle_stack(size_t size)
 {
-    if (thread_recycle_stack_count) {
+    if (thread_recycle_stack_count > 0) {
 	/* TODO: check stack size if stack sizes are variable */
 	return thread_recycle_stack_slot[--thread_recycle_stack_count];
     }
@@ -2331,6 +2335,8 @@ thread_recycle_stack(size_t size) https://github.com/ruby/ruby/blob/trunk/ruby_2_4/vm.c#L2335
 void
 rb_thread_recycle_stack_release(VALUE *stack)
 {
+    VM_ASSERT(stack != NULL);
+
 #if USE_THREAD_DATA_RECYCLE
     if (thread_recycle_stack_count < RECYCLE_MAX) {
 	thread_recycle_stack_slot[thread_recycle_stack_count++] = stack;
@@ -2414,6 +2420,10 @@ thread_free(void *ptr) https://github.com/ruby/ruby/blob/trunk/ruby_2_4/vm.c#L2420
 
     if (ptr) {
 	th = ptr;
+	if (th->stack != NULL) {
+	    rb_thread_recycle_stack_release(th->stack);
+	    th->stack = NULL;
+	}
 
 	if (!th->root_fiber) {
 	    RUBY_FREE_UNLESS_NULL(th->stack);
Index: ruby_2_4
===================================================================
--- ruby_2_4	(revision 59515)
+++ ruby_2_4	(revision 59516)

Property changes on: ruby_2_4
___________________________________________________________________
Modified: svn:mergeinfo
## -0,0 +0,1 ##
   Merged /trunk:r59462,59474

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