normal	2018-12-04 04:49:54 +0900 (Tue, 04 Dec 2018)

  New Revision: 66171

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

  Log:
    process.c: fix ETXTBUSY from MJIT compiler process
    
    This affects test/ruby/test_process.rb (test_execopt_env_path).
    
    Since MJIT uses vfork+execve in a separate thread, there can be
    small window in-between vfork and execve where tmp_script.cmd is
    held open by the vforked child.  vfork only pauses the MJIT
    thread, not any Ruby Threads, so our call to Process.spawn will
    hit ETXTBUSY in that window unless we fork.
    
    main thread                       | MJIT thread
    ----------------------------------------------------
    fd = open(tmp)                    |                |
                                      | vfork for CC   |   CC running
    write                             |                | ---------------
    fchmod                            |                |  sees "fd" here
    close(fd)                         |                |
        Process.spawn called          |                |
    vfork  (spawn)|    (new process)  |                |
                  | execve => TXTBUSY |                |
                  |                   |                | execve (FD_CLOEXEC on fd)
                  |                   | vfork returns  |
    
    Holding the waitpid_lock whenever we intend to spawn a process
    prevents the MJIT thread from spawning a process while we are
    spawning in Ruby-land.

  Modified files:
    trunk/process.c
    trunk/test/ruby/test_process.rb
Index: process.c
===================================================================
--- process.c	(revision 66170)
+++ process.c	(revision 66171)
@@ -916,6 +916,8 @@ do_waitpid(rb_pid_t pid, int *st, int fl https://github.com/ruby/ruby/blob/trunk/process.c#L916
 #endif
 }
 
+#define WAITPID_LOCK_ONLY ((struct waitpid_state *)-1)
+
 struct waitpid_state {
     struct list_node wnode;
     rb_execution_context_t *ec;
@@ -3923,12 +3925,13 @@ retry_fork_async_signal_safe(int *status https://github.com/ruby/ruby/blob/trunk/process.c#L3925
     volatile int try_gc = 1;
     struct child_handler_disabler_state old;
     int err;
+    rb_vm_t *vm = w && WAITPID_USE_SIGCHLD ? GET_VM() : 0;
 
     while (1) {
         prefork();
         disable_child_handler_before_fork(&old);
-        if (w && WAITPID_USE_SIGCHLD) {
-            rb_native_mutex_lock(&rb_ec_vm_ptr(w->ec)->waitpid_lock);
+        if (vm) {
+            rb_native_mutex_lock(&vm->waitpid_lock);
         }
 #ifdef HAVE_WORKING_VFORK
         if (!has_privilege())
@@ -3954,12 +3957,12 @@ retry_fork_async_signal_safe(int *status https://github.com/ruby/ruby/blob/trunk/process.c#L3957
 #endif
         }
 	err = errno;
-        if (w && WAITPID_USE_SIGCHLD) {
-            if (pid > 0) {
+        if (vm) {
+            if (pid > 0 && w != WAITPID_LOCK_ONLY) {
                 w->pid = pid;
-                list_add(&rb_ec_vm_ptr(w->ec)->waiting_pids, &w->wnode);
+                list_add(&vm->waiting_pids, &w->wnode);
             }
-            rb_native_mutex_unlock(&rb_ec_vm_ptr(w->ec)->waitpid_lock);
+            rb_native_mutex_unlock(&vm->waitpid_lock);
         }
 	disable_child_handler_fork_parent(&old);
         if (0 < pid) /* fork succeed, parent process */
@@ -3995,7 +3998,8 @@ fork_check_err(int *status, int (*chfunc https://github.com/ruby/ruby/blob/trunk/process.c#L3998
     error_occurred = recv_child_error(ep[0], &err, errmsg, errmsg_buflen);
     if (error_occurred) {
         if (status) {
-            VM_ASSERT(w == 0 && "only used by extensions");
+            VM_ASSERT((w == 0 || w == WAITPID_LOCK_ONLY) &&
+                      "only used by extensions");
             rb_protect(proc_syswait, (VALUE)pid, status);
         }
         else if (!w) {
@@ -4340,7 +4344,7 @@ rb_spawn_process(struct rb_execarg *earg https://github.com/ruby/ruby/blob/trunk/process.c#L4344
     rb_last_status_set((status & 0xff) << 8, 0);
     pid = 1;			/* dummy */
 # endif
-    if (eargp->waitpid_state) {
+    if (eargp->waitpid_state && eargp->waitpid_state != WAITPID_LOCK_ONLY) {
         eargp->waitpid_state->pid = pid;
     }
     rb_execarg_run_options(&sarg, NULL, errmsg, errmsg_buflen);
@@ -4369,6 +4373,15 @@ static rb_pid_t https://github.com/ruby/ruby/blob/trunk/process.c#L4373
 rb_execarg_spawn(VALUE execarg_obj, char *errmsg, size_t errmsg_buflen)
 {
     struct spawn_args args;
+    struct rb_execarg *eargp = rb_execarg_get(execarg_obj);
+
+    /*
+     * Prevent a race with MJIT where the compiler process where
+     * can hold an FD of ours in between vfork + execve
+     */
+    if (!eargp->waitpid_state && mjit_enabled) {
+        eargp->waitpid_state = WAITPID_LOCK_ONLY;
+    }
 
     args.execarg = execarg_obj;
     args.errmsg.ptr = errmsg;
Index: test/ruby/test_process.rb
===================================================================
--- test/ruby/test_process.rb	(revision 66170)
+++ test/ruby/test_process.rb	(revision 66171)
@@ -343,11 +343,6 @@ class TestProcess < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_process.rb#L343
   end
 
   def test_execopt_env_path
-    # http://ci.rvm.jp/results/trunk-mjit@silicon-docker/1455223
-    # http://ci.rvm.jp/results/trunk-mjit@silicon-docker/1450027
-    # http://ci.rvm.jp/results/trunk-mjit@silicon-docker/1469867
-    skip 'this randomly fails with MJIT' if RubyVM::MJIT.enabled?
-
     bug8004 = '[ruby-core:53103] [Bug #8004]'
     Dir.mktmpdir do |d|
       open("#{d}/tmp_script.cmd", "w") {|f| f.puts ": ;"; f.chmod(0755)}

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