Skip to content

Data race on Client.conn between the auto-reconnect monitor and Client.Close (v0.9.0) #883

Description

@FingersXed

Data race on Client.conn between the auto-reconnect monitor and Client.Close (v0.9.0)

Following up on #882 with a second, distinct data race I found in the same area
(secure-channel / connection lifecycle) during the same -race run.

Version / environment

  • github.com/gopcua/opcua v0.9.0 (latest release). Present on current master too
    (the conn field and the Dial/Close accesses are unchanged).
  • Go 1.26, reproduced on darwin/arm64 (the racing code is arch-independent).
  • SecurityMode = None, a single *opcua.Client. The application drives connect /
    disconnect / reconnect from one goroutine, serialized behind its own mutex — it
    never calls the client concurrently from the caller side.

Summary

The race detector reports a data race on the unexported field Client.conn, entirely
within gopcua's own goroutines:

  • the background auto-reconnect monitor ((*Client).monitor, started internally by
    Connect) re-dials on connection loss and writes c.conn in (*Client).Dial, while
  • (*Client).Close reads c.conn to close the underlying connection.

Close cancels the monitor's context (c.mcancel()) but does not wait for the monitor
goroutine to exit before touching c.conn, and c.conn is not guarded by any mutex. So a
Close that overlaps an in-flight reconnect races with the monitor's Dial.

Race report (-race, application frames anonymized as <app>)

WARNING: DATA RACE
Write at 0x… by goroutine 91:
  github.com/gopcua/opcua.(*Client).Dial()
      github.com/gopcua/opcua@v0.9.0/client.go:621        // c.conn, err = c.cfg.dialer.Dial(...)
  github.com/gopcua/opcua/uacp.(*Dialer).Dial()
      github.com/gopcua/opcua@v0.9.0/uacp/conn.go:95
  github.com/gopcua/opcua.(*Client).monitor()
      github.com/gopcua/opcua@v0.9.0/client.go:418         // reconnect loop re-dials
  github.com/gopcua/opcua.(*Client).Connect.func1.gowrap1()
      github.com/gopcua/opcua@v0.9.0/client.go:283

Previous read at 0x… by goroutine 87:
  github.com/gopcua/opcua.(*Client).Close()
      github.com/gopcua/opcua@v0.9.0/client.go:668         // if c.conn != nil { c.conn.Close() }
  <app> (*Connector).Disconnect()                          // calls client.Close(ctx)
  <app> (*Connector).Connect()                             // disconnect-before-reconnect path
  <app> reconnect routine — single serialized goroutine

Goroutine 91 (running) created at:                         // the monitor goroutine
  github.com/gopcua/opcua.(*Client).Connect.func1()
      github.com/gopcua/opcua@v0.9.0/client.go:283         // go c.monitor(mctx)
  github.com/gopcua/opcua.(*Client).Connect()
      github.com/gopcua/opcua@v0.9.0/client.go:258
  <app> (*Connector).Connect()                             // opcua.NewClient(...).Connect(ctx)
  <app> reconnect routine

Goroutine 87 (running):
  the application's single serialized connect/disconnect goroutine,
  calling Connector.Disconnect() -> Client.Close().

Analysis

c.conn is a plain field with no mutex (unlike secureChannel, which has
SecureChannel()/setSecureChannel() accessors, or the subMux-guarded subscription
state). Every access is unsynchronized — the write in Dial:

// (*Client).Dial — client.go:621
c.conn, err = c.cfg.dialer.Dial(ctx, c.endpointURL)   // WRITE

and the read in Close:

// (*Client).Close — client.go:650-669
if c.mcancel != nil {
    c.mcancel()          // cancels the monitor context, but does NOT wait for it to exit
}
...
if c.conn != nil {       // READ, races with monitor's Dial
    c.conn.Close()
}

With AutoReconnect enabled (the default), the monitor goroutine started by Connect
(go c.monitor(mctx)) re-dials on connection loss (monitorDial → writes c.conn).
Because Close only cancels the monitor's context and then immediately reads c.conn
without joining the goroutine, the two overlap. The application cannot prevent this from
the caller side: even a single, fully-serialized caller trips it, because the monitor
goroutine runs independently inside gopcua and Close does not synchronize with it.

A fix would either guard c.conn with a mutex (as secureChannel already is), or have
Close wait for the monitor goroutine to actually return before closing the connection
(e.g. a done-channel / sync.WaitGroup the monitor signals on exit).

Reproduction

  1. Create a client with default options (AutoReconnect on) and a short channel lifetime.
  2. Connect, then repeatedly drop/restore the connection (or issue Disconnect/Connect
    while the server bounces) so Close overlaps the monitor's reconnect Dial.
  3. Build/run with -race.

The race fires when a Close lands while the monitor is mid-reconnect.

Impact

Rare in production (needs a Close concurrent with an in-flight reconnect), but a genuine
data race on connection state → risk of use-after-close / corrupted connection handling.

Related

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