Skip to content

Commit

Permalink
Fix race in reload and gateway sublist check (#4127)
Browse files Browse the repository at this point in the history 
Fixes the following race: during reload account sublist can be changed:
https://github.com/nats-io/nats-server/blob/26994655963d9bdc31150c6b101d11573c4fc667/server/reload.go#L1598-L1610
so this can become a race while checking interest in the gateway code
here:
https://github.com/nats-io/nats-server/blob/79de3302be4572ab1ee8da4a5f8860689c0087e8/server/gateway.go#L2683

```
=== RUN   TestJetStreamSuperClusterPeerReassign
==================
WARNING: DATA RACE
Write at 0x00c0010854f0 by goroutine 15595:
  github.com/nats-io/nats-server/v2/server.(*Server).reloadAuthorization.func2()
      /home/travis/gopath/src/github.com/nats-io/nats-server/server/reload.go:1610 +0x486
  sync.(*Map).Range()
      /home/travis/.gimme/versions/go1.19.8.linux.amd64/src/sync/map.go:354 +0x225
  github.com/nats-io/nats-server/v2/server.(*Server).reloadAuthorization()
      /home/travis/gopath/src/github.com/nats-io/nats-server/server/reload.go:1594 +0x35d
  github.com/nats-io/nats-server/v2/server.(*Server).applyOptions()
      /home/travis/gopath/src/github.com/nats-io/nats-server/server/reload.go:1454 +0xf4
  github.com/nats-io/nats-server/v2/server.(*Server).reloadOptions()
      /home/travis/gopath/src/github.com/nats-io/nats-server/server/reload.go:908 +0x204
  github.com/nats-io/nats-server/v2/server.(*Server).ReloadOptions()
      /home/travis/gopath/src/github.com/nats-io/nats-server/server/reload.go:847 +0x4a4
  github.com/nats-io/nats-server/v2/server.(*Server).Reload()
      /home/travis/gopath/src/github.com/nats-io/nats-server/server/reload.go:782 +0x125
  github.com/nats-io/nats-server/v2/server.(*cluster).removeJetStream()
      /home/travis/gopath/src/github.com/nats-io/nats-server/server/jetstream_helpers_test.go:1498 +0x310
  github.com/nats-io/nats-server/v2/server.TestJetStreamSuperClusterPeerReassign()
      /home/travis/gopath/src/github.com/nats-io/nats-server/server/jetstream_super_cluster_test.go:395 +0xa38
  testing.tRunner()
      /home/travis/.gimme/versions/go1.19.8.linux.amd64/src/testing/testing.go:1446 +0x216
  testing.(*T).Run.func1()
      /home/travis/.gimme/versions/go1.19.8.linux.amd64/src/testing/testing.go:1493 +0x47
Previous read at 0x00c0010854f0 by goroutine 15875:
  github.com/nats-io/nats-server/v2/server.(*Server).gatewayHandleSubjectNoInterest()
      /home/travis/gopath/src/github.com/nats-io/nats-server/server/gateway.go:2683 +0x12d
  github.com/nats-io/nats-server/v2/server.(*client).processInboundGatewayMsg()
      /home/travis/gopath/src/github.com/nats-io/nats-server/server/gateway.go:2980 +0x595
  github.com/nats-io/nats-server/v2/server.(*client).processInboundMsg()
      /home/travis/gopath/src/github.com/nats-io/nats-server/server/client.go:3532 +0xc7
  github.com/nats-io/nats-server/v2/server.(*client).parse()
      /home/travis/gopath/src/github.com/nats-io/nats-server/server/parser.go:497 +0x34f9
  github.com/nats-io/nats-server/v2/server.(*client).readLoop()
      /home/travis/gopath/src/github.com/nats-io/nats-server/server/client.go:1284 +0x17e8
  github.com/nats-io/nats-server/v2/server.(*Server).createGateway.func1()
      /home/travis/gopath/src/github.com/nats-io/nats-server/server/gateway.go:858 +0x37
Goroutine 15595 (running) created at:
  testing.(*T).Run()
      /home/travis/.gimme/versions/go1.19.8.linux.amd64/src/testing/testing.go:1493 +0x75d
  testing.runTests.func1()
      /home/travis/.gimme/versions/go1.19.8.linux.amd64/src/testing/testing.go:1846 +0x99
  testing.tRunner()
      /home/travis/.gimme/versions/go1.19.8.linux.amd64/src/testing/testing.go:1446 +0x216
  testing.runTests()
      /home/travis/.gimme/versions/go1.19.8.linux.amd64/src/testing/testing.go:1844 +0x7ec
  testing.(*M).Run()
      /home/travis/.gimme/versions/go1.19.8.linux.amd64/src/testing/testing.go:1726 +0xa84
  github.com/nats-io/nats-server/v2/server.TestMain()
      /home/travis/gopath/src/github.com/nats-io/nats-server/server/sublist_test.go:1577 +0x292
  main.main()
      _testmain.go:3615 +0x324
Goroutine 15875 (running) created at:
  github.com/nats-io/nats-server/v2/server.(*Server).startGoRoutine()
      /home/travis/gopath/src/github.com/nats-io/nats-server/server/server.go:3098 +0x88
  github.com/nats-io/nats-server/v2/server.(*Server).createGateway()
      /home/travis/gopath/src/github.com/nats-io/nats-server/server/gateway.go:858 +0xfc4
  github.com/nats-io/nats-server/v2/server.(*Server).startGatewayAcceptLoop.func1()
      /home/travis/gopath/src/github.com/nats-io/nats-server/server/gateway.go:553 +0x48
  github.com/nats-io/nats-server/v2/server.(*Server).acceptConnections.func1()
      /home/travis/gopath/src/github.com/nats-io/nats-server/server/server.go:2184 +0x58
==================
    testing.go:1319: race detected during execution of test
--- FAIL: TestJetStreamSuperClusterPeerReassign (2.08s)
```
  • Loading branch information
@derekcollison
derekcollison committed May 3, 2023
2 parents 8cb3293 + 938ffcb commit b61e411
Showing 1 changed file with 5 additions and 6 deletions.
11 changes: 5 additions & 6 deletions server/gateway.go
View file
Expand Up @@ -2680,12 +2680,11 @@ func (s *Server) gatewayHandleSubjectNoInterest(c *client, acc *Account, accName
// If there is no subscription for this account, we would normally
// send an A-, however, if this account has the internal subscription
// for service reply, send a specific RS- for the subject instead.
hasSubs := acc.sl.Count() > 0
if !hasSubs {
acc.mu.RLock()
hasSubs = acc.siReply != nil
acc.mu.RUnlock()
}
// Need to grab the lock here since sublist can change during reload.
acc.mu.RLock()
hasSubs := acc.sl.Count() > 0 || acc.siReply != nil
acc.mu.RUnlock()

// If there is at least a subscription, possibly send RS-
if hasSubs {
sendProto := false
Expand Down

0 comments on commit b61e411

Please sign in to comment.