mirror of
https://github.com/ruby/ruby.git
synced 2022-11-09 12:17:21 -05:00
Mark active_units
to avoid SEGV on mjit_recompile and compact_all_jit_code.
For some reason, ISeqs on stack are sometimes GC-ed (why?) and therefore
it may run mjit_recompile on a GC-ed ISeq, which I expected d07183ec85
to fix but apparently it may refer to random things if already GC-ed.
Marking active_units would workaround the situation.
http://ci.rvm.jp/results/trunk-mjit-wait@phosphorus-docker/3292740
Also, while compact_all_jit_code was executed, we saw some SEGVs where
CCs seemed to be already GC-ed, meaning their owner ISeq was not marked
properly. Even if units are still in active_units, it's not guaranteed
that their ISeqs are in use. So in this case we need to mark active_units
for a legitimate reason.
http://ci.rvm.jp/results/trunk-mjit-wait@phosphorus-docker/3293277
http://ci.rvm.jp/results/trunk-mjit-wait@phosphorus-docker/3293090
This commit is contained in:
parent
a574df14e4
commit
1fdc97f1b7
3 changed files with 41 additions and 4 deletions
41
mjit.c
41
mjit.c
|
@ -350,10 +350,7 @@ mjit_recompile(const rb_iseq_t *iseq)
|
|||
|
||||
verbose(1, "JIT recompile: %s@%s:%d", RSTRING_PTR(iseq->body->location.label),
|
||||
RSTRING_PTR(rb_iseq_path(iseq)), FIX2INT(iseq->body->location.first_lineno));
|
||||
iseq->body->jit_func = (mjit_func_t)NOT_ADDED_JIT_ISEQ_FUNC;
|
||||
|
||||
if (iseq->body->jit_unit == NULL) // mjit_free_iseq is already called
|
||||
return;
|
||||
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");
|
||||
|
@ -361,6 +358,7 @@ mjit_recompile(const rb_iseq_t *iseq)
|
|||
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)) {
|
||||
mjit_wait(iseq->body);
|
||||
|
@ -937,6 +935,41 @@ mjit_finish(bool close_handle_p)
|
|||
verbose(1, "Successful MJIT finish");
|
||||
}
|
||||
|
||||
// Called by rb_vm_mark() to mark active_units so that we do not GC ISeq which
|
||||
// may still be referred to by mjit_recompile() or compact_all_jit_code().
|
||||
void
|
||||
mjit_mark(void)
|
||||
{
|
||||
if (!mjit_enabled)
|
||||
return;
|
||||
RUBY_MARK_ENTER("mjit");
|
||||
|
||||
// We need to release a lock when calling rb_gc_mark to avoid doubly acquiring
|
||||
// a lock by by mjit_gc_start_hook inside rb_gc_mark.
|
||||
//
|
||||
// Because an MJIT worker may modify active_units anytime, we need to convert
|
||||
// the linked list to an array to safely loop its ISeqs without keeping a lock.
|
||||
CRITICAL_SECTION_START(4, "mjit_mark");
|
||||
int length = active_units.length;
|
||||
rb_iseq_t **iseqs = ALLOCA_N(rb_iseq_t *, length);
|
||||
|
||||
struct rb_mjit_unit *unit = NULL;
|
||||
int i = 0;
|
||||
list_for_each(&active_units.head, unit, unode) {
|
||||
iseqs[i] = unit->iseq;
|
||||
i++;
|
||||
}
|
||||
CRITICAL_SECTION_FINISH(4, "mjit_mark");
|
||||
|
||||
for (i = 0; i < length; i++) {
|
||||
if (iseqs[i] == NULL) // ISeq is GC-ed
|
||||
continue;
|
||||
rb_gc_mark((VALUE)iseqs[i]);
|
||||
}
|
||||
|
||||
RUBY_MARK_LEAVE("mjit");
|
||||
}
|
||||
|
||||
// Called by rb_iseq_mark() to mark cc_entries captured for MJIT
|
||||
void
|
||||
mjit_mark_cc_entries(const struct rb_iseq_constant_body *const body)
|
||||
|
|
2
mjit.h
2
mjit.h
|
@ -98,6 +98,7 @@ extern void mjit_gc_start_hook(void);
|
|||
extern void mjit_gc_exit_hook(void);
|
||||
extern void mjit_free_iseq(const rb_iseq_t *iseq);
|
||||
extern void mjit_update_references(const rb_iseq_t *iseq);
|
||||
extern void mjit_mark(void);
|
||||
extern struct mjit_cont *mjit_cont_new(rb_execution_context_t *ec);
|
||||
extern void mjit_cont_free(struct mjit_cont *cont);
|
||||
extern void mjit_add_class_serial(rb_serial_t class_serial);
|
||||
|
@ -200,6 +201,7 @@ static inline void mjit_cont_free(struct mjit_cont *cont){}
|
|||
static inline void mjit_gc_start_hook(void){}
|
||||
static inline void mjit_gc_exit_hook(void){}
|
||||
static inline void mjit_free_iseq(const rb_iseq_t *iseq){}
|
||||
static inline void mjit_mark(void){}
|
||||
static inline void mjit_add_class_serial(rb_serial_t class_serial){}
|
||||
static inline void mjit_remove_class_serial(rb_serial_t class_serial){}
|
||||
static inline VALUE mjit_exec(rb_execution_context_t *ec) { return Qundef; /* unreachable */ }
|
||||
|
|
2
vm.c
2
vm.c
|
@ -2594,6 +2594,8 @@ rb_vm_mark(void *ptr)
|
|||
rb_gc_mark_values(RUBY_NSIG, vm->trap_list.cmd);
|
||||
|
||||
rb_id_table_foreach_values(vm->negative_cme_table, vm_mark_negative_cme, NULL);
|
||||
|
||||
mjit_mark();
|
||||
}
|
||||
|
||||
RUBY_MARK_LEAVE("vm");
|
||||
|
|
Loading…
Reference in a new issue