Project

General

Profile

Bug #14968

[PATCH] io.c: make all pipes nonblocking by default

Added by normalperson (Eric Wong) 3 months ago. Updated 2 months ago.

Status:
Open
Priority:
Normal
Assignee:
-
Target version:
-
[ruby-core:88305]

Description

Crap, I always planned to have something like this for Feature #13618; but introducing a race condition for Timer-thread
elimination in [Misc #14937] forced me to introduce this early.

Anyways, I might have to revert and reintroduce timer-thread if this
change is unnacceptable :< (I HATE timer thread)

io.c: make all pipes nonblocking by default

All normal Ruby IO methods (IO#read, IO#gets, IO#write, ...) are
all capable of appearing to be "blocking" when presented with a
file description with the O_NONBLOCK flag set; so there is
little risk of incompatibility within Ruby-using programs.

The biggest compatibility risk is when spawning external
programs.  As a result, stdin, stdout, and stderr are now always
made blocking before exec-family calls.

Timer-thread elimination in https://bugs.ruby-lang.org/issues/14937
introduced a race condition in signal handling.  It is possible
to receive a signal inside BLOCKING_REGION right before read/write
syscalls.  If this patch cannot be accepted, I will have to revert
to reintroduce timer-thread and increase resource use (which
led to other failures in the past).  The race condition
introduced for [Misc #14937] led to rare CI failures on a few
tests:

- test/ruby/test_thread.rb (test_thread_timer_and_interrupt):
  http://www.rubyist.net/~akr/chkbuild/debian/ruby-trunk/log/20180805T080500Z.fail.html.gz

- test/ruby/test_io.rb (test_race_gets_and_close):
  http://ci.rvm.jp/results/trunk@P895/1190369

This change is ALSO necessary to take advantage of (proposed
lightweight concurrency (aka "auto-Fiber") or any similar
proposal: https://bugs.ruby-lang.org/issues/13618

TODO: all sockets and FIFOs non-blocking by default, too

History

#1 [ruby-core:88309] Updated by normalperson (Eric Wong) 3 months ago

Bug #14968: [PATCH] io.c: make all pipes nonblocking by default
https://bugs.ruby-lang.org/issues/14968

Updated patch on top of r64203 since I restored timer-thread :<

https://80x24.org/spew/20180806053511.2421-1-e@80x24.org/raw

#2 [ruby-core:88314] Updated by normalperson (Eric Wong) 2 months ago

https://bugs.ruby-lang.org/issues/14968

Updated patch on top of r64203 since I restored timer-thread :<

https://80x24.org/spew/20180806053511.2421-1-e@80x24.org/raw

Greg, btw, does the above patch work on Windows? In fact, it
seems possible to implement io/nonblock for Windows if
the non-blocking flag can be cleared successfully on sockets and
pipes. I don't know why nobody did it before, though.

#3 [ruby-core:88315] Updated by MSP-Greg (Greg L) 2 months ago

normalperson (Eric Wong)

The patch was fine, but the Appveyor build stopped.

Earlier, r64206 built fine, and I added the patch, which also built on that.

The build log is at
https://ci.appveyor.com/api/buildjobs/d0s8pbatmb7ax667/log

I can attach it if that would be helpful. Thanks, Greg

#4 [ruby-core:88317] Updated by normalperson (Eric Wong) 2 months ago

Greg.mpls@gmail.com wrote:

The build log is at
https://ci.appveyor.com/api/buildjobs/d0s8pbatmb7ax667/log

I can attach it if that would be helpful. Thanks, Greg

Thanks, seeing the "ASYNC BUG" and EACCESS was enough.
I removed the error checking (not important in that code
path) and did some minor cleanups:

https://80x24.org/spew/20180806231256.14039-1-e@80x24.org/raw

#5 [ruby-core:88319] Updated by MSP-Greg (Greg L) 2 months ago

normalperson (Eric Wong) Eric,

The zip of all the logs is at

https://ci.appveyor.com/api/buildjobs/pwyhyvkao2nssxy1/artifacts/zlogs_trunk_2018-08-07_64208.7z

All of the test-all failures show something similar to:

[Errno::ENOENT] exception expected, not.
Class: <Errno::EBADF>

The test-spec failures are similar. Test summary below:

13 Total Failures/Errors                           Build No 1032    Job Id pwyhyvkao2nssxy1
   ruby 2.6.0dev (2018-08-07 trunk 64208) [x64-mingw32]
   2018-08-07 02:29:51 UTC

test-all  19301 tests, 2250081 assertions, 5 failures, 0 errors, 107 skips, 107 skips shown

test-spec  3690 files, 28734 examples, 210530 expectations, 2 failures, 4 errors, 0 tagged
mspec      3690 files, 28736 examples, 210420 expectations, 2 failures, 4 errors, 0 tagged

test-basic test succeeded
btest      PASS all 1392 tests

#6 [ruby-core:88320] Updated by normalperson (Eric Wong) 2 months ago

Greg.mpls@gmail.com wrote:

All of the test-all failures show something similar to:

[Errno::ENOENT] exception expected, not.
Class: <Errno::EBADF>

Thanks Greg. The following patch (on top of my previous one) might
fix it, but it's a bit dirty. No rush to try it.

 --- a/process.c
 +++ b/process.c
 @@ -3454,7 +3454,11 @@ rb_execarg_run_options(const struct rb_execarg *eargp, struct rb_execarg *sargp,
 rb_execarg_allocate_dup2_tmpbuf(sargp, RARRAY_LEN(ary));
 }
 }
 -    stdfd_clear_nonblock();
 +    {
 +        int preserve = errno;
 +        stdfd_clear_nonblock();
 +        errno = preserve;
 +    }

 return 0;
 }

13 Total Failures/Errors Build No 1032 Job Id pwyhyvkao2nssxy1

Anyways, it doesn't seem like the problems from moving from
blocking to non-blocking pipes by default are insurmountable
and it's another step towards getting lightweight concurrency
into Ruby.

#7 [ruby-core:88377] Updated by usa (Usaku NAKAMURA) 2 months ago

BTW, Windows does not have nonblocking pipe.
Yes, we may be able to write emulation layer with overlapped I/O functions, but it's very difficult.

#8 [ruby-core:88379] Updated by normalperson (Eric Wong) 2 months ago

usa@garbagecollect.jp wrote:

BTW, Windows does not have nonblocking pipe.

Uh oh, really? I saw rb_w32_set_nonblock() has is_pipe() check
and {Get,Set}NamedPipeHandleState calls, already, so that's
something else?

Yes, we may be able to write emulation layer with overlapped
I/O functions, but it's very difficult.

Maybe it's not a big deal to not have non-blocking pipes.

Does win32 have TOCTTOU race in signal handling? All *nix
has this problem, but maybe it's a *nix-only problem.
I see you don't have ubf_list like thread_pthread.c has.

Thanks.

#9 [ruby-core:88382] Updated by usa (Usaku NAKAMURA) 2 months ago

Uh oh, really? I saw rb_w32_set_nonblock() has is_pipe() check
and {Get,Set}NamedPipeHandleState calls, already, so that's
something else?

  1. A pipe may be a named pipe, or an anonymous pipe :)
  2. The code is not tested.
  3. Microsoft says that "Note that nonblocking mode ... should not be used to achieve asynchronous input and output (I/O) with named pipes."

Does win32 have TOCTTOU race in signal handling?

Maybe. But I should note that Windows does not have real signals :P

#10 [ruby-core:88405] Updated by shyouhei (Shyouhei Urabe) 2 months ago

I guess you are reinventing libuv?
It claims to support Windows.
I don't say we should use it but it might be worth looking at.

#11 [ruby-core:88410] Updated by normalperson (Eric Wong) 2 months ago

shyouhei@ruby-lang.org wrote:

I guess you are reinventing libuv?
It claims to support Windows.

I guess usa can take code from that project (MIT license)
for portability.

I'm not sure if non-blocking pipes makes much of a difference
for Windows since it doesn't have real signals; and most
Ruby API are still synchronous to users.

I don't say we should use it but it might be worth looking at.

Right; event loop design pattern doesn't map to our VM.

Also available in: Atom PDF