k0kubun	2018-11-27 00:47:20 +0900 (Tue, 27 Nov 2018)

  New Revision: 66001

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

  Log:
    vm_trace.c: MJIT-limited thread-safety for postponed_job
    
    [Bug #15316]

  Modified files:
    trunk/mjit.c
    trunk/mjit.h
    trunk/vm_trace.c
Index: mjit.c
===================================================================
--- mjit.c	(revision 66000)
+++ mjit.c	(revision 66001)
@@ -106,6 +106,20 @@ mjit_gc_finish_hook(void) https://github.com/ruby/ruby/blob/trunk/mjit.c#L106
     CRITICAL_SECTION_FINISH(4, "mjit_gc_finish_hook");
 }
 
+/* Wrap critical section to prevent [Bug #15316] */
+void
+mjit_postponed_job_register_start_hook(void)
+{
+    CRITICAL_SECTION_START(4, "mjit_postponed_job_register_start_hook");
+}
+
+/* Unwrap critical section of mjit_postponed_job_register_start_hook() */
+void
+mjit_postponed_job_register_finish_hook(void)
+{
+    CRITICAL_SECTION_FINISH(4, "mjit_postponed_job_register_finish_hook");
+}
+
 /* Iseqs can be garbage collected.  This function should call when it
    happens.  It removes iseq from the unit.  */
 void
Index: mjit.h
===================================================================
--- mjit.h	(revision 66000)
+++ mjit.h	(revision 66001)
@@ -66,6 +66,8 @@ RUBY_SYMBOL_EXPORT_END https://github.com/ruby/ruby/blob/trunk/mjit.h#L66
 
 extern int mjit_compile(FILE *f, const struct rb_iseq_constant_body *body, const char *funcname, struct rb_call_cache *cc_entries, union iseq_inline_storage_entry *is_entries);
 extern void mjit_init(struct mjit_options *opts);
+extern void mjit_postponed_job_register_start_hook(void);
+extern void mjit_postponed_job_register_finish_hook(void);
 extern void mjit_gc_start_hook(void);
 extern void mjit_gc_finish_hook(void);
 extern void mjit_free_iseq(const rb_iseq_t *iseq);
@@ -131,6 +133,8 @@ void mjit_child_after_fork(void); https://github.com/ruby/ruby/blob/trunk/mjit.h#L133
 #else /* USE_MJIT */
 static inline struct mjit_cont *mjit_cont_new(rb_execution_context_t *ec){return NULL;}
 static inline void mjit_cont_free(struct mjit_cont *cont){}
+static inline void mjit_postponed_job_register_start_hook(void){}
+static inline void mjit_postponed_job_register_finish_hook(void){}
 static inline void mjit_gc_start_hook(void){}
 static inline void mjit_gc_finish_hook(void){}
 static inline void mjit_free_iseq(const rb_iseq_t *iseq){}
Index: vm_trace.c
===================================================================
--- vm_trace.c	(revision 66000)
+++ vm_trace.c	(revision 66001)
@@ -1588,7 +1588,7 @@ enum postponed_job_register_result { https://github.com/ruby/ruby/blob/trunk/vm_trace.c#L1588
     PJRR_INTERRUPTED = 2
 };
 
-/* Async-signal-safe */
+/* Async-signal-safe, thread-safe against MJIT worker thread */
 static enum postponed_job_register_result
 postponed_job_register(rb_execution_context_t *ec, rb_vm_t *vm,
                        unsigned int flags, rb_postponed_job_func_t func, void *data, int max, int expected_index)
@@ -1596,11 +1596,13 @@ postponed_job_register(rb_execution_cont https://github.com/ruby/ruby/blob/trunk/vm_trace.c#L1596
     rb_postponed_job_t *pjob;
 
     if (expected_index >= max) return PJRR_FULL; /* failed */
+    if (mjit_enabled) mjit_postponed_job_register_start_hook();
 
     if (ATOMIC_CAS(vm->postponed_job_index, expected_index, expected_index+1) == expected_index) {
         pjob = &vm->postponed_job_buffer[expected_index];
     }
     else {
+        if (mjit_enabled) mjit_postponed_job_register_finish_hook();
         return PJRR_INTERRUPTED;
     }
 
@@ -1609,6 +1611,7 @@ postponed_job_register(rb_execution_cont https://github.com/ruby/ruby/blob/trunk/vm_trace.c#L1611
     pjob->data = data;
 
     RUBY_VM_SET_POSTPONED_JOB_INTERRUPT(ec);
+    if (mjit_enabled) mjit_postponed_job_register_finish_hook();
 
     return PJRR_SUCCESS;
 }

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