Add accept_socket() to tcp and tls servers

[jbaldwin]

This will allow for a graceful shutdown.

Closes #377

[tglane]

Regarding the failing macOS tests:
It seems that kqueue will only be notified over connection shutdown via the EV_EOF filter if the peer disconnects. If we call shutdown or even close on the accepting socket ourselves (like it's done in the test) it is not guaranteed that the kernel will notify the kqueue listener.

Im not quite sure how we should work around that right now but it would probably require some reworking.

[tglane]

Unfortunately this is not how EV_EOF works. It is a flag that is set by the filters registered on arrival, so we can check for EV_EOF when receiving the kqueue events.

This means that in next_events we can check for a specific filter (e.g. EVFILT_READ or EVFILT_WRITE) if there is the flag EV_EOF present. But again: This is only mandatory for the situation when the peer disconnects/closes the socket.

For reference (from kqueue docs)

EV_EOF	     Filters may set this flag to indicate filter-specific EOF
		     condition.

[jbaldwin]

I see, its always set on READ or WRITE registrations and is reported back to the user, you don't have to set it.

[tglane]

Indeed. You should also move the EV_ERROR check up before the read/write check in the if-statement.

But unfortunately, thats exactly what I tested before on my machine and it was Not fixing the issue. The problem stays the same: The EV_EOF flag is not set when the accept socket is shutdown.

[jbaldwin]

Yeah ok, I think I'll wrap the test is !macos and comment why it isn't tested due to kqueue limitations and we'll get this merged. I'm not really sure how else to handle this for that platform right now.

[tglane]

Probably a good idea.

This problem really only applies to the accepting server_socket. The client_socket will register the shutdown call as this will result in a disconnecting peer which will than trigger the EV_EOF event. It might be an option to have a pipe fd as an additional member of the server_socket. We could than add a shutdown method to the server_socket that would send a signal to the pipe. In server_socket::poll we could use Coro::when_any on the socket and the pipe and use that for noticing the shutdown. The drawback would be that we would still not be able to use the shutdown method on the native socket for that. Just an idea I came up with but it probably needs more though. What do you think? (I would only implement that for the macOS builds)

[jbaldwin]

I think it would be worth a shot on another PR if you want to give it a go, I'm prepping this one to skip the test and will get it merged today if possible.

[jbaldwin]

Moving the error check up caused a benchmark test to fail, the test might be incorrect or failing on shutdown

[tglane]

I was about to comment the exact same thing as a solution for your failing test :)👍🏼 But there are a few more spots in bench.cpp where the same if statement should be applied.
Maybe add REQUIRE_THREAD_SAFE(pstatus == coro::poll_status::closed); inside the if block?

[jbaldwin]

So the more I play around with this the more it feels like kqueue is possibly reporting two states back, read and closed, and by moving closed above read/write its causing other tests to fail now too that were passing because the read poll was valid as well.

I think I'm going to put the closed check last again for now to get this merged and leave the test off for macos and lets open another issue to handle kqueue responding with multiple events.

For instance the test_tcp_server.cpp tests on line 44 just started failing... but it used to pass everytime, and its only failing because this code is now prioritizing closed over read for that poll call.

edit: thinking about this logically the EV_EOF is a flag on the return, which means that kqueue can report multiple event types, and its auto-registered unlike epoll.

[tglane]

edit: thinking about this logically the EV_EOF is a flag on the return, which means that kqueue can report multiple event types, and its auto-registered unlike epoll.

With kqueue you register for read/write events on the socket and kqueue will report back when a read or write event happen. The thing is, that for kqueue EV_EOF is just a flag for the read/write events. This means, that when we close the read half of the socket ( shutdown(fd, SHUT_RD); ). kqueue will report this as a read event but sets the end of file flag for this. Same goes for errors. Because of that my first implementation of the match statement in io_notifier_kqueue::event_to_poll_status was wrong since it was checking only for read/write at the beginning which is returning true even in case of eof or error.

I think I'm going to put the closed check last again for now to get this merged and leave the test off for macos and lets open another issue to handle kqueue responding with multiple events.

Good idea. I`d like to work on that issue when I have the time :)