Add multi-waiter support for TCP accept - #109
Conversation
…sive The network_socket_accept_tcp function can only return either a valid sealed capability or an untagged nullptr, so the caller cannot distinguish a timeout from an out-of-memory failure. Change it so that it can return an invalid capability that encodes the error code.
| static constexpr size_t NumFutexTypes = SocketEventType::SocketAcceptEvent + 1; | ||
| // Sentinel stored in the socket's event futexes to mark the socket as torn | ||
| // down. | ||
| static constexpr uint32_t SocketNotAvailable = -1; |
There was a problem hiding this comment.
Please can you add doc comments for these?
| * an untagged value on failure. | ||
| * This returns either a valid sealed socket type or an untagged capability that | ||
| * encodes the error code, and here are what each of them represent: | ||
| * -ENOMEM: allocation of the wrapper timed out or does not have memory for now. |
There was a problem hiding this comment.
| * -ENOMEM: allocation of the wrapper timed out or does not have memory for now. | |
| * `-ENOMEM`: allocation of the wrapper timed out or does not have memory for now. |
(And the same markup below)
| { | ||
| /* | ||
| * An ephemeral call is needed here to prevent UAF when | ||
| * dereferencing the socket wrapper. |
There was a problem hiding this comment.
I don't understand this. The claim is dropped when you call signal_event_futex and it looks as if the only thing that's happening in the caller is pointer arithmetic, not dereference.
It's generally bad style to assume an ephemeral claim will be valid on the way into a function. Looking inside signal_event_futex, it appears that it does a load and a CAS first, which could trap, and then a notify (which drops the claim). I think moving the claim into that function would make ownership clearer.
There was a problem hiding this comment.
Thanks! I see the distinction now. I’ll move the ephemeral claim into signal_event_futex instead, and notify_all will clear the it.
| __clang_ignored_warning_push("-Wcheri-capability-misuse"); | ||
| auto errCode = reinterpret_cast<Socket>(retVal); | ||
| __clang_ignored_warning_pop(); |
There was a problem hiding this comment.
It would be nice to have C and C++ helpers for this (separate PR).
|
Accidentally hit approve. There are a couple of small things that are worth fixing in this before it lands. |
Allow a single thread to wait for incoming connections across multiple listening sockets at once using multiwaiter_wait(), instead of having to block in network_socket_accept_tcp() on one socket at a time. Each socket now exposes an array of futex called eventFutexState, that a caller can register them with a multiwaiter. For this PR, only the added the accept functionality, others remains commented out for now. A thread can watch the accept futexes of several listening sockets simultaneously, block in multiwaiter_wait() until one of them signals a ready connection, and then accept that connection. The blocking wait therefore moves out of accept() and into the multiwaiter: accept() is called only once a connection is known to be ready, so it no longer has to block, which is what makes it possible to wait on several listening sockets at the same time from a single thread. Key changes: Added Event futex array (tcpip-internal.h, NetAPI.h) for socket wrapper: Only SocketAcceptEvent is implemented today, room reserved for future receive/send events. The value of the futex counts the ready events currently pending on the socket. For SocketAcceptEvent, it represents the number of connections that are ready to be accepted. The reserved value SocketNotAvailable (0xFFFFFFFF) means the socket will be freed soon. Read-only getter: network_socket_get_event_source() returns a capability to a socket's event futex stripped to read-only. Producing events: on_tcp_connect increments the SocketAcceptEvent futex and calls notify_all() when a new connection becomes ready. To let the callback can find the wrapper from the raw socket, network_socket_create_and_bind() now applys a bidirectional link between the wrapper and raw socket. Consuming events: network_socket_accept_tcp() decrements the futex after it successfully accepts a connection. This only keeps the futex value align with the semantic desbribed above. It does not call notify_all(), since dequeuing an existing connection is not a new, so no need to wake the threads up. Avoid waiting forever when socket is not available: the futex lives in the socket wrapper's heap memory, so a thread must never be left blocked on a socket that is being freed. Both network_socket_close() and the reset handler (reset_network_stack_state) set every event futex to SocketNotAvailable and call notify_all() before that memory goes away. The value change wakes any thread blocked in multiwaiter_wait(), which then observes the sentinel and returns instead of sleeping on a dead socket. A subsequent accept() will see the sentinel and reports the socket as unavailable.
9cdf152 to
6c4576c
Compare
New feature description:
Allow a single thread to wait for incoming connections across multiple
listening sockets at once using multiwaiter_wait(), instead of having to
block in network_socket_accept_tcp() on one socket at a time.
Each socket now exposes an array of futex called eventFutexState, that
a caller can register them with a multiwaiter. For this PR, only the added
the accept functionality, others remains commented out for now. A thread
can watch the accept futexes of several listening sockets simultaneously,
block in multiwaiter_wait() until one of them signals a ready connection, and then
accept that connection. The blocking wait therefore moves out of accept() and
into the multiwaiter: accept() is called only once a connection is known to be
ready, so it no longer has to block, which is what makes it possible to wait on
several listening sockets at the same time from a single thread.
Key changes:
Added Event futex array (tcpip-internal.h, NetAPI.h) for socket wrapper: Only
SocketAcceptEvent is implemented today, room reserved for future receive/send
events. The value of the futex counts the ready events currently pending on the
socket. For SocketAcceptEvent, it represents the number of connections that are
ready to be accepted. The reserved value SocketNotAvailable (0xFFFFFFFF) means the
socket will be freed soon.
Read-only getter: network_socket_get_event_source() returns a capability to
a socket's event futex stripped to read-only.
Producing events: on_tcp_connect increments the SocketAcceptEvent futex and calls
notify_all() when a new connection becomes ready. To let the callback can find
the wrapper from the raw socket, network_socket_create_and_bind() now applys
a bidirectional link between the wrapper and raw socket.
Consuming events: network_socket_accept_tcp() decrements the futex after it
successfully accepts a connection. This only keeps the futex value align with the
semantic desbribed above. It does not call notify_all(), since dequeuing an existing
connection is not a new, so no need to wake the threads up.
Avoid waiting forever when socket is not available: the futex lives in the socket
wrapper's heap memory, so a thread must never be left blocked on a socket
that is being freed. Both network_socket_close() and the reset handler
(reset_network_stack_state) set every event futex to SocketNotAvailable and
call notify_all() before that memory goes away. The value change wakes any
thread blocked in multiwaiter_wait(), which then observes the sentinel and
returns instead of sleeping on a dead socket. A subsequent accept() will see the
sentinel and reports the socket as unavailable.