Bug #8770

[PATCH] process.c: avoid EINTR from Process.spawn

Added by Eric Wong over 1 year ago. Updated about 1 year ago.

[ruby-core:56524]
Status:Assigned
Priority:Normal
Assignee:Nobuyoshi Nakada
ruby -v:44687,44706,44727 Backport:1.9.3: REQUIRED, 2.0.0: REQUIRED, 2.1: DONE

Description

(send_child_error): retry write on EINTR
(recv_child_error): retry read on EINTR

I've been getting occasional Errno::EINTR from Process.spawn with
just the prog name in the error message. This is probably the cause
of it, as I haven't been able to get a consistent reproduction of
the Errno::EINTR.

I've been hitting this on 2.0.0-p247, so it probably needs backport.

0001-process.c-avoid-EINTR-from-Process.spawn.patch Magnifier - avoid EINTR from Process.spawn (3.18 KB) Eric Wong, 08/11/2013 11:39 AM

Associated revisions

Revision 44727
Added by Akira Tanaka about 1 year ago

  • process.c (READ_FROM_CHILD): Apply the last hunk of 0001-process.c-avoid-EINTR-from-Process.spawn.patch written by Eric Wong in [Bug #8770].

Revision 44727
Added by Akira Tanaka about 1 year ago

  • process.c (READ_FROM_CHILD): Apply the last hunk of 0001-process.c-avoid-EINTR-from-Process.spawn.patch written by Eric Wong in [Bug #8770].

History

#1 Updated by Nobuyoshi Nakada over 1 year ago

patch please

#3 Updated by Yui NARUSE over 1 year ago

  • Status changed from Open to Assigned
  • Assignee set to Nobuyoshi Nakada

#4 Updated by Eric Wong about 1 year ago

It looks like r44687 implements a partial fix for this, but my
proposed patch covers more cases and is more complete.

* process.c (recv_child_error): Fix deadlock in rb_fork_internal when a
  signal is sent to the parent process while Ruby is forking in IO.popen.

 Patch by Scott Francis. Closes GH-513.

#5 Updated by Nobuyoshi Nakada about 1 year ago

  • Backport changed from 1.9.3: UNKNOWN, 2.0.0: UNKNOWN to 1.9.3: REQUIRED, 2.0.0: REQUIRED, 2.1: REQUIRED
  • Status changed from Assigned to Closed

Oops, I missed to include the reference in r44706.

#6 Updated by Eric Wong about 1 year ago

nobu@ruby-lang.org wrote:

Oops, I missed to include the reference in r44706.

No worries. Interesting, now I notice test_process now fails under
heavy load with this patch...

This should fix things: http://bogomips.org/ruby.git/patch?id=81b6f37afe34

git://80x24.org/ruby.git test_process-avoid-teardown

#8 Updated by Eric Wong about 1 year ago

nobu@ruby-lang.org wrote:

Is it an issue of the test code?

http://c5664.rubyci.org/~chkbuild/ruby-trunk/log/20140125T233302Z.log.html.gz

Odd, I did not get this:

10) Error:
TestProcess#test_deadlock_by_signal_at_forking:
Errno::EINTR: Interrupted system call - /home/chkbuild/build/20140125T233302Z/ruby/ruby
/home/chkbuild/build/20140125T233302Z/ruby/test/ruby/test_process.rb:1875:in popen'
/home/chkbuild/build/20140125T233302Z/ruby/test/ruby/test_process.rb:1875:in
block in test_deadlock_by_signal_at_forking'
/home/chkbuild/build/20140125T233302Z/ruby/test/ruby/test_process.rb:1873:in times'
/home/chkbuild/build/20140125T233302Z/ruby/test/ruby/test_process.rb:1873:in
test_deadlock_by_signal_at_forking'

Instead, I got this (with TESTS=-j32):

/home/ew/ruby/test/ruby/test_process.rb:15:in waitall': SIGTERM (SignalException)
/home/ew/ruby/test/ruby/test_process.rb:15:in
teardown'
/home/ew/ruby/lib/minitest/unit.rb:1280:in `block in run'

I don't think I was able to reproduce it on a single-threaded test-all.

#9 Updated by Akira Tanaka about 1 year ago

I think the Errno::EINTR error occur inTestProcess#test_deadlock_by_signal_at_forking because
r44706 doesn't contain the last hunk in 0001-process.c-avoid-EINTR-from-Process.spawn.patch.

I committed the hunk at r44727.

#10 Updated by Eric Wong about 1 year ago

akr@fsij.org wrote:

I think the Errno::EINTR error occur inTestProcess#test_deadlock_by_signal_at_forking because
r44706 doesn't contain the last hunk in 0001-process.c-avoid-EINTR-from-Process.spawn.patch.

I committed the hunk at r44727.

Good catch, things look good, thanks!

#11 Updated by Aman Gupta about 1 year ago

  • Tracker changed from Bug to Backport
  • Project changed from Ruby trunk to Backport21
  • Category deleted (core)
  • Status changed from Closed to Assigned
  • Target version deleted (2.1.0)

#12 Updated by Yui NARUSE about 1 year ago

  • Backport set to 1.9.3: UNKNOWN, 2.0.0: UNKNOWN, 2.1: UNKNOWN
  • ruby -v set to -
  • Project changed from Backport21 to Ruby trunk
  • Tracker changed from Backport to Bug

#13 Updated by Yui NARUSE about 1 year ago

  • ruby -v changed from - to 44687,44706,44727
  • Backport changed from 1.9.3: UNKNOWN, 2.0.0: UNKNOWN, 2.1: UNKNOWN to 1.9.3: REQUIRED, 2.0.0: REQUIRED, 2.1: REQUIRED

COMMITS: 44687,44706,44727

#14 Updated by Yui NARUSE about 1 year ago

  • Backport changed from 1.9.3: REQUIRED, 2.0.0: REQUIRED, 2.1: REQUIRED to 1.9.3: REQUIRED, 2.0.0: REQUIRED, 2.1: DONE

r45061 on ruby_2_1

Also available in: Atom PDF