ruby-changes:51641
From: naruse <ko1@a...>
Date: Thu, 5 Jul 2018 00:09:04 +0900 (JST)
Subject: [ruby-changes:51641] naruse:r63852 (trunk): Revert r63758 and related commits
naruse 2018-07-05 00:08:56 +0900 (Thu, 05 Jul 2018) New Revision: 63852 https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=63852 Log: Revert r63758 and related commits The change is unstable on Windows. Please re-commit it when it correctly supports Windows. Modified files: trunk/configure.ac trunk/ext/pty/pty.c trunk/internal.h trunk/mjit.c trunk/process.c trunk/signal.c trunk/spec/ruby/core/process/wait2_spec.rb trunk/spec/ruby/core/process/wait_spec.rb trunk/spec/ruby/core/process/waitall_spec.rb trunk/test/ruby/test_optimization.rb trunk/test/ruby/test_process.rb trunk/test/ruby/test_rubyoptions.rb trunk/thread.c trunk/thread_pthread.c trunk/vm_core.h trunk/win32/Makefile.sub Index: thread.c =================================================================== --- thread.c (revision 63851) +++ thread.c (revision 63852) @@ -413,10 +413,6 @@ rb_vm_gvl_destroy(rb_vm_t *vm) https://github.com/ruby/ruby/blob/trunk/thread.c#L413 gvl_release(vm); gvl_destroy(vm); rb_native_mutex_destroy(&vm->thread_destruct_lock); - if (0) { - /* may be held by running threads */ - rb_native_mutex_destroy(&vm->waitpid_lock); - } } void @@ -4135,9 +4131,6 @@ rb_gc_set_stack_end(VALUE **stack_end_p) https://github.com/ruby/ruby/blob/trunk/thread.c#L4131 #endif -/* signal.c */ -void ruby_sigchld_handler(rb_vm_t *); - /* * */ @@ -4170,7 +4163,6 @@ timer_thread_function(void *arg) https://github.com/ruby/ruby/blob/trunk/thread.c#L4163 rb_native_mutex_unlock(&vm->thread_destruct_lock); /* check signal */ - ruby_sigchld_handler(vm); rb_threadptr_check_signal(vm->main_thread); #if 0 @@ -4255,9 +4247,6 @@ rb_thread_atfork_internal(rb_thread_t *t https://github.com/ruby/ruby/blob/trunk/thread.c#L4247 } rb_vm_living_threads_init(vm); rb_vm_living_threads_insert(vm, th); - - /* may be held by MJIT threads in parent */ - rb_native_mutex_initialize(&vm->waitpid_lock); vm->fork_gen++; vm->sleeper = 0; @@ -5010,7 +4999,6 @@ Init_Thread(void) https://github.com/ruby/ruby/blob/trunk/thread.c#L4999 gvl_init(th->vm); gvl_acquire(th->vm, th); rb_native_mutex_initialize(&th->vm->thread_destruct_lock); - rb_native_mutex_initialize(&th->vm->waitpid_lock); rb_native_mutex_initialize(&th->interrupt_lock); th->pending_interrupt_queue = rb_ary_tmp_new(0); @@ -5314,25 +5302,3 @@ rb_uninterruptible(VALUE (*b_proc)(ANYAR https://github.com/ruby/ruby/blob/trunk/thread.c#L5302 return rb_ensure(b_proc, data, rb_ary_pop, cur_th->pending_interrupt_mask_stack); } - -#ifndef USE_NATIVE_SLEEP_COND -# define USE_NATIVE_SLEEP_COND (0) -#endif - -#if !USE_NATIVE_SLEEP_COND -rb_nativethread_cond_t * -rb_sleep_cond_get(const rb_execution_context_t *ec) -{ - rb_nativethread_cond_t *cond = ALLOC(rb_nativethread_cond_t); - rb_native_cond_initialize(cond); - - return cond; -} - -void -rb_sleep_cond_put(rb_nativethread_cond_t *cond) -{ - rb_native_cond_destroy(cond); - xfree(cond); -} -#endif /* !USE_NATIVE_SLEEP_COND */ Index: spec/ruby/core/process/wait2_spec.rb =================================================================== --- spec/ruby/core/process/wait2_spec.rb (revision 63851) +++ spec/ruby/core/process/wait2_spec.rb (revision 63852) @@ -4,37 +4,31 @@ describe "Process.wait2" do https://github.com/ruby/ruby/blob/trunk/spec/ruby/core/process/wait2_spec.rb#L4 before :all do # HACK: this kludge is temporarily necessary because some # misbehaving spec somewhere else does not clear processes - # Note: background processes are unavoidable with MJIT, - # but we shouldn't reap them from Ruby-space begin Process.wait(-1, Process::WNOHANG) - without_feature :mjit do - $stderr.puts "Leaked process before wait2 specs! Waiting for it" - end + $stderr.puts "Leaked process before wait2 specs! Waiting for it" leaked = Process.waitall - $stderr.puts "leaked before wait2 specs: #{leaked}" unless leaked.empty? - with_feature :mjit do - # Ruby-space should not see PIDs used by mjit - leaked.should be_empty - end + $stderr.puts "leaked before wait2 specs: #{leaked}" rescue Errno::ECHILD # No child processes rescue NotImplementedError end end - platform_is_not :windows do - it "returns the pid and status of child process" do - pidf = Process.fork { Process.exit! 99 } - results = Process.wait2 - results.size.should == 2 - pidw, status = results - pidf.should == pidw - status.exitstatus.should == 99 + without_feature :mjit do # [Bug #14867] + platform_is_not :windows do + it "returns the pid and status of child process" do + pidf = Process.fork { Process.exit! 99 } + results = Process.wait2 + results.size.should == 2 + pidw, status = results + pidf.should == pidw + status.exitstatus.should == 99 + end end - end - it "raises a StandardError if no child processes exist" do - lambda { Process.wait2 }.should raise_error(Errno::ECHILD) - lambda { Process.wait2 }.should raise_error(StandardError) + it "raises a StandardError if no child processes exist" do + lambda { Process.wait2 }.should raise_error(Errno::ECHILD) + lambda { Process.wait2 }.should raise_error(StandardError) + end end end Index: spec/ruby/core/process/waitall_spec.rb =================================================================== --- spec/ruby/core/process/waitall_spec.rb (revision 63851) +++ spec/ruby/core/process/waitall_spec.rb (revision 63852) @@ -8,41 +8,43 @@ describe "Process.waitall" do https://github.com/ruby/ruby/blob/trunk/spec/ruby/core/process/waitall_spec.rb#L8 end end - it "returns an empty array when there are no children" do - Process.waitall.should == [] - end - - it "takes no arguments" do - lambda { Process.waitall(0) }.should raise_error(ArgumentError) - end + without_feature :mjit do # [Bug #14867] + it "returns an empty array when there are no children" do + Process.waitall.should == [] + end - platform_is_not :windows do - it "waits for all children" do - pids = [] - pids << Process.fork { Process.exit! 2 } - pids << Process.fork { Process.exit! 1 } - pids << Process.fork { Process.exit! 0 } - Process.waitall - pids.each { |pid| - lambda { Process.kill(0, pid) }.should raise_error(Errno::ESRCH) - } + it "takes no arguments" do + lambda { Process.waitall(0) }.should raise_error(ArgumentError) end - it "returns an array of pid/status pairs" do - pids = [] - pids << Process.fork { Process.exit! 2 } - pids << Process.fork { Process.exit! 1 } - pids << Process.fork { Process.exit! 0 } - a = Process.waitall - a.should be_kind_of(Array) - a.size.should == 3 - pids.each { |pid| - pid_status = a.assoc(pid) - pid_status.should be_kind_of(Array) - pid_status.size.should == 2 - pid_status.first.should == pid - pid_status.last.should be_kind_of(Process::Status) - } + platform_is_not :windows do + it "waits for all children" do + pids = [] + pids << Process.fork { Process.exit! 2 } + pids << Process.fork { Process.exit! 1 } + pids << Process.fork { Process.exit! 0 } + Process.waitall + pids.each { |pid| + lambda { Process.kill(0, pid) }.should raise_error(Errno::ESRCH) + } + end + + it "returns an array of pid/status pairs" do + pids = [] + pids << Process.fork { Process.exit! 2 } + pids << Process.fork { Process.exit! 1 } + pids << Process.fork { Process.exit! 0 } + a = Process.waitall + a.should be_kind_of(Array) + a.size.should == 3 + pids.each { |pid| + pid_status = a.assoc(pid) + pid_status.should be_kind_of(Array) + pid_status.size.should == 2 + pid_status.first.should == pid + pid_status.last.should be_kind_of(Process::Status) + } + end end end end Index: spec/ruby/core/process/wait_spec.rb =================================================================== --- spec/ruby/core/process/wait_spec.rb (revision 63851) +++ spec/ruby/core/process/wait_spec.rb (revision 63852) @@ -8,87 +8,85 @@ describe "Process.wait" do https://github.com/ruby/ruby/blob/trunk/spec/ruby/core/process/wait_spec.rb#L8 begin leaked = Process.waitall puts "leaked before wait specs: #{leaked}" unless leaked.empty? - with_feature :mjit do - # Ruby-space should not see PIDs used by mjit - leaked.should be_empty - end rescue NotImplementedError end end - it "raises an Errno::ECHILD if there are no child processes" do - lambda { Process.wait }.should raise_error(Errno::ECHILD) - end - - platform_is_not :windows do - it "returns its childs pid" do - pid = Process.spawn(ruby_cmd('exit')) - Process.wait.should == pid + without_feature :mjit do # [Bug #14867] + it "raises an Errno::ECHILD if there are no child processes" do + lambda { Process.wait }.should raise_error(Errno::ECHILD) end - it "sets $? to a Process::Status" do - pid = Process.spawn(ruby_cmd('exit')) - Process.wait - $?.should be_kind_of(Process::Status) - $?.pid.should == pid - end + platform_is_not :windows do + it "returns its childs pid" do + pid = Process.spawn(ruby_cmd('exit')) + Process.wait.should == pid + end - it "waits for any child process if no pid is given" do - pid = Process.spawn(ruby_cmd('exit')) - Process.wait.should == pid - lambda { Process.kill(0, pid) }.should raise_error(Errno::ESRCH) - end + it "sets $? to a Process::Status" do + pid = Process.spawn(ruby_cmd('exit')) + Process.wait + $?.should be_kind_of(Process::Status) + $?.pid.should == pid + end - it "waits for a specific child if a pid is given" do - pid1 = Process.spawn(ruby_cmd('exit')) - pid2 = Process.spawn(ruby_cmd('exit')) - Process.wait(pid2).should == pid2 - Process.wait(pid1).should == pid1 - lambda { Process.kill(0, pid1) }.should raise_error(Errno::ESRCH) - lambda { Process.kill(0, pid2) }.should raise_error(Errno::ESRCH) - end + it "waits for any child process if no pid is given" do + pid = Process.spawn(ruby_cmd('exit')) + Process.wait.should == pid + lambda { Process.kill(0, pid) }.should raise_error(Errno::ESRCH) + end - it "coerces the pid to an Integer" do - pid1 = Process.spawn(ruby_cmd('exit')) - Process.wait(mock_int(pid1)).should == pid1 - lambda { Process.kill(0, pid1) }.should raise_error(Errno::ESRCH) - end + it "waits for a specific child if a pid is given" do + pid1 = Process.spawn(ruby_cmd('exit')) + pid2 = Process.spawn(ruby_cmd('exit')) + Process.wait(pid2).should == pid2 + Process.wait(pid1).should == pid1 + lambda { Process.kill(0, pid1) }.should raise_error(Errno::ESRCH) + lambda { Process.kill(0, pid2) }.should raise_error(Errno::ESRCH) + end - # This spec is probably system-dependent. - it "waits for a child whose process group ID is that of the calling process" do - pid1 = Process.spawn(ruby_cmd('exit'), pgroup: true) - pid2 = Process.spawn(ruby_cmd('exit')) + it "coerces the pid to an Integer" do + pid1 = Process.spawn(ruby_cmd('exit')) + Process.wait(mock_int(pid1)).should == pid1 + lambda { Process.kill(0, pid1) }.should raise_error(Errno::ESRCH) + end - Process.wait(0).should == pid2 - Process.wait.should == pid1 - end + # This spec is probably system-dependent. + it "waits for a child whose process group ID is that of the calling process" do + pid1 = Process.spawn(ruby_cmd('exit'), pgroup: true) + pid2 = Process.spawn(ruby_cmd('exit')) - # This spec is probably system-dependent. - it "doesn't block if no child is available when WNOHANG is used" do - read, write = IO.pipe - pid = Process.fork do - read.close - Signal.trap("TERM") { Process.exit! } - write << 1 - write.close - sleep + Process.wait(0).should == pid2 + Process.wait.should == pid1 end - Process.wait(pid, Process::WNOHANG).should be_nil + # This spec is probably system-dependent. + it "doesn't block if no child is available when WNOHANG is used" do + read, write = IO.pipe + pid = Process.fork do + read.close + Signal.trap("TERM") { Process.exit! } + write << 1 + write.close + sleep + end - # wait for the child to setup its TERM handler - write.close - read.read(1) - read.close + Process.wait(pid, Process::WNOHANG).should be_nil - Process.kill("TERM", pid) - Process.wait.should == pid - end + # wait for the child to setup its TERM handler + write.close + read.read(1) + read.close - it "always accepts flags=0" do - pid = Process.spawn(ruby_cmd('exit')) - Process.wait(-1, 0).should == pid - lambda { Process.kill(0, pid) }.should raise_error(Errno::ESRCH) + Process.kill("TERM", pid) + Process.wait.should == pid + end + + it "always accepts flags=0" do + pid = Process.spawn(ruby_cmd('exit')) + Process.wait(-1, 0).should == pid + lambda { Process.kill(0, pid) }.should raise_error(Errno::ESRCH) + end end end end Index: mjit.c =================================================================== --- mjit.c (revision 63851) +++ mjit.c (revision 63852) @@ -80,7 +80,6 @@ https://github.com/ruby/ruby/blob/trunk/mjit.c#L80 #include "constant.h" #include "id_table.h" #include "ruby_assert.h" -#include "ruby/thread.h" #include "ruby/util.h" #include "ruby/version.h" @@ -119,10 +118,6 @@ extern void rb_native_cond_wait(rb_nativ https://github.com/ruby/ruby/blob/trunk/mjit.c#L118 extern int rb_thread_create_mjit_thread(void (*child_hook)(void), void (*worker_func)(void)); -/* process.c */ -rb_pid_t ruby_waitpid_locked(rb_vm_t *, rb_pid_t, int *status, int options, - rb_nativethread_cond_t *cond); - #define RB_CONDATTR_CLOCK_MONOTONIC 1 #ifdef _WIN32 @@ -268,7 +263,7 @@ real_ms_time(void) https://github.com/ruby/ruby/blob/trunk/mjit.c#L263 static int sprint_uniq_filename(char *str, size_t size, unsigned long id, const char *prefix, const char *suffix) { - return snprintf(str, size, "%s/%sp%"PRI_PIDT_PREFIX"uu%lu%s", tmp_dir, prefix, getpid(), id, suffix); + return snprintf(str, size, "%s/%sp%luu%lu%s", tmp_dir, prefix, (unsigned long) getpid(), id, suffix); } /* Return an unique file name in /tmp with PREFIX and SUFFIX and @@ -406,42 +401,23 @@ start_process(const char *path, char *co https://github.com/ruby/ruby/blob/trunk/mjit.c#L401 static int exec_process(const char *path, char *const argv[]) { - int stat, exit_code = -2; + int stat, exit_code; pid_t pid; - rb_vm_t *vm = (RUBY_SIGCHLD || SIGCHLD_LOSSY) ? GET_VM() : 0; - rb_nativethread_cond_t cond; - - if (vm) { - rb_native_cond_initialize(&cond); - rb_native_mutex_lock(&vm->waitpid_lock); - } pid = start_process(path, argv); - for (;pid > 0;) { - pid_t r = vm ? ruby_waitpid_locked(vm, pid, &stat, 0, &cond) - : waitpid(pid, &stat, 0); - if (r == -1) { - if (errno == EINTR) continue; - fprintf(stderr, "[%"PRI_PIDT_PREFIX"d] waitpid(%"PRI_PIDT_PREFIX"d): %s (SIGCHLD=%d,%u)\n", - getpid(), pid, strerror(errno), - RUBY_SIGCHLD, SIGCHLD_LOSSY); + if (pid <= 0) + return -2; + + for (;;) { + waitpid(pid, &stat, 0); + if (WIFEXITED(stat)) { + exit_code = WEXITSTATUS(stat); + break; + } else if (WIFSIGNALED(stat)) { + exit_code = -1; break; - } - else if (r == pid) { - if (WIFEXITED(stat)) { - exit_code = WEXITSTATUS(stat); - break; - } else if (WIFSIGNALED(stat)) { - exit_code = -1; - break; - } } } - - if (vm) { - rb_native_mutex_unlock(&vm->waitpid_lock); - rb_native_cond_destroy(&cond); - } return exit_code; } @@ -1515,15 +1491,12 @@ mjit_init(struct mjit_options *opts) https://github.com/ruby/ruby/blob/trunk/mjit.c#L1491 static void stop_worker(void) { - rb_execution_context_t *ec = GET_EC(); - stop_worker_p = TRUE; while (!worker_stopped) { verbose(3, "Sending cancel signal to worker"); CRITICAL_SECTION_START(3, "in stop_worker"); rb_native_cond_broadcast(&mjit_worker_wakeup); CRITICAL_SECTION_FINISH(3, "in stop_worker"); - RUBY_VM_CHECK_INTS(ec); } } Index: internal.h =================================================================== --- internal.h (revision 63851) +++ internal.h (revision 63852) @@ -2042,9 +2042,6 @@ VALUE rb_gcd_normal(VALUE self, VALUE ot https://github.com/ruby/ruby/blob/trunk/internal.h#L2042 VALUE rb_gcd_gmp(VALUE x, VALUE y); #endif -/* signal.c (export) */ -int rb_grantpt(int fd); - /* string.c (export) */ #ifdef RUBY_ENCODING_H /* internal use */ Index: vm_core.h =================================================================== --- vm_core.h (revision 63851) +++ vm_core.h (revision 63852) @@ -92,21 +92,6 @@ https://github.com/ruby/ruby/blob/trunk/vm_core.h#L92 #define RUBY_NSIG NSIG -#if defined(SIGCLD) -# define RUBY_SIGCHLD (SIGCLD) -#elif defined(SIGCHLD) -# define RUBY_SIGCHLD (SIGCHLD) -#else -# define RUBY_SIGCHLD (0) -#endif - -/* platforms with broken or non-existent SIGCHLD work by polling */ -#if defined(__APPLE__) || defined(__WIN32__) || defined(_WIN32) -# define SIGCHLD_LOSSY (1) -#else -# define SIGCHLD_LOSSY (0) -#endif - #ifdef HAVE_STDARG_PROTOTYPES #include <stdarg.h> #define va_init_list(a,b) va_start((a),(b)) @@ -568,9 +553,6 @@ typedef struct rb_vm_struct { https://github.com/ruby/ruby/blob/trunk/vm_core.h#L553 #endif rb_serial_t fork_gen; - rb_nativethread_lock_t waitpid_lock; - struct list_head waiting_pids; /* PID > 0: <=> struct waitpid_state */ - struct list_head waiting_grps; /* PID <= 0: <=> struct waitpid_state */ struct list_head waiting_fds; /* <=> struct waiting_fd */ struct list_head living_threads; VALUE thgroup_default; @@ -1579,8 +1561,6 @@ static inline void https://github.com/ruby/ruby/blob/trunk/vm_core.h#L1561 rb_vm_living_threads_init(rb_vm_t *vm) { list_head_init(&vm->waiting_fds); - list_head_init(&vm->waiting_pids); - list_head_init(&vm->waiting_grps); list_head_init(&vm->living_threads); vm->living_thread_num = 0; } Index: test/ruby/test_optimization.rb =================================================================== --- test/ruby/test_optimization.rb (revision 63851) +++ test/ruby/test_optimization.rb (revision 63852) @@ -707,7 +707,7 @@ class TestRubyOptimization < Test::Unit: https://github.com/ruby/ruby/blob/trunk/test/ruby/test_optimization.rb#L707 end def test_clear_unreachable_keyword_args - assert_separately [], <<-END, timeout: 30 + assert_separately [], <<-END, timeout: 20 script = <<-EOS if true else Index: test/ruby/test_process.rb =================================================================== --- test/ruby/test_process.rb (revision 63851) +++ test/ruby/test_process.rb (revision 63852) @@ -1426,6 +1426,7 @@ class TestProcess < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_process.rb#L1426 end def test_wait_without_arg + skip "[Bug #14867]" if RubyVM::MJIT.enabled? with_tmpchdir do write_file("foo", "sleep 0.1") pid = spawn(RUBY, "foo") @@ -1434,6 +1435,7 @@ class TestProcess < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_process.rb#L1435 end def test_wait2 + skip "[Bug #14867]" if RubyVM::MJIT.enabled? with_tmpchdir do write_file("foo", "sleep 0.1") pid = spawn(RUBY, "foo") @@ -1442,6 +1444,7 @@ class TestProcess < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_process.rb#L1444 end (... truncated) -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/