Bug #8770

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

Added by Eric Wong 8 months ago. Updated about 1 month ago.

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

Description

(sendchilderror): retry write on EINTR
(recvchilderror): 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 3 months ago

  • process.c (READFROMCHILD): 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 8 months ago

patch please

#3 Updated by Yui NARUSE 7 months ago

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

#4 Updated by Eric Wong 3 months 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 3 months ago

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

Oops, I missed to include the reference in r44706.

#6 Updated by Eric Wong 3 months 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 3 months 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#testdeadlockbysignalatforking:
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 testdeadlockbysignalatforking'
/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
testdeadlockbysignalat_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 3 months ago

I think the Errno::EINTR error occur inTestProcess#testdeadlockbysignalat_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 3 months ago

akr@fsij.org wrote:

I think the Errno::EINTR error occur inTestProcess#testdeadlockbysignalat_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 2 months 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 month ago

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

#13 Updated by Yui NARUSE about 1 month 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 month 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 ruby21

Also available in: Atom PDF