k0kubun	2019-04-14 21:40:44 +0900 (Sun, 14 Apr 2019)

  New Revision: 67551

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

  Log:
    Do not execute MJIT copy job when ISeq is GC-ed
    
    I assumed that ISeq is never GC-ed by `in_jit` + `mjit_mark` on copy job
    ISeq, but unfortunately I found SEGV on `mjit_copy_job_handler` in which
    iseq->body was somehow Qnil. And it seems to be fixed by disabling the
    job when `mjit_free_iseq` is called for the ISeq.

  Modified files:
    trunk/mjit.c
    trunk/mjit_worker.c
Index: mjit.c
===================================================================
--- mjit.c	(revision 67550)
+++ mjit.c	(revision 67551)
@@ -33,11 +33,16 @@ mjit_copy_job_handler(void *data) https://github.com/ruby/ruby/blob/trunk/mjit.c#L33
     // Make sure that this job is never executed when:
     //   1. job is being modified
     //   2. alloca memory inside job is expired
-    // Note that job->iseq is guarded from GC by `mjit_mark`.
+    //   3. ISeq is GC-ed
     if (job->finish_p) {
         CRITICAL_SECTION_FINISH(3, "in mjit_copy_job_handler");
         return;
     }
+    else if (job->iseq == NULL) { // ISeq GC notified in mjit_mark_iseq
+        job->finish_p = true;
+        CRITICAL_SECTION_FINISH(3, "in mjit_copy_job_handler");
+        return;
+    }
 
     const struct rb_iseq_constant_body *body = job->iseq->body;
     if (job->cc_entries) {
@@ -113,6 +118,9 @@ mjit_free_iseq(const rb_iseq_t *iseq) https://github.com/ruby/ruby/blob/trunk/mjit.c#L118
     if (!mjit_enabled)
         return;
     CRITICAL_SECTION_START(4, "mjit_free_iseq");
+    if (mjit_copy_job.iseq == iseq) {
+        mjit_copy_job.iseq = NULL;
+    }
     if (iseq->body->jit_unit) {
         // jit_unit is not freed here because it may be referred by multiple
         // lists of units. `get_from_list` and `mjit_finish` do the job.
Index: mjit_worker.c
===================================================================
--- mjit_worker.c	(revision 67550)
+++ mjit_worker.c	(revision 67551)
@@ -1195,15 +1195,17 @@ mjit_copy_cache_from_main_thread(const r https://github.com/ruby/ruby/blob/trunk/mjit_worker.c#L1195
     }
 
     CRITICAL_SECTION_START(3, "in mjit_copy_cache_from_main_thread");
-    bool result = job->finish_p;
+    bool success_p = job->finish_p;
     // Disable dispatching this job in mjit_copy_job_handler while memory allocated by alloca
     // could be expired after finishing this function.
     job->finish_p = true;
 
     in_jit = true; // Prohibit GC during JIT compilation
+    if (job->iseq == NULL) // ISeq GC is notified in mjit_mark_iseq
+        success_p = false;
     job->iseq = NULL; // Allow future GC of this ISeq from here
     CRITICAL_SECTION_FINISH(3, "in mjit_copy_cache_from_main_thread");
-    return result;
+    return success_p;
 }
 
 // The function implementing a worker. It is executed in a separate

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