Comfy-Org / Comfy-Org/ComfyUI

WebSocket reconnect can unregister the replacement client

Open
#16,010 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
133k
Forks
15.7k
Avg merge
1d 7h
Merged PRs (30d)
158

Description

Reconnecting with the same `clientId` can unregister the replacement WebSocket

- A reconnect replaces `self.sockets[sid]` but does not retire the old handler.
- When the old handler exits, its unconditional cleanup removes the new socket and metadata.
- The stale handler can also write its feature flags into the replacement connection's metadata.

Evidence and suggested direction

At `server.py:273-283`, a supplied `clientId` causes `self.sockets.pop(sid, None)`, after which the new `ws` and fresh metadata are stored under the same key. Popping the mapping does not close or stop the displaced WebSocket handler, which continues its `async for` loop at `server.py:295`.

Both handlers therefore own the same `sid`. If the old connection receives its first text frame, `server.py:299-305` writes through `self.sockets_metadata[sid]`, which now belongs to the replacement connection. More importantly, whenever the old handler exits, `server.py:324-326` unconditionally pops that `sid`, removing the live replacement socket and metadata. Later targeted and broadcast events no longer reach the current reconnect.

Cleanup should be ownership-conditional, for example removing the entry only when `self.sockets.get(sid) is ws`, and the reconnect path should explicitly close or otherwise retire the displaced socket. A regression test can connect twice with one `clientId`, close the first connection, then assert that the second remains registered and receives a server event; a second assertion can ensure stale feature negotiation cannot mutate the replacement's metadata.

Reviewed at `12d5279438bfefc058a269eae805ceab6047777f`.

Contributor guide

Open the contributing guide

Research direction

Start with server.py:273-283 to trace reconnect replacement, then read the handler loop at server.py:295 and cleanup at server.py:324-326. Reproduce the issue by connecting twice with one clientId, closing the first connection, and checking that the second remains registered and receives a server event; also verify stale feature negotiation cannot change its metadata.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend, networking
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.