Two problems surfaced once disconnect stopped crashing.
Disconnecting the automatic local connection left auto_connection_client
pointing at the freed client and auto_connection_active still true.
RescanDevices and GetDetectionPercent/String trust those, so a rescan
afterward called into freed memory and aborted (std::system_error from
send_in_progress.lock() on a destroyed mutex). Clear the pointer and the
flag in the teardown when the removed client is the auto connection.
onDetectionEnded never lowered the detection view; only the progress
handler did, at 100 percent. A real detection reaches 100, but a client
teardown borrows DETECTION_STARTED/COMPLETE to make plugins release the
controllers and sends no progress, so the "detecting devices" overlay
stuck on after a disconnect until Cancel. Lower it unconditionally when
the sequence ends.
When a connection dropped, the listener thread deleted all of that
server's controllers immediately, before anything else was notified, so
the GUI, ResourceManager, and plugins kept dereferencing freed
controllers. Clicking Disconnect had the same problem and also ran the
teardown inline on the GUI thread, where UpdateDeviceList's
DEVICE_LIST_UPDATED is a BlockingQueuedConnection back to that thread,
so the device pages were not torn down before the controllers were
freed and a later queued onDetectionEnded dereferenced them.
Neither path frees controllers inline now. NetworkClient moves them to
an orphaned list and a dedicated teardown thread frees them: plugins
are warned first, then the same DETECTION_STARTED, UpdateDeviceList,
DETECTION_COMPLETE sequence a rescan runs tears down the device pages
that still reference the controllers, and only then are they freed, so
a controller always outlives its page. Both the connection-loss path
and the disconnect button queue to that thread, so the blocking handler
runs on the GUI thread while the controllers are still alive, and
StopClient's join stays off the GUI and listener threads.
StopClient guards profilemanager_thread and nulls it after the delete,
but nothing ever set it, so destroying a client that never started
read an uninitialized pointer. client_active was unset until
StartClient and is read on the same path.
StopServer guards the pointer and nulls it after the delete, but
nothing ever set it, so destroying a server that never started read an
uninitialized pointer.
RGBController_UpdateCallback sent the SignalUpdate packet synchronously
via SendRequest_RGBController_SignalUpdate, which runs inside
RGBController::SignalUpdate. A blocking send there (slow client) keeps
SignalCalls > 0, so WaitSignalCalls -- the callback drain that
Unregister/ClearCallbacks/Shutdown run while tearing callbacks down on a
rescan -- waits forever. That is the client/server rescan deadlock.
Each client now owns an outbound queue and a dedicated send thread. The
signal path builds the packet (brief AccessMutex, as before) and queues
it; the client's own thread does the blocking send. A slow client backs
up only its own queue and blocks only its own thread, so the signal path
and the callback drain are never blocked.
UPDATELEDS packets coalesce per controller (a newer frame replaces the
pending one), so a stalled client's queue stays bounded to one packet
per controller instead of growing without bound; non-coalescable events
fall back to a hard cap with drop-oldest. The client destructor stops
the thread with shutdown(SD_BOTH), which unblocks an in-flight send, so
the join at teardown cannot hang. A queue entry owns its packet header
and data buffer; the send thread frees the buffer after the send, so
there is no copy and the packet outlives the controller safely.
Follow-up: the send still takes the shared send_in_progress, so a slow
client can still stall sends to others; routing response sends through
the same per-client path would make that a per-client lock.
net_port: add SD_BOTH (SHUT_RDWR) for the POSIX socket shims.
Adam Honse