* fix(rest): make WebSocket monitor listener teardown safe under concurrency
The commit-1 tests and race detector exposed three related defects in the
monitor listener's Close, whose check-then-act select was unsound:
1. With a full event queue (a slow or disconnecting client), the
`case <-ml.c:` branch received a buffered event, mistook it for the
closed-channel signal, and skipped both RemoveListener and close: the
listener stayed registered, and the hub actor then blocked forever
inside Receive on the next dispatch, wedging every hub consumer.
2. The read and write pumps both call Close when the connection drops;
two callers could pass the select's default branch together and the
second close panicked with "close of closed channel".
3. Even made exactly-once, closing the event queue was still unsafe:
RemoveListener returns once its op is queued, not processed, so the
hub actor could be mid-send into the queue when Close closed it
(send on closed channel panic inside the actor).
Fix: Close deregisters and closes a `done` channel via sync.Once; the
event queue is never closed. Receive and Delete select their enqueue
against done and return errListenerClosed when aborted, so the hub
removes the listener itself even if RemoveListener has not run yet, and
a blocked enqueue aborts instead of wedging the actor. The write pump
exits on done and sends the closing frame.
Each defect has a regression test that fails against the previous code
(verified: no-op close delivered post-close events, concurrent closes
panicked, and close raced the actor's blocked send). The write pump
keeps its c-close branch for safety; the black-box monitor tests from
commit 1 pass unchanged.
* fix(rest): close listener done channel before enqueueing removal
Copilot review on PR #623 correctly identified a remaining deadlock in
msgListener.Close: when the hub actor is blocked inside this listener's
Receive (full event queue) and the hub op queue is also full,
RemoveListener cannot enqueue, so execution never reached close(ml.done) --
the only operation able to release the actor. Under the same slow-client
condition, concurrent dispatchers keeping the hub queue saturated made
Close hang permanently.
Fix: close the done channel (guarded by the existing sync.Once) before
queueing the RemoveListener op. The blocked actor is released first, keeps
draining the op queue, and the removal can then be queued even when the
queue is full. As a side effect, a Close racing a not-yet-processed
AddListener now also deregisters correctly, since the removal op can no
longer be overtaken by the earlier registration op.
Regression test TestMsgListenerCloseFullHubOpQueue wedges the actor in
Receive with a saturated op queue before calling Close; verified it fails
("Close deadlocked") against the previous ordering and passes with -race
after the fix, along with the full pkg/rest and pkg/msghub suites.
The black-box monitor tests showed socketv1_controller.go and
socketv2_controller.go were near-identical: same constants (identical
values), upgrader, read/write pumps, Close, and handler bodies, differing
only in the event payload shape and v1 dropping deletions.
Extract the shared machinery into socket_controller.go:
- msgListener[T] generic listener with toStored/toDeleted payload
transforms; a nil toDeleted preserves the socketv1 behavior of ignoring
deletions
- socketReadPump / socketWritePump free functions (WriteJSON is
payload-type agnostic)
- serveMonitor shared handler body (upgrade, gauge tracking, listener
registration, pump wiring)
The version files shrink to their listener constructors and the four
exported handlers, whose signatures are unchanged. v1 payloads are now
shaped at enqueue time rather than write time; the JSON on the wire is
identical, pinned by the commit-1 tests, which pass unmodified.
Adds direct unit tests for the extracted listener logic: watches()
filtering, Receive/Delete filtering + payload shaping, nil-toDeleted
deletion dropping, and sequential Close idempotency.
James Hillyerd