[前][次][番号順一覧][スレッド一覧]

ruby-changes:66415

From: Takashi <ko1@a...>
Date: Thu, 3 Jun 2021 14:15:06 +0900 (JST)
Subject: [ruby-changes:66415] 86c262541a (master): Fix a race condition around mjit_recompile

https://git.ruby-lang.org/ruby.git/commit/?id=86c262541a

From 86c262541ad07528842d76dab4b9b34bd888d5f4 Mon Sep 17 00:00:00 2001
From: Takashi Kokubun <takashikkbn@g...>
Date: Wed, 2 Jun 2021 22:07:44 -0700
Subject: Fix a race condition around mjit_recompile

This fixes SEGVs like https://github.com/ruby/ruby/runs/2715166621?check_suite_focus=true.

When mjit_recompile is called when mjit_compile is compiling the exact
same iseq (and after it called mjit_capture_cc_entries), iseq->body->jit_unit
is re-created and its cc_entries becomes NULL. Then, when it tries to
lookup cc_entries through iseq->body->jit_unit, it fails.
---
 mjit.c        | 21 +++++++++++++--------
 mjit_worker.c |  4 ++++
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/mjit.c b/mjit.c
index 035353d..d9c83c4 100644
--- a/mjit.c
+++ b/mjit.c
@@ -350,17 +350,22 @@ mjit_recompile(const rb_iseq_t *iseq) https://github.com/ruby/ruby/blob/trunk/mjit.c#L350
             RSTRING_PTR(rb_iseq_path(iseq)), FIX2INT(iseq->body->location.first_lineno));
     assert(iseq->body->jit_unit != NULL);
 
-    // Lazily move active_units to stale_units to avoid race conditions around active_units with compaction
-    CRITICAL_SECTION_START(3, "in rb_mjit_recompile_iseq");
-    iseq->body->jit_unit->stale_p = true;
-    pending_stale_p = true;
-    CRITICAL_SECTION_FINISH(3, "in rb_mjit_recompile_iseq");
-
-    iseq->body->jit_func = (mjit_func_t)NOT_ADDED_JIT_ISEQ_FUNC;
-    mjit_add_iseq_to_process(iseq, &iseq->body->jit_unit->compile_info);
     if (UNLIKELY(mjit_opts.wait)) {
+        remove_from_list(iseq->body->jit_unit, &active_units);
+        add_to_list(iseq->body->jit_unit, &stale_units);
+        mjit_add_iseq_to_process(iseq, &iseq->body->jit_unit->compile_info);
         mjit_wait(iseq->body);
     }
+    else {
+        // Lazily move active_units to stale_units to avoid race conditions around active_units with compaction.
+        // Also, it's lazily moved to unit_queue as well because otherwise it won't be added to stale_units properly.
+        // It's good to avoid a race condition between mjit_add_iseq_to_process and mjit_compile around jit_unit as well.
+        CRITICAL_SECTION_START(3, "in rb_mjit_recompile_iseq");
+        iseq->body->jit_unit->stale_p = true;
+        iseq->body->jit_func = (mjit_func_t)NOT_ADDED_JIT_ISEQ_FUNC;
+        pending_stale_p = true;
+        CRITICAL_SECTION_FINISH(3, "in rb_mjit_recompile_iseq");
+    }
 }
 
 // Recompile iseq, disabling send optimization
diff --git a/mjit_worker.c b/mjit_worker.c
index 3a73f14..50f1b07 100644
--- a/mjit_worker.c
+++ b/mjit_worker.c
@@ -1396,6 +1396,8 @@ unload_units(void) https://github.com/ruby/ruby/blob/trunk/mjit_worker.c#L1396
     }
 }
 
+static void mjit_add_iseq_to_process(const rb_iseq_t *iseq, const struct rb_mjit_compile_info *compile_info);
+
 // The function implementing a worker. It is executed in a separate
 // thread by rb_thread_create_mjit_thread. It compiles precompiled header
 // and then compiles requested ISeqs.
@@ -1445,6 +1447,8 @@ mjit_worker(void) https://github.com/ruby/ruby/blob/trunk/mjit_worker.c#L1447
                         unit->stale_p = false;
                         remove_from_list(unit, &active_units);
                         add_to_list(unit, &stale_units);
+                        // Lazily put it to unit_queue as well to avoid race conditions on jit_unit with mjit_compile.
+                        mjit_add_iseq_to_process(unit->iseq, &unit->iseq->body->jit_unit->compile_info);
                     }
                 }
             }
-- 
cgit v1.1


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

[前][次][番号順一覧][スレッド一覧]