Skip to content

Commit

Permalink
process.c: fix ETXTBUSY from MJIT compiler process
Browse files Browse the repository at this point in the history
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.

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@66171 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
  • Loading branch information
normal committed Dec 3, 2018
1 parent 5ce5335 commit a226434
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 13 deletions.
29 changes: 21 additions & 8 deletions process.c
Expand Up @@ -916,6 +916,8 @@ do_waitpid(rb_pid_t pid, int *st, int flags)
#endif
}

#define WAITPID_LOCK_ONLY ((struct waitpid_state *)-1)

struct waitpid_state {
struct list_node wnode;
rb_execution_context_t *ec;
Expand Down Expand Up @@ -3923,12 +3925,13 @@ retry_fork_async_signal_safe(int *status, int *ep,
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())
Expand All @@ -3954,12 +3957,12 @@ retry_fork_async_signal_safe(int *status, int *ep,
#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 */
Expand Down Expand Up @@ -3995,7 +3998,8 @@ fork_check_err(int *status, int (*chfunc)(void*, char *, size_t), void *charg,
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) {
Expand Down Expand Up @@ -4340,7 +4344,7 @@ rb_spawn_process(struct rb_execarg *eargp, char *errmsg, size_t errmsg_buflen)
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);
Expand Down Expand Up @@ -4369,6 +4373,15 @@ static rb_pid_t
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;
Expand Down
5 changes: 0 additions & 5 deletions test/ruby/test_process.rb
Expand Up @@ -343,11 +343,6 @@ def test_execopts_env
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)}
Expand Down

0 comments on commit a226434

Please sign in to comment.