Refuse connections once the server starts closing, so its close cannot hang #160

Merged

cruelacid merged 1 commit from worktree-nec-89-destroy-race into main 2026-09-24 22:33:54 +01:00

Conversation 0 Commits 1 Files changed 3 +132 -6

cruelacid commented 2026-09-24 21:39:05 +01:00

Owner

Copy link

Task: NEC-89

NEC-89's e2e hang was the test server's close(), not Obsidian. The close codes added in #157 showed it in soak run 710, which caught the hang twice in 40 iterations. The early disconnect was the server's own shutdown, code 1001 "server restarting", so the test was stuck in await server.close().

How it hung

1. WsServer.destroy() closes the sockets it has, then waits up to 2 s for each close to be acknowledged.
2. An upgrade that arrived during that wait was accepted and added to the client map.
3. The final clear() then dropped it without ever closing it.
4. The harness's http.close() waits for every connection to end, so it waited on that socket until vitest's 300 s timeout.

The fix. destroy() now marks the server as closing before it touches a socket.

• A later upgrade gets a 503.
• A handshake that finishes after that point is closed with destroy()'s own code.
• Anything still in the client map after the wait is terminated.

Production impact. Small. Production closes the HTTP server before closing sockets and exits after a 10 s drain, so at worst a late socket slowed a deploy's shutdown.

Tests. shutdown-race.test.ts makes the race deterministic by pausing the first client's socket, so destroy() is still waiting when the second client connects.

• Both tests fail on the unfixed code; one of them hangs exactly as CI did.
• Each of the three guards makes them pass on its own.

Still to prove. An 80-iteration soak dispatched on this branch. The current rate is about 1 hang in 20 iterations, so 0 hangs in 80 would be about 1.6% likely if nothing had changed.

Changelog

NONE

🤖 Generated with Claude Code

https://claude.ai/code/session_01Q6gkmHZ3pYvsZCCpiA9qe9

Task: NEC-89 NEC-89's e2e hang was the test server's `close()`, not Obsidian. The close codes added in #157 showed it in soak run 710, which caught the hang twice in 40 iterations. The early disconnect was the server's own shutdown, code 1001 "server restarting", so the test was stuck in `await server.close()`. **How it hung** 1. `WsServer.destroy()` closes the sockets it has, then waits up to 2 s for each close to be acknowledged. 2. An upgrade that arrived during that wait was accepted and added to the client map. 3. The final `clear()` then dropped it without ever closing it. 4. The harness's `http.close()` waits for every connection to end, so it waited on that socket until vitest's 300 s timeout. **The fix.** `destroy()` now marks the server as closing before it touches a socket. - A later upgrade gets a 503. - A handshake that finishes after that point is closed with `destroy()`'s own code. - Anything still in the client map after the wait is terminated. **Production impact.** Small. Production closes the HTTP server before closing sockets and exits after a 10 s drain, so at worst a late socket slowed a deploy's shutdown. **Tests.** `shutdown-race.test.ts` makes the race deterministic by pausing the first client's socket, so `destroy()` is still waiting when the second client connects. - Both tests fail on the unfixed code; one of them hangs exactly as CI did. - Each of the three guards makes them pass on its own. **Still to prove.** An 80-iteration soak dispatched on this branch. The current rate is about 1 hang in 20 iterations, so 0 hangs in 80 would be about 1.6% likely if nothing had changed. ## Changelog NONE 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01Q6gkmHZ3pYvsZCCpiA9qe9

cruelacid added the

area:server

label 2026-09-24 21:39:05 +01:00

cruelacid added 1 commit 2026-09-24 21:39:05 +01:00

Refuse connections once the server starts closing, so its close cannot hang ... 1370c2d966

NEC-89's e2e hang was never Obsidian. Soak run 710 caught it twice in 40
iterations, and the close codes #157 added showed the early disconnect
was the server's own shutdown (1001 "server restarting"): the test had
moved on to `await server.close()`, and that never finished.

WsServer.destroy() closes the sockets in its client map, then waits up
to 2 s for each close to be acknowledged. An upgrade that landed in the
wait was accepted and added to the map, and the final clear() dropped it
without closing it. The test harness then waits on http.close(), which
waits for every connection, so it waited on that socket until vitest's
300 s timeout. external-edit and repro-runaway close their server the
moment their vaults come up, while the second vault is still connecting,
which is why the hang only ever struck those two files.

destroy() now marks the server closing before it touches a socket. An
upgrade after that gets a 503. One whose handshake finishes after it is
closed with destroy()'s own code, and anything still in the map after
the wait is terminated rather than cleared past.

Production mostly avoided this: createShutdown closes the HTTP server
before closeSockets, and exits after a 10 s drain. At worst a late
socket slowed a deploy's shutdown.

shutdown-race.test.ts makes the race deterministic by pausing the first
client's socket, so its close acknowledgement never arrives and destroy()
sits in its wait while a second client connects. Both tests fail on the
unfixed code, one of them by hanging exactly as CI did. Each of the three
guards passes them alone, so the tests pin the fix as a whole, not
each guard.

The e2e README's reading of the hang as a renderer freeze is corrected,
and kept as the record of how it was wrong.

Task: NEC-89

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q6gkmHZ3pYvsZCCpiA9qe9

cruelacid force-pushed worktree-nec-89-destroy-race from 1370c2d966 to 151bfc729b 2026-09-24 22:28:10 +01:00 Compare

cruelacid merged commit 151bfc729b into main 2026-09-24 22:33:54 +01:00

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

area:docs

docs/

  

area:identity

packages/identity

  

area:ops

deploy/, CI, hosting

  

area:plugin

packages/plugin

  

area:server

packages/server

  

channel:community

Obsidian forum, Obsidian Discord, Reddit

  

channel:direct

Cold outreach to named firms

  

channel:owned

nectenda.com pages, the feedback board

  

channel:press

Hacker News, newsletters, outlets, YouTube creators

  

channel:social

Bluesky, Mastodon, X, YouTube accounts

  

e2ee-constrained

End-to-end encryption changes the shape of the answer

  

gate:at-ga

Held until Gates A-C close and terms/privacy leave DRAFT

  

gate:pre-ga

Safe to do before the launch gates close

  

marketing

Reach and positioning; work published off nectenda.com

  

parity

Relay feature-parity programme

  

relay:absent

4 - Not supported, not requested, not planned at Relay

  

relay:planned

2 - On Relay's roadmap (Active/Planned) or has an open PR

  

relay:requested

3 - Asked for by Relay users or under 'Considering'; not committed

  

relay:supported

1 - Relay ships this today

  

risk:additive

New message type, table or column; backward compatible

  

risk:contract

Changes something existing accounts or stored data depend on

  

risk:none

Client-only; the server is untouched

  

usability

How the product is understood and learned, not what it does

No labels

area:docs

area:identity

area:ops

area:plugin

area:server

channel:community

channel:direct

channel:owned

channel:press

channel:social

e2ee-constrained

gate:at-ga

gate:pre-ga

marketing

parity

relay:absent

relay:planned

relay:requested

relay:supported

risk:additive

risk:contract

risk:none

usability

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

Nectenda/nectenda!160

No description provided.