Skip to content

fix: don't release child namespace while a socket is still in middleware - #5588

Open
Jeremiah-D wants to merge 1 commit into
socketio:mainfrom
Jeremiah-D:fix/cleanup-namespace-middleware-race
Open

Jeremiah-D wants to merge 1 commit into
socketio:mainfrom
Jeremiah-D:fix/cleanup-namespace-middleware-race

Conversation

@Jeremiah-D

Copy link
Copy Markdown

With cleanupEmptyChildNamespaces on, a child namespace was released as soon as sockets.size hit 0, even while another socket was still in its (async) middleware. That socket then joined the released object, the next connection got a new object under the same name, broadcasts missed sockets, and the later cleanup deleted the _nsps entry by name, unregistering the newer object while its sockets were still connected.

The fix only cleans up once both connected and pre-connect sockets are gone, and only deletes the _nsps entry if it still points to the object being released.

Verified with the repro from #5587 (adapted into a test in test/namespaces.ts): it fails before the fix and passes after. The full namespaces suite passes (45 passing); the full package suite has one pre-existing failure in test/close.ts ("should be able to close sio sending a srv", timeout) that also fails on unmodified main.

Fixes #5587

With cleanupEmptyChildNamespaces, a namespace was released as soon as
sockets.size hit 0, even while another socket was still in its (async)
middleware. That socket then joined the released object, the next
connection got a new object under the same name, so broadcasts missed
sockets and a later cleanup deleted the _nsps entry by name,
unregistering the newer object while its sockets were still connected.

Only clean up once both connected and pre-connect sockets are gone,
and only delete the _nsps entry if it still points to the object
being released.

Fixes socketio#5587
Copilot AI balanced review requested due to automatic review settings November 5, 2026 08:12

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

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.

cleanupEmptyChildNamespaces releases a child namespace while a socket is in its middleware, and the cleanup unregisters a newer namespace by name

2 participants