Skip to content

feat(network): support a caller-owned poll - #153

Merged
Bronek merged 5 commits into
mainfrom
bronek/network_poll_refactor
Sep 4, 2026
Merged

feat(network): support a caller-owned poll#153
Bronek merged 5 commits into
mainfrom
bronek/network_poll_refactor

Conversation

@Bronek

@Bronek Bronek commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

TcpNetworkCore now holds state and operations shared by two wrappers. TcpNetwork retains its internal Poll, Events, and poll_with; the new TcpNetworkWithExternalPoll registers sockets through a caller-provided Registry. Both wrappers expose core operations through Deref.

This allows a caller to drive multiple TCP networks from one Poll and route readiness events by token.

External polling contract

Each polling cycle has four steps:

  1. Call pre_poll to deliver pending disconnect notifications and attempt due reconnects.
  2. Poll using the caller-owned Poll and Events buffer.
  3. Pass each event in the network's token range to handle_event.
  4. Call post_poll to deliver pending disconnect notifications.

If post_poll is skipped, pending notifications remain queued for a later post_poll or pre_poll. The caller chooses the poll timeout. Dropping the network drops its sockets but does not explicitly deregister them. TcpNetworkWithExternalPoll includes a compiling rustdoc example of the loop.

Token ranges

TcpNetworkWithExternalPoll::new takes a half-open Range<usize>. Listener and connection tokens are assigned from that range in ascending order and are not reused; persistent outbound endpoints retain their token across reconnects. An operation that requires another token panics when the range is exhausted. This replaces the previous check, which guarded only against usize overflow.

Example in rustdoc is suggesting 2^48 tokens in a token range: this number might seem too large for most uses, but there is literally no cost to using such a large range. The example aims to establish a good practice (range exhaustion causes panic).

handle_event uses a debug_assert to reject tokens outside the configured range in debug builds. The caller is responsible for assigning non-overlapping ranges and routing events to the appropriate network; overlapping ranges are not detected. The owned TcpNetwork uses an internal range starting at zero, so its constructor is unchanged.

Compatibility and versioning

Method-call syntax continues to work through Deref, but moved methods are no longer available through fully qualified paths such as TcpNetwork::send_with. Version bumped to 0.2.1 (new feature: TcpNetworkWithExternalPoll)

Tests

  • Two networks with disjoint token ranges exchange traffic through one poll with caller-side token dispatch.
  • A persistent endpoint reconnects with the same token, and disconnect notifications are checked in the phase that reports them.
  • Range exhaustion names the exhausted range, and an out-of-range event triggers the debug assertion without producing a TcpEvent in release builds.
  • The owned network starts at token zero, with listeners and connections sharing the counter.

Assisted-by: Claude:claude-fable-5
Assisted-by: Codex:gpt-5.6-sol

Bronek and others added 3 commits September 4, 2026 14:40
Separate the TCP network state from Poll by storing a Registry in
NetworkState. TcpNetwork continues to own its Poll and Events, while
shared network operations move to TcpNetworkRaw.

Add TcpNetworkWithExternalPoll so a caller can register TCP sockets with
its own poll. The caller drives reconnect processing, passes relevant
events to the network, and drains pending disconnect notifications
through pre_poll, epoll_event, and post_poll.

The external-poll constructor accepts a half-open token range. Listener
and connection tokens are assigned from that range in ascending order,
and an operation requiring another token panics when the range is
exhausted. The caller remains responsible for avoiding collisions with
other tokens registered with the same poll.

TcpNetwork and TcpNetworkWithExternalPoll expose the shared operations
through Deref. Ordinary method-call syntax remains available, but fully
qualified calls to methods previously defined directly on TcpNetwork
will need updating.

Co-authored-by: vladimir-ea <vladimir@gattaca.com>
Assisted-by: Claude:claude-fable-5
Assisted-by: Codex:gpt-5.6-sol
Export TcpNetworkCore and TcpNetworkWithExternalPoll. Rename the shared
type from TcpNetworkRaw to TcpNetworkCore and rename epoll_event to the
platform-neutral handle_event.

Document the internally polled and caller-polled variants, including the
pre_poll, poll, handle_event, and post_poll sequence. Add a debug
assertion to detect events routed outside the network's configured token
range.

Add integration coverage for two TCP networks driven by one Poll with
disjoint token ranges. The test exercises bidirectional messages,
reconnection with a stable outbound token, and disconnect notification
delivery. Additional tests cover token-range exhaustion and the owned
network's shared listener/connection token counter.

Assisted-by: Claude:claude-fable-5
Assisted-by: Codex:gpt-5.6-sol
Add a compiling example for TcpNetworkWithExternalPoll showing the
pre_poll, poll, token dispatch, handle_event, and post_poll sequence.

Add coverage for dispatching an event outside the network's configured
token range. Debug builds reject it through the containment assertion;
release builds emit no TcpEvent for the unknown token.

Assisted-by: Claude:claude-fable-5
Assisted-by: Codex:gpt-5.6-sol
@Bronek
Bronek requested review from a team, ltitanb and vladimir-ea September 4, 2026 14:58
Comment thread crates/flux-network/src/tcp/network.rs Outdated
/// [`Self::handle_event`] for each event in this network's token range, and
/// [`Self::post_poll`]. The caller owns the poll, event buffer, and timeout.
/// Dropping this value drops its sockets but does not explicitly deregister
/// them.

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.

does it not deregister them? the TcpNetworkCore#state has the Registry - we could add a Drop impl to NetworkState that deregisters on drop? this would be harmless for the TcpNetwork that owns its own Poll

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. Will also close_connection_socket - which is a minor behaviour change for TcpNetwork but I think a useful one.

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.

Added in 505759c

Comment thread Cargo.toml
repository = "https://github.com/gattaca-com/flux"
rust-version = "1.91.0"
version = "0.2.0"
version = "0.2.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.

think we merged already 0.3.0

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.

Not yet 😄

Attempt to deregister registered connection sockets and listeners when
NetworkState is dropped. This matters for a caller-owned poll because,
on Linux, a duplicated descriptor can keep an epoll registration alive
after the original descriptor closes.

Order TcpNetwork's core before its poll so network cleanup runs while
the owned poll is still alive.

Add a Linux-only regression test that keeps a duplicated listener
descriptor open and verifies that dropping the state removes its
listener and endpoint registrations.

Assisted-by: Claude:claude-fable-5
Assisted-by: Codex:gpt-5.6-sol
@Bronek
Bronek merged commit d70c8f9 into main Sep 4, 2026
3 checks passed
@Bronek
Bronek deleted the bronek/network_poll_refactor branch September 4, 2026 16:48
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.

3 participants