fix(server): close the connection a device reconnect displaced

[otavio]

What

Stale device connections are now torn down when the device reconnects, instead of being
detected, logged and left resident. The connection map they live in became consistent
under concurrent access, which it had to be before eviction could be built on it.

Why

Investigating a managed deployment (58,670 devices, 51,187 online) turned up the ssh
service logging "Multiple connections stored for the same identifier." with "size":2 at
175 occurrences/hour. The manager detected the duplicate and only warned about it.

A device that reconnects before the server has noticed its previous socket is gone
registers a second connection under the same identifier. Only the last one is ever dialed,
so the earlier one is dead weight: it holds its socket and buffers until its own ping loop
times out — up to 35s away, and never at all for a peer that vanished without resetting the
connection.

The wider "104k connections for 58k devices" claim in the issue turned out to be a
measurement artifact (a gateway netns counts both legs of every proxied connection), so
this is not the multi-gigabyte win that issue proposed. These are the two real defects that
survived the analysis. Refs: shellhub-io/team#202 — cross-repo, so it will not auto-close.

Changes

• SyncSliceMap: Store and Delete were read-modify-write over a sync.Map. Two
  concurrent registrations for one key could drop one, leaving a live connection the
  manager never recorded and therefore never tears down. Guarded with a mutex, and both
  now report what they observed under the lock — Store returns the values it displaced,
  Delete the count that survived. Returning them is not a convenience: reading either
  back afterwards would race with the next caller, which is the bug being fixed.

• SyncSliceMap.Delete: drops the key once its last value is gone. Previously an
  entry stayed behind for every device the server had ever seen.

• Manager.evict: closes the connections a registration displaced, in both Set (v1
  revdial) and Bind (v2 yamux). It runs in its own goroutine — this is teardown of an
  already-abandoned connection, and revdial.Dialer.Close blocks until its owner observes
  the close. Neither belongs on the path registering the live connection.

• Offline reporting: DialerDoneCallback now fires only when Delete reports no
  connections remaining. Without this, evicting a stale connection would mark a device that
  had just reconnected as offline. It also fixes the pre-existing, rarer flap where a stale
  connection dying on its own timeout did the same.

Testing

go test -race ./ssh/... and golangci-lint run ./ssh/... are clean.

Worth a reviewer's attention:

• The eviction test was checked against a deliberately broken evict (close removed) and
  fails on both assertions there, so it is not vacuous.
• TestConcurrentStoreAndDeleteReportOneWinner pins the contract the offline decision
  depends on: across 50 concurrent teardowns of one key, exactly one Delete may observe
  it empty. If that ever reports more than one, devices get spurious offline writes.
• The size > 1 warning on the Dial path was deliberately kept. With eviction in place it
  should become rare enough to be a real signal rather than the background noise it is now
  — it is the cheapest way to tell whether this actually worked in production.

[github-actions]

Claude finished @otavio's task in 5m 42s —— View job

---

Code Review Complete

Reviewed 4 files across code quality, security, testing, language patterns, and architecture — no new issues found. The code looks good as-is.

A few things I specifically verified, since they carry the correctness of the fix:

• SyncSliceMap race-freedom — every method takes the sync.RWMutex, the map is lazily initialized under the write lock, and Store returns slices.Clone(...) so the displaced slice handed to evict never aliases the map's live backing array that later append/slices.DeleteFunc calls mutate in place.
• evict goroutine safety — both stored types (*revdial.Dialer, *yamux.Session) implement io.Closer; revdial.Dialer.Close's closeOnce + donec rendezvous means the displaced dialer's still-alive watcher goroutine consumes the close (or it's an idempotent no-op), so the fire-and-forget goroutine can't leak or deadlock.
• Offline gating — firing DialerDoneCallback only when Delete(...) == 0 is race-safe under the shared mutex and matches the contract TestConcurrentStoreAndDeleteReportOneWinner pins (exactly one concurrent Delete observes the key empty), so evicting a stale duplicate no longer marks a just-reconnected device offline.
• Cross-repo — cloud/ imports only dialer.Dialer/DialTo, never Manager/SyncSliceMap; the Store/Delete signature changes are internal to the package, and Set/Bind's public signatures are unchanged. No breakage.
• Tests — the eviction test polls with assert.Eventually (no raw sleeps), TestDeleteDropsTheKey inspects the internal map rather than just size, and the concurrent tests are serialized by the mutex rather than timing-dependent.

To request another review round, comment /review.

--- · fix/ssh-duplicate-device-connections