Feature #16597: missing poll()

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

[michals (Michal Suchánek)]

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.

[michals (Michal Suchánek)]

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

[akr (Akira Tanaka)]

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

[naruse (Yui NARUSE)]

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.

[michals (Michal Suchánek)]

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

[michals (Michal Suchánek)]

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().

[michals (Michal Suchánek)]

• 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.

[akr (Akira Tanaka)]

Is there a test?

[michals (Michal Suchánek)]

• 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.

[michals (Michal Suchánek)]

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

[michals (Michal Suchánek)]

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

[michals (Michal Suchánek)]

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

[michals (Michal Suchánek)]

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

[michals (Michal Suchánek)]

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

[michals (Michal Suchánek)]

• File 0001-io-add-additional-argument-to-fd_select-calls.patch added
• File 0002-select_internal-add-additional-argument-for-poll-err.patch added
• File 0003-io-make-use-of-poll-POLLERR-flag.patch added
• File 0004-io-add-select_with_poll-class-method-when-USE_POLL.patch added
• File 0005-io-document-IO.select_with_poll.patch added
• File 0006-thread-use-poll-in-rb_fd_select.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], [], []]

[akr (Akira Tanaka)]

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.

[michals (Michal Suchánek)]

• File deleted (test.rb)

[michals (Michal Suchánek)]

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

[michals (Michal Suchánek)]

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

[michals (Michal Suchánek)]

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

[michals (Michal Suchánek)]

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

[michals (Michal Suchánek)]

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

[michals (Michal Suchánek)]

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

[michals (Michal Suchánek)]

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

I use this with local (UNIX) sockets but did not backport the current patchset to my production ruby version yet. I did not test TCP sockets.

[michals (Michal Suchánek)]

• File tcp_test.rb tcp_test.rb added

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.

[michals (Michal Suchánek)]

• File 0008-test-fix-up-wait_for_single_fd-to-accept-RB_WAITFD_E.patch added

Adding test fixup. With this tests pass on master/2.6/2.5

[michals (Michal Suchánek)]

• File unix_test.rb unix_test.rb added

Test with unix socket

[akr (Akira Tanaka)]

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

Test with unix socket

Test needs expected results.

[michals (Michal Suchánek)]

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.

[michals (Michal Suchánek)]

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

[michals (Michal Suchánek)]

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

[michals (Michal Suchánek)]

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

[michals (Michal Suchánek)]

• File test_close.rb added
• File 0009-rb_fd_select-raise-exception-on-bad-fd-when-using-po.patch added

Attaching test and fix.

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

[michals (Michal Suchánek)]

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

[michals (Michal Suchánek)]

• File 0009-rb_fd_select-raise-exception-on-bad-fd-when-using-po.patch added

Updated patch

[michals (Michal Suchánek)]

• File 0010-select_with_poll-do-not-reaise-exception-on-bad-fd.patch added
• File 0011-tests-add-a-few-more-tests-for-select.patch added

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

[michals (Michal Suchánek)]

• File deleted (test.rb)

[michals (Michal Suchánek)]

• File deleted (0001-io-move-poll-defines-to-internal-io.h.patch)

[michals (Michal Suchánek)]

• File deleted (0002-io-add-additional-argument-to-fd_select-calls.patch)

[michals (Michal Suchánek)]

• File deleted (0003-select_internal-add-additional-argument-for-poll-err.patch)

[michals (Michal Suchánek)]

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

[michals (Michal Suchánek)]

• File deleted (0005-io-make-use-of-poll-POLLERR-flag.patch)

[michals (Michal Suchánek)]

• File deleted (0006-io-add-select_with_poll-class-method-when-USE_POLL.patch)

[michals (Michal Suchánek)]

• File deleted (0007-io-document-IO.select_with_poll.patch)

[michals (Michal Suchánek)]

• File deleted (0008-test-fix-up-wait_for_single_fd-to-accept-RB_WAITFD_E.patch)

[michals (Michal Suchánek)]

• File deleted (0011-tests-add-a-few-more-tests-for-select.patch)

[michals (Michal Suchánek)]

• File deleted (0010-select_with_poll-do-not-reaise-exception-on-bad-fd.patch)

[michals (Michal Suchánek)]

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

[michals (Michal Suchánek)]

• File deleted (test_close.rb)

[michals (Michal Suchánek)]

• File 0007-io-document-IO.select_with_poll.patch 0007-io-document-IO.select_with_poll.patch added
• File 0008-test-fix-up-wait_for_single_fd-to-accept-RB_WAITFD_E.patch 0008-test-fix-up-wait_for_single_fd-to-accept-RB_WAITFD_E.patch added
• File 0009-rb_fd_select-raise-exception-on-bad-fd-when-using-po.patch 0009-rb_fd_select-raise-exception-on-bad-fd-when-using-po.patch added
• File 0010-select_with_poll-do-not-reaise-exception-on-bad-fd.patch 0010-select_with_poll-do-not-reaise-exception-on-bad-fd.patch added
• File 0001-io-move-poll-defines-to-internal-io.h.patch 0001-io-move-poll-defines-to-internal-io.h.patch added
• File 0002-io-add-additional-argument-to-fd_select-calls.patch 0002-io-add-additional-argument-to-fd_select-calls.patch added
• File 0003-select_internal-add-additional-argument-for-poll-err.patch 0003-select_internal-add-additional-argument-for-poll-err.patch added
• File 0004-thread-use-poll-in-rb_fd_select.patch 0004-thread-use-poll-in-rb_fd_select.patch added
• File 0005-io-make-use-of-poll-POLLERR-flag.patch 0005-io-make-use-of-poll-POLLERR-flag.patch added
• File 0006-io-add-select_with_poll-class-method-when-USE_POLL.patch 0006-io-add-select_with_poll-class-method-when-USE_POLL.patch added

[michals (Michal Suchánek)]

• File 0011-tests-add-a-few-more-tests-for-select.patch 0011-tests-add-a-few-more-tests-for-select.patch added
• File 0012-Update-extension.rdoc-for-rb_fd_select.patch 0012-Update-extension.rdoc-for-rb_fd_select.patch added

rebased

[michals (Michal Suchánek)]

Is there some change of getting the poll() semantic available in ruby?