Skip to content

fix(net): send our FIN when the guest closes before any payload - #159

Open
TadahiroYamamura wants to merge 1 commit into
earendil-works:mainfrom
TadahiroYamamura:fix/tcp-close-before-payload
Open

TadahiroYamamura wants to merge 1 commit into
earendil-works:mainfrom
TadahiroYamamura:fix/tcp-close-before-payload

Conversation

@TadahiroYamamura

Copy link
Copy Markdown

A4 of #155.

Problem
Before the first payload there is no upstream socket, so when the guest closes before sending anything (nc -z, etc.), Gondolin never sends a FIN back. The guest socket stays in FIN-WAIT-2 with no FIN even after 150s, and the natTable entry stays behind too.

Fix
In this case (a graceful close with no upstream socket and no protocol determined yet), send a FIN to the guest right away. The state becomes FIN_WAIT, so the existing "both sides closed" path removes the session. So that the FIN we send ACKs the guest's FIN, advancing myAck when the guest's FIN is accepted now happens before onTcpClose is called.

Tests
On both the HTTP path and the mapped TCP path, a guest FIN before any payload gets exactly one FIN back (with the right seq and ack), and natTable and tcpSessions end up empty. A guest RST before any payload gets no reply.

Please check

  • On its own, this PR removes the session without waiting for LAST-ACK (the ACK of our FIN), so the guest's final ACK is answered with the no-session RST. The guest is in TIME_WAIT by then, so we expect no impact on applications. Once fix(network-stack): drop guest-closed sessions once our FIN is acked #157 is merged, sending just the FIN and leaving the rest to fix(network-stack): drop guest-closed sessions once our FIN is acked #157's LAST-ACK handling would match the RFC
  • The cleanup relies on re-entering from the onTcpClose callback into handleTcpEnd. This is the same pattern as the existing handleTcpConnect calling handleTcpConnected from inside onTcpConnect
  • Because myAck is now advanced first, on every path the ack number of any segment sent from inside onTcpClose includes the guest's FIN. All existing tests pass
  • This conflicts with fix(network-stack): drop guest-closed sessions once our FIN is acked #157 on the lines where the guest's FIN is accepted (the same few lines in network-stack.ts). If both are merged, the correct order is "advance myAck → guestFinReceived = true → onTcpClose"

Suggested CHANGELOG entry
Fix guest TCP connections closed before sending any data never getting a FIN back, which left the guest in FIN-WAIT-2 and the session behind.

🤖 AI (Claude Code) was used for the investigation and for writing the code.

Contribution Agreement

Please ensure this PR follows the guidelines in CONTRIBUTING.md.

By submitting this pull request, I confirm the following:

I understand that the entity Earendil Inc. (incorporated in the state of Delaware in 2025) needs some rights from me in order to utilize my contributions in this PR. As a contributor I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Earendil Inc. can use, modify, copy, and redistribute my contributions, under Earendil Inc.'s choice of terms.

Before the first payload there is no upstream socket, so when the guest
closed first Gondolin never sent a FIN: the guest stayed in FIN-WAIT-2
and the natTable entry stayed behind. Send our FIN right away in that
case and let the existing "both sides closed" path drop the session.
Advance myAck before calling onTcpClose so that FIN acks the guest's.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TadahiroYamamura added a commit to TadahiroYamamura/gondolin that referenced this pull request Oct 19, 2026
…FIN is acked

When the guest's FIN arrived in FIN_WAIT, the session was dropped even
if our FIN was still unacked (CLOSING in RFC 9293), so a retransmitted
FIN or the final ACK hit no session. Drop it right away only when our
FIN is already acked; otherwise leave it to the ACK handling added in
the previous commit. Combined with earendil-works#159, every guest-initiated close of
a connection without an upstream takes this path.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
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.

1 participant