Project

General

Profile

Actions

Feature #18655

closed

Copy `IO#wait_readable`, `IO#wait_writable`, `IO#wait_priority` and `IO#wait` into core.

Added by byroot (Jean Boussier) about 2 years ago. Updated over 1 year ago.

Status:
Closed
Assignee:
-
Target version:
-
[ruby-core:108039]

Description

Extracted from [Feature #18566].

The decision was made to consider the methods from io-wait and io-nonblock one by one.

I think wait_readable and wait_writeable should be fairly non-controversial. They're quite essential to use IO#read_nonblock and IO#write_nonblock effectively.

Proposed patch: https://github.com/ruby/ruby/pull/5694

NB: if we only merge some methods, then io/wait must test which methods it needs to define or not. For now I use a if RUBY_VERSION >= "3.2" check in extconf.rb, but there might be a better approach.


Related issues 1 (0 open1 closed)

Related to Ruby master - Feature #18566: Merge `io-wait` and `io-nonblock` gems into core IOClosedActions
Actions #1

Updated by byroot (Jean Boussier) about 2 years ago

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

Updated by Eregon (Benoit Daloze) almost 2 years ago

Copying a general comment (https://github.com/ruby/ruby/pull/5694#issuecomment-1081700929) on the PR here:

I agree with @ioquatix (Samuel Williams) that it doesn't make sense to make IO#wait_readable and IO#wait_writable core, but IO#wait not, because of their close relation. And similarly it makes no sense to me to IO#leave wait_priority in the gem (that was just badly documented and that can be fixed easily).
The 3 IO#wait_* methods should be together and must stay consistent with one another, and also implementation-wise it's super weird if not in the same place. It's a similar argument for IO#wait.

The controversial methods in https://bugs.ruby-lang.org/issues/18566#note-16 seem ready? and nread.
Those seem fine to leave in the gem (very rarely used, and not so directly related to IO#wait*).

Updated by larskanis (Lars Kanis) almost 2 years ago

IMHO the problem with IO#wait is that its return value not well defined. Here is the issue: https://github.com/ruby/io-wait/issues/13

Updated by Eregon (Benoit Daloze) almost 2 years ago

@larskanis (Lars Kanis) That's a good point (it seems a bug).
And IO#wait also evolved more than IO#wait_{readable,writable,priority} IIRC.
It doesn't seem a very stable interface yet, and it's hard to use (STDOUT.wait(IO::READABLE | IO::WRITABLE) doesn't work as expected, it hangs)

All of these 4 methods end up in rb_io_wait() I think, but with a fair amount of code to wrap it.
So I think it'd be fair to leave IO#wait out of core for now then based on these reasons.

I think it'd make sense to include wait_priority, so the 3 wait_* with simple semantics are kept together.

Updated by ioquatix (Samuel Williams) almost 2 years ago

I'm fine with that.

Updated by ioquatix (Samuel Williams) almost 2 years ago

I've made a PR to move wait, wait_readable, wait_writable, wait_priority to io.c with some minor compatibility and documentation improvements.

https://github.com/ruby/ruby/pull/6036

Actions #7

Updated by ioquatix (Samuel Williams) almost 2 years ago

  • Subject changed from Merge `IO#wait_readable` and `IO#wait_writable` into core to Copy `IO#wait_readable`, `IO#wait_writable`, `IO#wait_priority` and `IO#wait` into core.

Updated by nobu (Nobuyoshi Nakada) almost 2 years ago

ioquatix (Samuel Williams) wrote in #note-6:

I've made a PR to move wait, wait_readable, wait_writable, wait_priority to io.c with some minor compatibility and documentation improvements.

https://github.com/ruby/ruby/pull/6036

This mixes the fix for compatibility (https://github.com/ruby/io-wait/issues/13), which is irrelevant to this PR.

Updated by ioquatix (Samuel Williams) almost 2 years ago

  • Status changed from Open to Closed

This is now completed.

Updated by matz (Yukihiro Matsumoto) over 1 year ago

After the discussion, we will drop wait_priority, but others are accepted.

Matz.

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0