Project

General

Profile

Feature #16597

missing poll()

Added by michals (Michal Suchánek) 2 months ago. Updated 20 days ago.

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

Description

When using a socket or a pipe for writing select() cannot determine that the socket is closed. It tells you that it is ready to write but if you don't have data to write you cannot tell that it is in fact closed.

ruby internally uses poll() which can tell when a write socket is closed (without attempting a write()) but presents the broken select() interface.


Files

test.rb (975 Bytes) test.rb michals (Michal Suchánek), 02/05/2020 10:05 PM
0001-io-move-poll-defines-to-internal-io.h.patch (3.44 KB) 0001-io-move-poll-defines-to-internal-io.h.patch michals (Michal Suchánek), 02/05/2020 10:05 PM
0002-io-add-additional-argument-to-fd_select-calls.patch (9.54 KB) 0002-io-add-additional-argument-to-fd_select-calls.patch michals (Michal Suchánek), 02/05/2020 10:05 PM
0003-select_internal-add-additional-argument-for-poll-err.patch (4.4 KB) 0003-select_internal-add-additional-argument-for-poll-err.patch michals (Michal Suchánek), 02/05/2020 10:05 PM
0004-thread-use-poll-in-rb_fd_select.patch (3.88 KB) 0004-thread-use-poll-in-rb_fd_select.patch michals (Michal Suchánek), 02/05/2020 10:05 PM
0005-io-make-use-of-poll-POLLERR-flag.patch (1.24 KB) 0005-io-make-use-of-poll-POLLERR-flag.patch michals (Michal Suchánek), 02/05/2020 10:06 PM
0006-io-add-select_with_poll-class-method-when-USE_POLL.patch (2.71 KB) 0006-io-add-select_with_poll-class-method-when-USE_POLL.patch michals (Michal Suchánek), 02/05/2020 10:06 PM
0007-io-document-IO.select_with_poll.patch (2.04 KB) 0007-io-document-IO.select_with_poll.patch michals (Michal Suchánek), 02/05/2020 10:06 PM
tcp_test.rb (1018 Bytes) tcp_test.rb michals (Michal Suchánek), 02/05/2020 10:29 PM
0008-test-fix-up-wait_for_single_fd-to-accept-RB_WAITFD_E.patch (1.86 KB) 0008-test-fix-up-wait_for_single_fd-to-accept-RB_WAITFD_E.patch michals (Michal Suchánek), 02/07/2020 04:00 PM
unix_test.rb (992 Bytes) unix_test.rb michals (Michal Suchánek), 02/07/2020 04:01 PM
test_close.rb (440 Bytes) test_close.rb michals (Michal Suchánek), 02/25/2020 04:10 PM
0009-rb_fd_select-raise-exception-on-bad-fd-when-using-po.patch (2.54 KB) 0009-rb_fd_select-raise-exception-on-bad-fd-when-using-po.patch michals (Michal Suchánek), 02/27/2020 09:56 PM
0010-select_with_poll-do-not-reaise-exception-on-bad-fd.patch (11.8 KB) 0010-select_with_poll-do-not-reaise-exception-on-bad-fd.patch michals (Michal Suchánek), 03/10/2020 03:52 PM
0011-tests-add-a-few-more-tests-for-select.patch (2.56 KB) 0011-tests-add-a-few-more-tests-for-select.patch michals (Michal Suchánek), 03/10/2020 03:52 PM

Updated by michals (Michal Suchánek) 2 months ago

ruby 2.5.5p157 (2019-03-15 revision 67260) [x86_64-linux-gnu]

Updated by akr (Akira Tanaka) 2 months ago

As far as I know, it is impossible for TCP because shutdown(SHUT_RD) and close() doesn't notify it to the other end of the connection if no unread data. (Please point out me if I'm wrong.)
poll() doesn't help for this problem with TCP.

(An approximation is getsockopt(TCP_INFO) for Linux which can be used to investigate TCP state of a connection.)

I'm not sure for pipe but kernel knows that the other end is closed or not.

What the event flag of poll() you want to use?
https://pubs.opengroup.org/onlinepubs/9699919799/functions/poll.html

Updated by naruse (Yui NARUSE) 2 months ago

akr (Akira Tanaka) wrote:

As far as I know, it is impossible for TCP because shutdown(SHUT_RD) and close() doesn't notify it to the other end of the connection if no unread data. (Please point out me if I'm wrong.)
poll() doesn't help for this problem with TCP.

(An approximation is getsockopt(TCP_INFO) for Linux which can be used to investigate TCP state of a connection.)

I'm not sure for pipe but kernel knows that the other end is closed or not.

What the event flag of poll() you want to use?
https://pubs.opengroup.org/onlinepubs/9699919799/functions/poll.html

If you want to know a connection is closed, you need to check FIN or RST packet of TCP.
They are sent by shutdown(SHUT_WR) or close (hard close) in socket.
To detect that, it needs to use poll with POLLHUP flag.

Note that it also needs to clear internal buffer before calling poll.
On Linux it can use recv with MSG_TRUNC, but on other platform it needs it needs to read all data already received.

Updated by michals (Michal Suchánek) about 2 months ago

  • File 0004-io-add-select_with_poll-class-method-when-USE_POLL.patch added
  • File 0003-io-make-use-of-poll-POLLERR-flag.patch added
  • File 0002-select_internal-add-additional-argument-for-poll-err.patch added
  • File 0001-io-add-additional-argument-to-fd_select-calls.patch added

Attaching RFC patchset.

It is not awesome but it at least does not crash and burn.

Tests pass which shows they could use some refinement.

Updated by michals (Michal Suchánek) about 2 months ago

Reading is generally covered with select(). You call select() and the fd appears as readable. If there is an error you get it on read(). It is write() that does not have sane semantic without poll().

Updated by michals (Michal Suchánek) about 2 months ago

  • File 0005-io-additional-cleanup-for-select_with_poll.patch added

Additional cleanup patch that removes the extra argument from select() return value and superfluous argument.

Updated by akr (Akira Tanaka) about 2 months ago

Is there a test?

Updated by michals (Michal Suchánek) about 2 months ago

  • File test.rb added

This one, and it does not work. There is another place that needs to be patched.

LD_LIBRARY_PATH=. ./ruby -I. -Ilib -I.ext/common -I.ext/x86_64-linux/ test.rb
select: [[:@inpipe, :@outpipe, :@errpipe], [:@inpipe], []]
poll: [[:@inpipe, :@outpipe, :@errpipe], [:@inpipe], [], [:@inpipe, :@outpipe, :@errpipe]]
select: [[], [:@inpipe], []]
poll: [[], [:@inpipe], [], [:@inpipe, :@outpipe, :@errpipe]]

expected:

select: [[:@inpipe, :@outpipe, :@errpipe], [:@inpipe], []]
poll: [[:@inpipe, :@outpipe, :@errpipe], [:@inpipe], [], [:@inpipe, :@outpipe, :@errpipe]]
select: [[], [:@inpipe], []]
poll: [[], [:@inpipe], [], []]

Also while make test passes make check no longer does. It needs to be investigated if the test needs to be updated or the changed return value is not used anyway and can be changed back.

#9

Updated by michals (Michal Suchánek) about 2 months ago

  • File deleted (0001-io-add-additional-argument-to-fd_select-calls.patch)
#10

Updated by michals (Michal Suchánek) about 2 months ago

  • File deleted (0002-select_internal-add-additional-argument-for-poll-err.patch)
#11

Updated by michals (Michal Suchánek) about 2 months ago

  • File deleted (0003-io-make-use-of-poll-POLLERR-flag.patch)
#12

Updated by michals (Michal Suchánek) about 2 months ago

  • File deleted (0004-io-add-select_with_poll-class-method-when-USE_POLL.patch)
#13

Updated by michals (Michal Suchánek) about 2 months ago

  • File deleted (0005-io-additional-cleanup-for-select_with_poll.patch)

Updated by michals (Michal Suchánek) about 2 months ago

  • File 0006-thread-use-poll-in-rb_fd_select.patch added
  • File 0005-io-document-IO.select_with_poll.patch added
  • File 0004-io-add-select_with_poll-class-method-when-USE_POLL.patch added
  • File 0003-io-make-use-of-poll-POLLERR-flag.patch added
  • File 0002-select_internal-add-additional-argument-for-poll-err.patch added
  • File 0001-io-add-additional-argument-to-fd_select-calls.patch added

Updated patchset. Passes basic test but not TestParallel. For some reason it segfaults.

LD_LIBRARY_PATH=. ./ruby -I. -Ilib -I.ext/common -I.ext/x86_64-linux/ test.rb
select: [[:@inpipe, :@outpipe, :@errpipe], [:@inpipe, :@outpipe, :@errpipe], [:@inpipe, :@outpipe, :@errpipe]]
poll: [[:@inpipe, :@outpipe, :@errpipe], [:@inpipe, :@outpipe, :@errpipe], [:@inpipe, :@outpipe, :@errpipe], [:@inpipe, :@outpipe, :@errpipe]]
select: [[], [:@inpipe], []]
poll: [[], [:@inpipe], [], []]

Updated by akr (Akira Tanaka) about 2 months ago

michals (Michal Suchánek) wrote in #note-8:

This one, and it does not work. There is another place that needs to be patched.

Is there a test for socket?
I have interest in TCP.

#16

Updated by michals (Michal Suchánek) about 2 months ago

  • File deleted (test.rb)
#17

Updated by michals (Michal Suchánek) about 2 months ago

  • File deleted (0001-io-add-additional-argument-to-fd_select-calls.patch)
#18

Updated by michals (Michal Suchánek) about 2 months ago

  • File deleted (0002-select_internal-add-additional-argument-for-poll-err.patch)
#19

Updated by michals (Michal Suchánek) about 2 months ago

  • File deleted (0003-io-make-use-of-poll-POLLERR-flag.patch)
#20

Updated by michals (Michal Suchánek) about 2 months ago

  • File deleted (0004-io-add-select_with_poll-class-method-when-USE_POLL.patch)
#21

Updated by michals (Michal Suchánek) about 2 months ago

  • File deleted (0005-io-document-IO.select_with_poll.patch)
#22

Updated by michals (Michal Suchánek) about 2 months ago

  • File deleted (0006-thread-use-poll-in-rb_fd_select.patch)

Updated by michals (Michal Suchánek) about 2 months ago

There is some issue with TCP sockets:

LD_LIBRARY_PATH=. ./ruby -I. -Ilib -I.ext/common -I.ext/x86_64-linux/ tcp_test.rb
select: [[:@sock], [:@sock], []]
poll: [[:@sock], [:@sock], [], []]
select: [[], [:@sock], []]
poll: [[], [:@sock], [], []]

There is certainly difference between the socket open or closed but it does not appear in the error set. I suspect the problem is that you get socket reported as closed only when receiving RST because FIN just means no more data is supposed to go in that direction. But when a side closes the socket it sends FIN and you only get RST as response to data or bookkeeping packet. TCP keepalive option does produce bookkeeping packets regularly which should allow you to detect closed sockets eventually.

Updated by akr (Akira Tanaka) about 2 months ago

michals (Michal Suchánek) wrote in #note-26:

Test with unix socket

Test needs expected results.

Updated by michals (Michal Suchánek) about 2 months ago

This is the current result:

$ ruby unix_test.rb
select: [[:@sock], [:@sock], [:@sock]]
poll: [[:@sock], [:@sock], [:@sock], [:@sock]]
select: [[], [:@sock], []]
poll: [[], [:@sock], [], []]

If this is to be expected or the emulation should be adjusted might need some discussion.

I expect that using select without emulation would give
select: [[:@sock], [], []]
in the first line.

To be more specific: select() indicates error indirectly by returning the fd as ready to read if asked for reading. This completely fails if the user did not ask for reading on the socket and fails to distinguish readiness to read from error. Returning the socket as ready to for anything asked when error is reported from poll() somewhat emulates the select() behavior extending it to write sockets. The return value necessarily differs from that of select() to do so. The tests can be made conditional on USE_POLL but ultimately this will lead to different behavior on different systems.

Updated by michals (Michal Suchánek) about 2 months ago

and BTW is it possible to get notifications from this bugtracker? From my POV it just silently sinks data.

Updated by michals (Michal Suchánek) about 1 month ago

An extension that makes poll() features available and does not require integration into ruby core is eventmachine.

Updated by michals (Michal Suchánek) about 1 month ago

There is a bug with handling POLLNVAL in the new code.

Updated by michals (Michal Suchánek) about 1 month ago

Attaching test and fix.

This fixes the problem with not raising exception but now my patched interpreter tends to crash. Need to investigate.

#33

Updated by michals (Michal Suchánek) about 1 month ago

  • File deleted (0009-rb_fd_select-raise-exception-on-bad-fd-when-using-po.patch)

Updated by michals (Michal Suchánek) 20 days ago

Additional patches:

  • Do no throw exception form poll() because it can tell the user which exact descriptor is bad which is lost in the exception
  • Add tests

Also in https://github.com/hramrach/ruby

Also available in: Atom PDF