Skip to content

HubConnection: complete mutex discipline for Heartbeat/ControlChannel fields #93

@ptone

Description

@ptone

Summary

PR #60 (fix: synchronize HubConnection control channel lifecycle) introduced mutex-guarded writes to HubConnection.Heartbeat and HubConnection.ControlChannel in Start() and Stop(), along with a ccWg WaitGroup to prevent goroutine leaks across reconnects. However, several read sites in other files still access these fields without acquiring hc.mu, which means data races remain possible under the Go memory model.

Unsynchronized reads of guarded fields

The following locations read conn.Heartbeat or conn.ControlChannel without holding conn.mu.RLock():

  • handlers.go:179-180handleHubConnections() reads both fields for status reporting
  • server.go:1303-1304getFirstHeartbeat() reads conn.Heartbeat (only hubMu is held, not conn.mu)
  • server.go:1336-1337logHubConnections() reads both fields for logging
  • handlers.go:2091-2092forceHeartbeatAll() reads conn.Heartbeat (only hubMu is held)

The hubMu lock protects the connection map, but does not synchronize access to fields within a HubConnection. Since Stop() now writes these fields under hc.mu.Lock(), all readers must also acquire hc.mu.RLock().

Suggested fix

Add accessor methods (e.g., hc.GetHeartbeat(), hc.GetControlChannel()) that acquire hc.mu.RLock(), similar to the existing GetStatus() pattern, and update all call sites.

Missing race-condition test coverage

There are no tests exercising concurrent Start/Stop/Reinitialize paths. A go test -race scenario that calls Reinitialize while a Connect goroutine is in-flight would validate the fix and catch regressions.

Unguarded writes in Reinitialize

In Reinitialize() (hub_connection.go:203-209), fields like Credentials, BrokerID, SecretKey, and HubClient are written without holding hc.mu. This is likely safe today because Stop() serializes via ccWg.Wait() before Start() is called, but it is inconsistent with the locking discipline established by this PR and could become a problem if call patterns change.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions