vm_trace.c: fix infinite hook

* thread.c (rb_threadptr_execute_interrupts): flush postponed job only
  once at last.
* vm_trace.c (rb_postponed_job_flush): defer calling postponed jobs
  registered while flushing to get rid of infinite reentrance of
  ObjectSpace.after_gc_start_hook.  [ruby-dev:47400] [Bug #8492]

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@43245 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

4 files changed, 87 insertions, 28 deletions

diff --git a/ChangeLog b/ChangeLog
index 447ad6b5d8..93fc38e0f0 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+Fri Oct 11 03:36:49 2013  Nobuyoshi Nakada  <nobu@ruby-lang.org>
+
+	* thread.c (rb_threadptr_execute_interrupts): flush postponed job only
+	  once at last.
+
+	* vm_trace.c (rb_postponed_job_flush): defer calling postponed jobs
+	  registered while flushing to get rid of infinite reentrance of
+	  ObjectSpace.after_gc_start_hook.  [ruby-dev:47400] [Bug #8492]
+
 Thu Oct 10 23:04:00 2013  Masaki Matsushita  <glass.saga@gmail.com>
 
 	* array.c (rb_ary_or): remove unused variables.
diff --git a/test/objspace/test_objspace.rb b/test/objspace/test_objspace.rb
index 133b1e90a4..861e40e62b 100644
--- a/test/objspace/test_objspace.rb
+++ b/test/objspace/test_objspace.rb
@@ -172,4 +172,23 @@ class TestObjSpace < Test::Unit::TestCase
     assert_equal(nil, ObjectSpace.allocation_sourcefile(obj2))
     assert_equal(nil, ObjectSpace.allocation_sourcefile(obj3))
   end
+
+  def test_after_gc_start_hook_with_GC_stress
+    bug8492 = '[ruby-dev:47400] [Bug #8492]: infinite after_gc_start_hook reentrance'
+    assert_nothing_raised(Timeout::Error, bug8492) do
+      assert_in_out_err(%w[-robjspace], <<-'end;', /\A[1-9]/, timeout: 2)
+        stress, GC.stress = GC.stress, false
+        count = 0
+        ObjectSpace.after_gc_start_hook = proc {count += 1}
+        begin
+          GC.stress = true
+          3.times {Object.new}
+        ensure
+          GC.stress = stress
+          ObjectSpace.after_gc_start_hook = nil
+        end
+        puts count
+      end;
+    end
+  end
 end
diff --git a/thread.c b/thread.c
index 4789dfa0e8..11ec96ea6c 100644
--- a/thread.c
+++ b/thread.c
@@ -1924,29 +1924,33 @@ rb_threadptr_to_kill(rb_thread_t *th)
     TH_JUMP_TAG(th, TAG_FATAL);
 }
 
+static inline rb_atomic_t
+threadptr_get_interrupts(rb_thread_t *th)
+{
+    rb_atomic_t interrupt;
+    rb_atomic_t old;
+
+    do {
+	interrupt = th->interrupt_flag;
+	old = ATOMIC_CAS(th->interrupt_flag, interrupt, interrupt & th->interrupt_mask);
+    } while (old != interrupt);
+    return interrupt & (rb_atomic_t)~th->interrupt_mask;
+}
+
 void
 rb_threadptr_execute_interrupts(rb_thread_t *th, int blocking_timing)
 {
+    rb_atomic_t interrupt;
+    int postponed_job_interrupt = 0;
+
     if (th->raised_flag) return;
 
-    while (1) {
-	rb_atomic_t interrupt;
-	rb_atomic_t old;
+    while ((interrupt = threadptr_get_interrupts(th)) != 0) {
 	int sig;
 	int timer_interrupt;
 	int pending_interrupt;
-	int postponed_job_interrupt;
 	int trap_interrupt;
 
-	do {
-	    interrupt = th->interrupt_flag;
-	    old = ATOMIC_CAS(th->interrupt_flag, interrupt, interrupt & th->interrupt_mask);
-	} while (old != interrupt);
-
-	interrupt &= (rb_atomic_t)~th->interrupt_mask;
-	if (!interrupt)
-	    return;
-
 	timer_interrupt = interrupt & TIMER_INTERRUPT_MASK;
 	pending_interrupt = interrupt & PENDING_INTERRUPT_MASK;
 	postponed_job_interrupt = interrupt & POSTPONED_JOB_INTERRUPT_MASK;
@@ -1984,10 +1988,6 @@ rb_threadptr_execute_interrupts(rb_thread_t *th, int blocking_timing)
 	    }
 	}
 
-	if (postponed_job_interrupt) {
-	    rb_postponed_job_flush(th->vm);
-	}
-
 	if (timer_interrupt) {
 	    unsigned long limits_us = TIME_QUANTUM_USEC;
 
@@ -2004,6 +2004,10 @@ rb_threadptr_execute_interrupts(rb_thread_t *th, int blocking_timing)
 	    rb_thread_schedule_limits(limits_us);
 	}
     }
+
+    if (postponed_job_interrupt) {
+	rb_postponed_job_flush(th->vm);
+    }
 }
 
 void
diff --git a/vm_trace.c b/vm_trace.c
index e590583d11..063bd828ff 100644
--- a/vm_trace.c
+++ b/vm_trace.c
@@ -1444,19 +1444,46 @@ rb_postponed_job_register_one(unsigned int flags, rb_postponed_job_func_t func,
 void
 rb_postponed_job_flush(rb_vm_t *vm)
 {
-    rb_postponed_job_t *pjob;
-
-    while (1) {
-	int index = vm->postponed_job_index;
-
-	if (index <= 0) {
-	    return; /* finished */
+    rb_thread_t *cur_th = GET_THREAD();
+    volatile struct {
+	rb_thread_t *thread;
+	unsigned long interrupt_mask;
+	int index, old_index;
+    } save;
+    int index = vm->postponed_job_index, old_index = index;
+
+    save.thread = cur_th;
+    save.interrupt_mask = cur_th->interrupt_mask;
+
+    cur_th->interrupt_mask |= POSTPONED_JOB_INTERRUPT_MASK;
+    TH_PUSH_TAG(cur_th);
+    if (EXEC_TAG()) {
+	/* ignore all jumps, just continue */
+	cur_th = save.thread;
+	index = save.index;
+	old_index = save.old_index;
+    }
+    while (index > 0) {
+	rb_postponed_job_t *pjob = &vm->postponed_job_buffer[--index];
+	void *data = pjob->data;
+	rb_postponed_job_func_t func = pjob->func;
+
+	pjob->func = 0;		/* not to execute again */
+	if (old_index > 0) {
+	    if (ATOMIC_CAS(vm->postponed_job_index, old_index, index) == old_index) {
+		old_index = index;
+	    }
+	    else {
+		old_index = 0;
+	    }
 	}
-
-	if (ATOMIC_CAS(vm->postponed_job_index, index, index-1) == index) {
-	    pjob = &vm->postponed_job_buffer[index-1];
+	save.index = index;
+	save.old_index = old_index;
+	if (func) {
 	    /* do postponed job */
-	    pjob->func(pjob->data);
+	    (*func)(data);
 	}
     }
+    TH_POP_TAG();
+    cur_th->interrupt_mask = save.interrupt_mask;
 }