Skip to content

Add multi-waiter support for TCP accept - #109

Open
nding0405 wants to merge 2 commits into
CHERIoT-Platform:mainfrom
nding0405:tcp-accept-multiwaier
Open

Add multi-waiter support for TCP accept#109
nding0405 wants to merge 2 commits into
CHERIoT-Platform:mainfrom
nding0405:tcp-accept-multiwaier

Conversation

@nding0405

Copy link
Copy Markdown
Contributor

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:

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

  2. Read-only getter: network_socket_get_event_source() returns a capability to
    a socket's event futex stripped to read-only.

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

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

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

…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.
Comment thread include/NetAPI.h
Comment on lines +50 to +53
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;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please can you add doc comments for these?

Comment thread include/NetAPI.h Outdated
* 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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* -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)

Comment thread lib/tcpip/network_wrapper.cc Outdated
{
/*
* An ephemeral call is needed here to prevent UAF when
* dereferencing the socket wrapper.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I see the distinction now. I’ll move the ephemeral claim into signal_event_futex instead, and notify_all will clear the it.

Comment on lines +854 to +856
__clang_ignored_warning_push("-Wcheri-capability-misuse");
auto errCode = reinterpret_cast<Socket>(retVal);
__clang_ignored_warning_pop();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to have C and C++ helpers for this (separate PR).

@davidchisnall

Copy link
Copy Markdown
Contributor

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.
@nding0405
nding0405 force-pushed the tcp-accept-multiwaier branch from 9cdf152 to 6c4576c Compare July 28, 2026 01:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants