Feature #18668: Merge `io-nonblock` gems into core

• Status: Open
• Assignee: -
• Target version: -

[Eregon (Benoit Daloze)]

The new io-nonblock gem defines just 3 methods on IO: (https://github.com/ruby/io-nonblock/blob/e20578b24405d225e6383d6c5ebfb7ffefac646b/ext/io/nonblock/nonblock.c#L137-L139)

• IO#nonblock?
• IO#nonblock=(nonblock)
• IO#nonblock(nonblock = true, &block)

I think these 3 methods should move to core, and the gem become a noop if these methods are already defined in core.
These methods are small and yet they access IO internals and their behavior is extremely unlikely to change.
Their behavior and names are well known and established.

The benefit of a gem seems nonexistent here (no point to version those 3 methods), while the cost is significant (have to support each Ruby implementation, while this code just makes more sense in each Ruby implementation's repo).

io/nonblock is useful to tell if an IO is in non-blocking mode and to set it upfront.
This is needed when using a Fiber scheduler but also other cases such as https://bugs.ruby-lang.org/issues/18630#note-9.

In fact io/nonblock is so small it's already core in TruffleRuby.
Many core IO methods even need to check/set whether an IO is nonblocking, so it's natural to just use the existing methods for that when such IO methods are written in Ruby.

No gem seems to depend on io-nonblock anyway, so it seems of no use to be a gem, and it should either be core or stdlib-not-a-gem:
https://github.com/ruby/io-nonblock/network/dependents
https://rubygems.org/gems/io-wait/reverse_dependencies

Proposal:

• Merge io-nonblock into io.c for Ruby 3.2
• Publish a new io-nonblock version that simply noops if the methods are already defined, or an empty gem (so the stdlib io/nonblock gets used).
• Add a lib/io/nonblock.rb stub for compatibility, with eventually a deprecation warning.

[Eregon (Benoit Daloze)]

• Related to Feature #18566: Merge `io-wait` and `io-nonblock` gems into core IO added

[shyouhei (Shyouhei Urabe)]

I don't think this gem deserves a room into core. Are there any practical usages of it? Note that IO#read_nonblock is an unrelated method (works with or without of this).

Also, are we going to add tons of trivial methods into core just because they are small? I guess we need good reasons for each of them to exist.

[Eregon (Benoit Daloze)]

Are there any practical usages of it?

I already mentioned them:

This is needed when using a Fiber scheduler but also other cases such as https://bugs.ruby-lang.org/issues/18630#note-9.

I believe one of the main use cases is to check whether an IO is in nonblocking mode (IO#nonblock?), and to set or ensure a specific IO instance is in nonblocking mode (IO#nonblock=, IO#nonblock {}).
This is essential for the Fiber scheduler and https://bugs.ruby-lang.org/issues/18630, they mostly don't work on IOs for which nonblock?=false.
However, such checking might be temporary or helpful for debugging but not necessarily kept in the code.
For instance https://github.com/ruby/ruby/blob/master/test/fiber/scheduler.rb requires it but does not seem to use the IO#nonblock* methods, but most likely they were used for debugging and finding out if everything is as expected.

Having *_nonblock methods but no way to observe whether it's in nonblocking mode in core feels like a missing piece.
We have IO#autoclose?, IO#binmode?, IO#close_on_exec? yet without require 'io/nonblock' we have no way to check if an IO is in nonblocking mode, even though we have an indirect way to set it (IO#*_nonblock).

You can also see it's used in several places with https://github.com/search?q=require+io%2Fnonblock&type=Code : 40000+ results

Also, are we going to add tons of trivial methods into core just because they are small? I guess we need good reasons for each of them to exist.

These methods already "exist", whether they are in core or not.
I don't understand why it makes such a big difference whether it's behind require 'io/nonblock' or not, these methods have always (or since so many years) been shipped with Ruby.

But if core devs feel strongly about it, I would also be fine for these methods to be in stdlib (as before the gem), and no longer a gem.
What is the advantage of having this in a gem, separate repository? I see none.

[shyouhei (Shyouhei Urabe)]

Eregon (Benoit Daloze) wrote in #note-3:

Are there any practical usages of it?

I already mentioned them:

This is needed when using a Fiber scheduler but also other cases such as https://bugs.ruby-lang.org/issues/18630#note-9.

And I see no use of the methods in question in the URL above. Maybe I’m missing something.

I believe one of the main use cases is to check whether an IO is in nonblocking mode (IO#nonblock?), and to set or ensure a specific IO instance is in nonblocking mode (IO#nonblock=, IO#nonblock {}).
This is essential for the Fiber scheduler and https://bugs.ruby-lang.org/issues/18630, they mostly don't work on IOs for which nonblock?=false.
However, such checking might be temporary or helpful for debugging but not necessarily kept in the code.
For instance https://github.com/ruby/ruby/blob/master/test/fiber/scheduler.rb requires it but does not seem to use the IO#nonblock* methods, but most likely they were used for debugging and finding out if everything is as expected.

OK I can understand it’s handy for debugging purposes. That, however, sounds more like it’s not the feature needed for everyone.

Having *_nonblock methods but no way to observe whether it's in nonblocking mode in core feels like a missing piece.
We have IO#autoclose?, IO#binmode?, IO#close_on_exec? yet without require 'io/nonblock' we have no way to check if an IO is in nonblocking mode, even though we have an indirect way to set it (IO#*_nonblock).

You can also see it's used in several places with https://github.com/search?q=require+io%2Fnonblock&type=Code: 40000+ results

Maybe I’m not doing the right thing but I have hard time finding out the actual use of the aforementioned feature in the URL, except testing codes. Do you want this for tests?

Also, are we going to add tons of trivial methods into core just because they are small? I guess we need good reasons for each of them to exist.

These methods already "exist", whether they are in core or not.
I don't understand why it makes such a big difference whether it's behind require 'io/nonblock' or not, these methods have always (or since so many years) been shipped with Ruby.

We want smaller core because of vulnerabilities. A separate gem can have a separate CVE, separate release, and administrators can avoid replacing everything only because a tiny fraction of our code has some issue. This is better than having security releases every time.

But if core devs feel strongly about it, I would also be fine for these methods to be in stdlib (as before the gem), and no longer a gem.
What is the advantage of having this in a gem, separate repository? I see none.

I see a big one.

[Eregon (Benoit Daloze)]

Sorry, it seems Redmine/I messed up the link by including the : in it.
This should work: https://github.com/search?q=require+io%2Fnonblock&type=Code
So that's ~40000 usages of `require io/nonblock' as far as I can see.
GitHub search isn't great though, so there is likely a lot of duplicate results (due vendored code, etc), and no easy way to search for method calls specifically.

This (or variants) might be more helpful, it uses the new GitHub search: https://cs.github.com/?scopeName=All+repos&scope=&q=.nonblock%3F+language%3ARuby

OK I can understand it’s handy for debugging purposes. That, however, sounds more like it’s not the feature needed for everyone.

I agree it's not needed by everyone. OTOH that can be said for so many methods in core.

IMHO, these 3 methods belong to IO core, implementation-wise and API-wise (ways to set it but not query it, annoying for debugging).
They might not be used a lot, but I don't think this is so important for core, there are plenty of methods in core not used much (e.g., most of Process methods).
I think stability and good naming are more important for core, and those methods are good at that.

We want smaller core because of vulnerabilities. A separate gem can have a separate CVE, separate release, and administrators can avoid replacing everything only because a tiny fraction of our code has some issue. This is better than having security releases every time.

Yes, but these 3 methods are clearly insignificant for vulnerabilities. Is that not obvious?
https://github.com/ruby/io-nonblock/blob/e20578b24405d225e6383d6c5ebfb7ffefac646b/ext/io/nonblock/nonblock.c is 97 SLOC of C code, if it was written in Ruby it'd probably be just 10 lines (e.g., using IO#fcntl).
It's tiny, it's extremely unlikely to have vulnerabilities, and it's never had a vulnerability in 10+ years AFAIK.
In fact, the vast majority of code in that repo (1257 SLOC according to cloc .) is all boilerplate that wouldn't be there if this was in core.

And if those methods were written in Ruby (e.g., using IO#fcntl), it'd probably be impossible to have a vulnerability in these 3 methods, so maybe that's a good reason to rewrite them using Ruby code.

So I understand the separate security release argument, but it seems clear there is a limit at which it's not worth the overhead, and io-nonblock seems way below that limit (in term of size and likeliness of vulnerabilities).
(joke: otherwise let's make a gem for every core method?)

[ioquatix (Samuel Williams)]

I support this change. In fact 90% of the code already exists in CRuby, it's just a matter of exposing it.

• I don't see any practical security advantage of this being a gem.
• I see an advantage of reducing code duplication by moving this into core.
• There are several methods like read and write which behave differently with nonblock=true. It doesn't make sense for a core semantic trait to be in a gem.

[ko1 (Koichi Sasada)]

-1 because I think the name IO#nonblock = makes me IO#read (or other methods) becomes nonblocking.

[ko1 (Koichi Sasada)]

There are several methods like read and write which behave differently with nonblock=true. It doesn't make sense for a core semantic trait to be in a gem.

Could you list the difference?

[mame (Yusuke Endoh)]

Can you explain what you expect IO#nonblock= to do?

IO#nonblock= is a low-level API to set O_NONBLOCK to the internal file descriptor of an IO. As far as I know, this was introduced in the Ruby 1.8 era to make IO operations (like IO#read) non-blocking.

# Ruby 1.8
$stdin.nonblock = true
$stdin.read #=> Errno::EAGAIN

Note that IO#read was a (relatively) simple wrapper for read(2) system call in the era. (More precisely, a wrapper for fread(3).)

However, since Ruby 1.9, IO#read is not a simple wrapper. It uses select(2) system call to wait until it is ready to read, and then call read(2) syscall. Therefore, O_NONBLOCK flag does not change the behavior of IO#read, AFAIK (let me know if I am wrong). It always works as a blocking operation, even if IO#nonblock= is set as true.

# Ruby 1.9
$stdin.nonblock = true
$stdin.read #=> wait and read

So, I think that IO#nonblock= does not work as you expect. If you want a non-blocking read, you have no choice but to use IO#read_nonblock.

[Eregon (Benoit Daloze)]

IO#read behaves externally/for the caller the same, i.e., it always "reads N bytes and block until it gets them (or EOF/error)".
But internally O_NONBLOCK does affect IO#read, and this is observable for instance with a Fiber scheduler.
If io.nonblock = true, then io.read will call the Fiber scheduler if the read(2) syscall returns EAGAIN.
The Fiber scheduler is then able to schedule another Fiber, instead of waiting/blocking, which improves CPU utilization and IO concurrency.

[mame (Yusuke Endoh)]

Eregon (Benoit Daloze) wrote in #note-10:

this is observable for instance with a Fiber scheduler.

Okay, can you show a self-contained program that demonstrates it? Is there another situation than a Fiber scheduler where IO#nonblock= is important?

A Fiber scheduler requires nonblock = true but not nonblock = false, right? If so, I think we may set O_NONBLOCK to all file descriptors by default. There is circumstantial evidence that this flag is not important to the interpreter itself (except a Fiber scheduler): whenIO#read_nonblock is called, O_NONBLOCK is implicitly set, and never reset.

require "io/nonblock"

p $stdin.nonblock? #=> false

begin
  p $stdin.read_nonblock(1)
rescue IO::WaitReadable
end

p $stdin.nonblock? #=> true

[Eregon (Benoit Daloze)]

mame (Yusuke Endoh) wrote in #note-11:

Okay, can you show a self-contained program that demonstrates it?

require 'io/nonblock'

class Scheduler
  def block
    raise
  end
  def unblock
    raise
  end
  def kernel_sleep
    raise
  end
  def io_wait(*)
    raise "io_wait called (expected)"
  end
  def fiber(&block)
    Fiber.new(&block).resume
  end
end

Fiber.set_scheduler Scheduler.new

$stdin.nonblock = true # needed to use scheduler for this IO

Fiber.schedule do
  print "> "
  p $stdin.gets
end

Is there another situation than a Fiber scheduler where IO#nonblock= is important?

Maybe not for IO#nonblock=. I think IO#nonblock? is useful for several cases, and notably to find out if a fd is nonblocking, which e.g., matters when it's passed to a C extension.
So it's an helpful method for debugging.

I think we may set O_NONBLOCK to all file descriptors by default.

IIRC there were issues with that, notably when @normalperson (Eric Wong) tried it a while ago. @ioquatix (Samuel Williams) might know more.
stdstreams/File seem nonblock?=false currently, and socket/pipe seem nonblock?=true currently.

Also, maybe O_NONBLOCK by default is actually slower for IO (without a Fiber scheduler)? It will typically cause more syscalls, isn't it?

There is circumstantial evidence that this flag is not important to the interpreter itself (except a Fiber scheduler): whenIO#read_nonblock is called, O_NONBLOCK is implicitly set, and never reset.

Yes, it does not matter for most methods, hence I don't see why it's a "problem" to include it into core.
AFAIK Ruby developers see stdlib/default gems as "code shipped with Ruby and should work well", and so have no reason to think that because IO#nonblock* is behind require 'io/nonblock' it should not be used.
My opinion is there is no problem if people use IO#nonblock*.
And there is also no problem if it is not used often (the case for many other core methods).

[normalperson (Eric Wong)]

"shyouhei (Shyouhei Urabe)" [e-mail] wrote:

I don't think this gem deserves a room into core. Are there any practical usages of it?

The functionality is already there; and it should be useful to check
if writing code that needs to be compatible between old/new Ruby
versions with different nonblock defaults.

Also, are we going to add tons of trivial methods into core just because they are small? I guess we need good reasons for each of them to exist.

Having many trivial .so is a waste of memory if they're loaded anyways in
common programs. See [ruby-core:107493] and
https://udrepper.livejournal.com/8790.html

(I also use etc' and socket' ext in all my scripts, anyways)

[normalperson (Eric Wong)]

"Eregon (Benoit Daloze)" [e-mail] wrote:

Issue #18668 has been updated by Eregon (Benoit Daloze).

mame (Yusuke Endoh) wrote in #note-11:

I think we may set O_NONBLOCK to all file descriptors by default.

IIRC there were issues with that, notably when @normalperson (Eric Wong)
tried it a while ago. @ioquatix (Samuel Williams) might know more.
stdstreams/File seem nonblock?=false currently, and
socket/pipe seem nonblock?=true currently.

nonblock=true as default is still risky to me. I tried it years
ago but the compatibility problems w.r.t. FD sharing (send_io,
exec) to non-Ruby programs put me off. AFAIK there were also
problems on the Windows side and I don't touch proprietary.

Also, maybe O_NONBLOCK by default is actually slower for IO
(without a Fiber scheduler)? It will typically cause more
syscalls, isn't it?

Yes, nonblock is slightly slower in terms of raw throughput for
a single endpoint (which the rare case). Nonblock makes sense
for the common case of multiple connections or the normal
Internet case when you have latency and bandwidth limitations.

[matz (Yukihiro Matsumoto)]

As @mame (Yusuke Endoh) described in #note-9, the design of io-nonblock is misleading. I'd rather like to make the gem deprecate.

Matz.

[Eregon (Benoit Daloze)]

I don't think the current design of io/nonblock is misleading. I would think the majority of people knowing about its existence know what it does. There is probably some bias here from people who knew about its 1.8 behavior, and that behavior was clearly broken as it would change the high-level semantics incorrectly of many other methods. So now we have the correct sensible semantics for those methods, IMHO no reason to deprecate.

Also we cannot deprecate it since the Fiber scheduler needs it, as I showed above.
And as @normalperson (Eric Wong) said, if one wants to use the Fiber scheduler and doesn't know the exact Ruby version, they need IO#nonblock= + IO#nonblock? to ensure the fd is O_NONBLOCK and so the Fiber scheduler is used.
Or simply if they don't know the exact kind of fd or don't want to rely on the changing defaults for IO instances.

https://bugs.ruby-lang.org/issues/18630 also needs io/nonblock, because that only applies to IO#nonblock? = true IO instances. And it has no effect on IO#nonblock? = false IO instances.

It seems clear core devs don't want it in core.
How about stdlib then?
A gem is clear overkill for this, and adding non-trivial overhead for every Ruby implementation.

[nobu (Nobuyoshi Nakada)]

TIL, IO#nonblock? doesn't work on Windows.
So I'm positive for the deprecation now.

[Eregon (Benoit Daloze)]

nobu (Nobuyoshi Nakada) wrote in #note-17:

TIL, IO#nonblock? doesn't work on Windows.
So I'm positive for the deprecation now.

So let's also deprecate half the methods of Process which don't work on Windows either?
I don't understand the argument.

My understanding is Windows doesn't support non-blocking for some file descriptors, hence of course IO#nonblock= should raise and IO#nonblock? should be false for those fds which can't be non-blocking on Windows.
Or is there an unexpected error there?

[Eregon (Benoit Daloze)]

@ko1 (Koichi Sasada) showed us the output on Windows:

require 'io/nonblock'
require 'socket'

# p STDOUT.nonblock?
p s = Socket.tcp('atdot.net', 80)
p s.nonblock = true
p s.nonblock? #=> `nonblock?': nonblock?() function is unimplemented on this machine (NotImplementedError)

We should fix IO#nonblock? to just be false on Windows.
And IO#nonblock= should raise on Windows, either always or when passed true.

BTW, several CRuby tests check if nonblock?, and they do it via IO.method_defined?("nonblock=") most likely to workaround that bug.

[Eregon (Benoit Daloze)]

Also I don't see why we need so much investigation into the details of io/nonblock and io/wait.
Those were always stdlib, and they became default gems (with no discussion AFAIK).
Now I'd like both of them to be back to stdlib, as they always were, and no longer gems.
Because those 2 gems here are significant overhead for all Ruby implementations (e.g. have to support each Ruby implementation in the gem, while this code just makes more sense in each Ruby implementation's repo).

I give up on trying to make them core, because core devs seem to feel strongly against it despite many arguments. And I personally don't care too much between core or stdlib. An extra require is no big deal, an extra gem is IMHO.

[ianks (Ian Ker-Seymer)]

IMO, we really do need to support this use case. Even if it were just fir testing purposes, it would still be worthwhile if it makes developing non-blocking gems easier.

Ruby should move towards better async support, not away from it. So it makes sense to remove extra friction where possible.

[shyouhei (Shyouhei Urabe)]

ianks (Ian Ker-Seymer) wrote in #note-21:

IMO, we really do need to support this use case. Even if it were just fir testing purposes, it would still be worthwhile if it makes developing non-blocking gems easier.

What precisely is "this use case" you mean? io.nonblock = true doesn't prevent things from blocking.