Skip to content

fix disconnect event in tcp connector#86

Merged
ltitanb merged 1 commit into
mainfrom
fix-backlog
Jun 5, 2026
Merged

fix disconnect event in tcp connector#86
ltitanb merged 1 commit into
mainfrom
fix-backlog

Conversation

@ltitanb

@ltitanb ltitanb commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Fix TCP disconnect reporting for connections that are dropped outside read-side event handling.

Issue

TcpConnector only emitted PollEvent::Disconnect when a connection was found dead while processing a mio event. Connections can also be dropped during send-side work:

  • broadcast writes that fail immediately
  • single-target writes that fail immediately
  • send backlog flushes that fail
  • backlog limit enforcement

Those paths removed the connection from ConnectionManager internally, but the caller never received a disconnect event. Callers that maintain their own token-to-peer state could then keep stale connection state forever.

Root Cause

disconnect_at_index was called directly from write/backlog paths without notifying the application. Only read-side polling called the user's handler with PollEvent::Disconnect.

Accepted inbound sockets also did not get TCP_USER_TIMEOUT, while outbound sockets did. That made unclean inbound connection loss slower to detect under network failure.

Fix

  • Add a pending-disconnect queue to ConnectionManager.
  • Route send-side and backlog-side connection drops through a helper that records the token before removing the connection.
  • Drain pending disconnects through poll_with and poll_with_produce, so callers receive the same PollEvent::Disconnect regardless of where the dead socket was detected.
  • Apply TCP_USER_TIMEOUT to accepted inbound sockets as well as outbound sockets.

This preserves the existing public API while fixing the event stream seen by callers.

Validation

  • Added backlog_disconnect_is_reported, which forces a backlog-triggered disconnect and asserts that PollEvent::Disconnect is emitted for the accepted stream token.
  • Ran cargo test -p flux-network.
  • Ran cargo clippy -p flux-network --all-targets -- -D warnings.
  • Ran git diff --check.

@ltitanb ltitanb requested a review from a team June 5, 2026 14:09

@devin-ai-integration devin-ai-integration Bot left a comment

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.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 4 additional findings.

Open in Devin Review

@ltitanb ltitanb merged commit 957cd5f into main Jun 5, 2026
2 checks passed
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