shipmates ships connect to Fleet Command over outbound websocket tunnels. That way nothing on the ship side has to be exposed to the internet. I didn’t write that tunnel layer. I used rancher/remotedialer, the same library Rancher uses to reach downstream cluster agents. I spent years around that code at Rancher, so it was the obvious pick. What I didn’t expect was for my CI’s race job to find two data races in it in the first week.

CI was the only place -race could run

-race needs cgo, which needs a C compiler. Neither machine I was writing shipmates on had one at the time. Every agent session that tried go test -race hit cgo: C compiler "gcc" not found. So I moved shipmates CI onto my ephemerd runners in PR #26. At the same time I added a dedicated race job on linux-x64. Today it runs in a golang:1.26.6-bookworm container, which ships gcc:

race:
  name: race (linux-x64)
  runs-on: [self-hosted, linux, x64]
  container: golang:1.26.6-bookworm
  timeout-minutes: 30
  steps:
    - uses: actions/checkout@v4
    # ...
    - name: Test with race detector
      run: go test -race -count=1 ./...

The shipmates fleet tests open and tear down real tunnels quickly. That job went red inside remotedialer v0.6.1, not in my code.

Race 1: Session.pingCancel

ConnectToProxyWithDialer builds the racing pair itself. It runs Serve on a goroutine and defers Close:

go func() { session.Serve(ctx) }()   // Serve -> startPings: s.pingCancel = cancel   (write, no lock)
defer session.Close()                // Close -> stopPings:  if s.pingCancel == nil  (read, no lock)

Here’s v0.6.1:

func (s *Session) startPings(rootCtx context.Context) {
	ctx, cancel := context.WithCancel(rootCtx)
	s.pingCancel = cancel
	s.pingWait.Add(1)
	go func() {
		defer s.pingWait.Done()
		// ticker loop ...
	}()
}

func (s *Session) stopPings() {
	if s.pingCancel == nil {
		return
	}
	s.pingCancel()
	s.pingWait.Wait()
}

Session already embeds a sync.RWMutex and the neighboring fields use it. pingCancel just never got put under it. In production the write and the read are separated by the lifetime of a session. You only see it when sessions open and close quickly, like in a reconnect storm or a test suite driving real tunnels.

Race 2: connection.err

doTunnelClose does an unguarded check-then-set on c.err. Three callers can hit it at the same time:

• a pipe copy noticing its peer failed
• Session.Close tearing everything down
• serveMessage on a protocol error

connection had no synchronization at all. The kicker is that remotedialer’s own TestExceedBuffer trips this race as soon as you turn the detector on. Upstream’s Makefile test target didn’t use -race. That’s how both of these survived.

Fork, reproduce first, then fix

I forked it and did the red/green thing on purpose. I wanted the detector’s report against unfixed code on record:

• cf2b335: add TestSessionCloseDoesNotRaceServeStartup and turn on -race in make test. This commit fails.
• 7e65b78: lock pingCancel, add an errMu around connection.err. Green.

The test does what ConnectToProxyWithDialer does in a loop. That gives the detector repeated shots at the startup window:

func TestSessionCloseDoesNotRaceServeStartup(t *testing.T) {
	for i := 0; i < 200; i++ {
		conn := &blockingWSConn{closed: make(chan struct{})}
		s := newSession(int64(i), "race-probe", conn)
		s.client = true // only the client side runs startPings

		ctx, cancel := context.WithCancel(context.Background())
		served := make(chan struct{})
		go func() {
			defer close(served)
			_, _ = s.Serve(ctx)
		}()

		s.Close() // the deferred Close, arriving while Serve is still starting

		_ = conn.Close()
		cancel()
		<-served
		s.pingWait.Wait()
	}
}

The fix for stopPings grabs the cancel func under the lock and releases the lock before cancelling. The ping goroutine’s callees also take that lock (getSessionKeys via sendSyncConnections). So pingWait.Wait() is never awaited while holding it. Nil-ing the field makes a second Close a no-op.

Then in shipmates, 84e0d0e pointed the dependency at the fork:

replace github.com/rancher/remotedialer => github.com/luthermonson/remotedialer v0.2.6-0.20260805014355-7e65b789c524

v0.6.1 and upstream main had zero Go-file difference at that point. So the fork was exactly the pinned version plus these fixes. Upstream PR to follow, drop the replace when it lands. Done, I thought.

The third commit

About three hours after those two commits, I opened rancher/remotedialer#265 with them. That was a minute after pinning shipmates to them. Within the hour the new test caught one more thing on a machine with more hardware threads. It failed about half of runs at 32. Look at what 7e65b78 did to startPings:

s.Lock()
s.pingCancel = cancel
s.Unlock()
s.pingWait.Add(1)   // still outside the lock

stopPings now reads pingCancel under the lock, sees it non-nil and goes on to pingWait.Wait(). But nothing says the Add(1) has happened yet. A Close that lands between the Unlock and the Add calls Wait concurrently with Add on a zero counter. The race detector flags it. At runtime it can panic with sync: WaitGroup misuse: Add called concurrently with Wait. That happens in exactly the reconnect storm the PR was supposed to fix.

The third commit, 00b5151, moves the Add inside the same critical section:

s.Lock()
s.pingCancel = cancel
s.pingWait.Add(1)
s.Unlock()

Now observing a non-nil cancel implies the Add happened-before the Wait. I pushed it to the PR branch and moved on.

What I didn’t do was bump the replace. shipmates stayed pinned to 7e65b789, the middle commit of the PR. Its stamp was about three hours before the PR even existed. I had the connection.err fix in full and two-thirds of the pingCancel fix. I didn’t notice until a review pass right before the v0.5.0 release, thirteen days later. PR #50 bumped it to the PR head:

replace github.com/rancher/remotedialer => github.com/luthermonson/remotedialer v0.2.6-0.20260805053951-00b5151280f8

Then I checked that the resolved module carries Add(1) between s.Lock() and s.Unlock(). I didn’t trust the pseudo-version.

Seeing all three states

To write this up I ran the same reproducer against all three versions straight out of the module cache. That’s upstream v0.6.1, the middle commit and the PR head. Each got 20 iterations with -race on Linux.

v0.6.1 is race 1, exactly as described. Write in startPings, read in stopPings:

WARNING: DATA RACE
Write at 0x00c000416de0 by goroutine 2007:
  github.com/rancher/remotedialer.(*Session).startPings()
      session.go:173 +0x58
  github.com/rancher/remotedialer.(*Session).Serve()
      session.go:223 +0x57

Previous read at 0x00c000416de0 by goroutine 1614:
  github.com/rancher/remotedialer.(*Session).stopPings()
      session.go:213 +0x31
  github.com/rancher/remotedialer.(*Session).Close()
      session.go:314 +0x3b
...
--- FAIL: TestSessionCloseDoesNotRaceServeStartup (0.01s)
    testing.go:1712: race detected during execution of test

The middle commit, the one I’d been shipping, still fails. But now it’s the WaitGroup. session.go:231 is s.pingWait.Wait() and session.go:179 is the s.pingWait.Add(1) that sat outside the lock:

WARNING: DATA RACE
Write at 0x00c00047e6a0 by goroutine 6057:
  runtime.racewrite()
      <autogenerated>:1 +0x1e
  github.com/rancher/remotedialer.(*Session).stopPings()
      session.go:231 +0xa7
  github.com/rancher/remotedialer.(*Session).Close()
      session.go:327 +0x3b

Previous read at 0x00c00047e6a0 by goroutine 6276:
  runtime.raceread()
      <autogenerated>:1 +0x1e
  github.com/rancher/remotedialer.(*Session).startPings()
      session.go:179 +0xaf
  github.com/rancher/remotedialer.(*Session).Serve()
      session.go:236 +0x57
...
--- FAIL: TestSessionCloseDoesNotRaceServeStartup (0.01s)
    testing.go:1712: race detected during execution of test

And the head is clean:

ok  	github.com/rancher/remotedialer	1.218s

The lesson I keep relearning: a replace to a pseudo-version is a pin to a commit, not to “my fix”. If the branch moves after you pin, you’re on whatever you pinned. Nothing will tell you.

Side effect: go install stops working

A replace directive in your go.mod means people can’t go install your module:

$ go install github.com/luthermonson/shipmates@v0.5.0
go: github.com/luthermonson/shipmates@v0.5.0 (in github.com/luthermonson/shipmates@v0.5.0):
	The go.mod file for the module providing named packages contains one or
	more replace directives. It must not contain directives that would cause
	it to be interpreted differently than if it were the main module.

Release binaries are fine. goreleaser builds in the module, so it honors the replace. The README and the v0.5.0 release notes (f2c4bdf) say to grab a release binary. The other option is git clone and go build until #265 merges. As of this writing the upstream PR is open with no reviews. It’s showing merge conflicts, so the fork stays for now.

Takeaways

• If your project depends on something concurrent, run your tests with -race even if upstream doesn’t. A suite that churns real connections finds things code reading won’t.
• Put the reproducer in its own commit before the fix. It made the upstream PR easy to explain, and it’s how I could rerun all three states here.
• After you pin a fork, check the pin again when the PR is final.

How I measured

The race output above came from three steps:

• copy each version of the module out of my Go module cache (rancher/remotedialer@v0.6.1, luthermonson/remotedialer@...-7e65b789c524 and @...-00b5151280f8)
• drop the fork’s session_close_race_test.go into the two that didn’t have it
• run go test -race -count=20 -run TestSessionCloseDoesNotRaceServeStartup . in each

That ran under WSL2 with Go 1.26.0, gcc 15.2 and 32 hardware threads on a Ryzen 9 5950X. Paths in the output are trimmed.

Links

• rancher/remotedialer#265 (the upstream PR, three commits)
• shipmates PR #26 (CI on ephemerd, the race job)
• shipmates PR #50 (bumping the pin to the complete fix)
• go.mod reference: replace directive