summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authork0kubun <k0kubun@b2dd03c8-39d4-4d8f-98ff-823fe69b080e>2018-11-06 07:22:25 +0000
committerk0kubun <k0kubun@b2dd03c8-39d4-4d8f-98ff-823fe69b080e>2018-11-06 07:22:25 +0000
commit7a2263fb389cbd0fb9cc8d86ed909b7e080ef1e7 (patch)
tree092c35f03dbbdcf557855e27c79254577d615c5a
parent9f17712d2c8af72e211467d09eb920ab320be307 (diff)
mjit_worker.c: strictly control MJIT copy job
-available region. reducing risk of SEGV in mjit_copy_job_handler() like http://ci.rvm.jp/results/trunk-mjit@silicon-docker/1446117 I'm not sure which exact part is causing "[BUG] Segmentation fault at 0x0000000000000008" on `(mjit_copy_job_handler+0x12) [0x564a6c4ce632] /home/ko1/ruby/src/trunk-mjit/mjit.c:26`... mjit.c: ditto git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@65569 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
-rw-r--r--mjit.c21
-rw-r--r--mjit_worker.c21
2 files changed, 26 insertions, 16 deletions
diff --git a/mjit.c b/mjit.c
index 7844ed9..4ff5ff9 100644
--- a/mjit.c
+++ b/mjit.c
@@ -24,15 +24,20 @@
static void
mjit_copy_job_handler(void *data)
{
- struct mjit_copy_job *job;
- if (stop_worker_p) {
- /* `copy_cache_from_main_thread()` stops to wait for this job. Then job
- data which is allocated by `alloca()` could be expired and we might
- not be able to access that. */
+ struct mjit_copy_job *job = data;
+ int finish_p;
+ CRITICAL_SECTION_START(3, "in mjit_copy_job_handler");
+ finish_p = job->finish_p;
+ CRITICAL_SECTION_FINISH(3, "in mjit_copy_job_handler");
+
+ if (stop_worker_p || finish_p) {
+ /* `stop_worker_p`: `copy_cache_from_main_thread()` stops to wait for this job.
+ Then job data which is allocated by `alloca()` could be expired and we might
+ not be able to access that.
+ `finish_p`: make sure that this job is never executed while job is being modified. */
return;
}
- job = (struct mjit_copy_job *)data;
if (job->cc_entries) {
memcpy(job->cc_entries, job->body->cc_entries, sizeof(struct rb_call_cache) * (job->body->ci_size + job->body->ci_kw_size));
}
@@ -40,10 +45,10 @@ mjit_copy_job_handler(void *data)
memcpy(job->is_entries, job->body->is_entries, sizeof(union iseq_inline_storage_entry) * job->body->is_size);
}
- CRITICAL_SECTION_START(3, "in MJIT copy job wait");
+ CRITICAL_SECTION_START(3, "in mjit_copy_job_handler");
job->finish_p = TRUE;
rb_native_cond_broadcast(&mjit_worker_wakeup);
- CRITICAL_SECTION_FINISH(3, "in MJIT copy job wait");
+ CRITICAL_SECTION_FINISH(3, "in mjit_copy_job_handler");
}
extern int rb_thread_create_mjit_thread(void (*worker_func)(void));
diff --git a/mjit_worker.c b/mjit_worker.c
index c8a7c21..fd83e7a 100644
--- a/mjit_worker.c
+++ b/mjit_worker.c
@@ -1136,11 +1136,17 @@ static void mjit_copy_job_handler(void *data);
static int
copy_cache_from_main_thread(struct mjit_copy_job *job)
{
- job->finish_p = FALSE;
+ CRITICAL_SECTION_START(3, "in copy_cache_from_main_thread");
+ job->finish_p = FALSE; /* allow dispatching this job in mjit_copy_job_handler */
+ CRITICAL_SECTION_FINISH(3, "in copy_cache_from_main_thread");
- if (!rb_postponed_job_register(0, mjit_copy_job_handler, (void *)job))
- return FALSE;
+ if (UNLIKELY(mjit_opts.wait)) {
+ mjit_copy_job_handler((void *)job);
+ return job->finish_p;
+ }
+ if (!rb_postponed_job_register_one(0, mjit_copy_job_handler, (void *)job))
+ return FALSE;
CRITICAL_SECTION_START(3, "in MJIT copy job wait");
/* checking `stop_worker_p` too because `RUBY_VM_CHECK_INTS(ec)` may not
lush mjit_copy_job_handler when EC_EXEC_TAG() is not TAG_NONE, and then
@@ -1159,6 +1165,8 @@ copy_cache_from_main_thread(struct mjit_copy_job *job)
void
mjit_worker(void)
{
+ struct mjit_copy_job job;
+
#ifndef _MSC_VER
if (pch_status == PCH_NOT_READY) {
make_pch();
@@ -1185,11 +1193,11 @@ mjit_worker(void)
verbose(3, "Getting wakeup from client");
}
unit = get_from_list(&unit_queue);
+ job.finish_p = TRUE; /* disable dispatching this job in mjit_copy_job_handler while it's being modified */
CRITICAL_SECTION_FINISH(3, "in worker dequeue");
if (unit) {
mjit_func_t func;
- struct mjit_copy_job job;
job.body = unit->iseq->body;
job.cc_entries = NULL;
@@ -1201,10 +1209,7 @@ mjit_worker(void)
/* Copy ISeq's inline caches values to avoid race condition. */
if (job.cc_entries != NULL || job.is_entries != NULL) {
- if (UNLIKELY(mjit_opts.wait)) {
- mjit_copy_job_handler((void *)&job); /* main thread is waiting in mjit_wait_call() and doesn't race */
- }
- else if (copy_cache_from_main_thread(&job) == FALSE) {
+ if (copy_cache_from_main_thread(&job) == FALSE) {
continue; /* retry postponed_job failure, or stop worker */
}
}