Page MenuHomeFreeBSD

POLLRDHUP
ClosedPublic

Authored by tmunro on Apr 14 2021, 5:37 AM.

Details

Summary

POSIX doesn't provide a way to find out if the other end has closed a socket without attempting to read (EOF) or write (connection reset etc). Linux 2.6.something added POLLRDHUP to detect that, and by now it's fairly widely used. It's a bit like POLLHUP, but it fires when remote shutdown(SHUT_WR) or close() sends a FIN, not only on errors/full-shutdown. At least illumos has also adopted this flag.

I am not sure what visibility the flag should have (__BSD_VISIBLE) or whether it should be in POLLSTANDARD.

Random examples of users of this flags in the wild (I happened to write one of these):

https://github.com/Exim/exim/blob/e4e884faa7f5a04d937282113681d97a355ed2af/src/src/acl.c#L3523
https://github.com/mysql/mysql-server/blob/7ed30a748964c009d4909cb8b4b22036ebdef239/vio/viosocket.cc#L750
https://github.com/postgres/postgres/blob/c30f54ad732ca5c8762bb68bbe0f51de9137dd72/src/backend/libpq/pqcomm.c#L1936

Test Plan

I added a simple test to tests/sys/netinet/socket_afinet.c. Is there a better place for this? It also works for Unix domain sockets, but I didn't include a test for that because it's going through the same code path anyway. I also considered tools/regression/poll/sockpoll.c but it is full of tests that fail already so I'm not sure what its status is.

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Please re-upload the patch with content (diff -U9999999)

sys/kern/uipc_socket.c
3565

Are you sure that the right semantic is to not send POLLRDHUP if POLLINIGNEOF is set?
POLLINIGNEOF is used to implement required semantic of the generations for named pipes AFAIR,

greg_unrelenting.technology added inline comments.
lib/libc/sys/poll.2
275

maybe mention illumos here too?

This time with -U999999, and mentioning illumos as requested.

tmunro added inline comments.
sys/kern/uipc_socket.c
3565

This is only intended to work for stream sockets. If we wanted to make it work for other kinds of descriptors, we'd have to invent the semantics for that because it doesn't work on other OSes either; it's not that clear what 'hangup' even means for a fifo anyway, since it can be opened and closed many times.

sys/kern/uipc_socket.c
3565

Well, this is exactly why POLLINIGNEOF does.

But if POLLRDHUP is only defined for streams, and POLLINIGNEOF is kernel-set bit for pipes/fifos, shouldn't it be completely ignored for POLLRDHUP?

tmunro marked an inline comment as done.
tmunro marked an inline comment as done.

Ok, now ignoring POLLINIGNEOF for this.

I am. in fact. more interested in a test which makes kernel report POLLRDHUP despite it not specified in events. Could you please add such test as well?

tests/sys/netinet/socket_afinet.c
27

Order include files properly.

tmunro marked an inline comment as done.

Added a test to show that we don't report POLLRDHUP unless you asked for it.

This all looks fine to me, two notes:

  1. Please split the test into two
  2. Please take a look at tools/regression/poll even if you do not do anything there. But ideally you would add a test there and record the result for Linux and us.

I think item 1 is the only pre-req for the commit, item 2 can be handled later.

In D29757#668704, @kib wrote:

This all looks fine to me, two notes:

  1. Please split the test into two

Done.

  1. Please take a look at tools/regression/poll even if you do not do anything there. But ideally you would add a test there and record the result for Linux and us.

Done. Will commit that part separately.

Do you agree that this is MFCable?

Do you agree that this is MFCable?

Sure, why would it not be?

Please upload the diff with the context.

kib added inline comments.
lib/libc/sys/poll.2
128

Don't forget to bump .Dd

273

It is really strange that .Dv face is not used for POLL* constants.

This revision is now accepted and ready to land.Apr 26 2021, 12:17 PM