Skip to content

Commit

Permalink
Fix data race when updating account (#4435)
Browse files Browse the repository at this point in the history
Fixes race that would make the `TestJetStreamJWTMove` test fail
sometimes:

[0]:
https://github.com/nats-io/nats-server/blob/f1bf4127c5be8005bf475cce422d9f66dd427da0/server/accounts.go#L3535
[1]:
https://github.com/nats-io/nats-server/blob/f1bf4127c5be8005bf475cce422d9f66dd427da0/server/server.go#L1902

 ```
=== FAIL: server TestJetStreamJWTMove/non-tiered/R1 (4.79s)
==================
WARNING: DATA RACE
Write at 0x00c0014631f8 by goroutine 22900:

github.com/nats-io/nats-server/v2/server.(*Server).updateAccountClaimsWithRefresh()
      /go/server/accounts.go:3535 +0x53dc

github.com/nats-io/nats-server/v2/server.(*Server).UpdateAccountClaims()
      /go/server/accounts.go:3074 +0x45

github.com/nats-io/nats-server/v2/server.(*Server).updateAccountWithClaimJWT()
      /go/server/server.go:1937 +0x3e5
  github.com/nats-io/nats-server/v2/server.(*Server).updateAccount()
      /go/server/server.go:1910 +0x1f1
  github.com/nats-io/nats-server/v2/server.(*Server).lookupAccount()
      /go/server/server.go:1875 +0x176
  github.com/nats-io/nats-server/v2/server.(*Server).LookupAccount()
      /go/server/server.go:1895 +0x2e4
  github.com/nats-io/nats-server/v2/server.(*Server).getRequestInfo()
      /go/server/jetstream_api.go:936 +0x2b4

github.com/nats-io/nats-server/v2/server.(*Server).jsStreamCreateRequest()
      /go/server/jetstream_api.go:1285 +0xca

github.com/nats-io/nats-server/v2/server.(*Server).jsStreamCreateRequest-fm()
      <autogenerated>:1 +0xcc

github.com/nats-io/nats-server/v2/server.(*Server).processJSAPIRoutedRequests()
      /go/server/jetstream_api.go:799 +0x60c

github.com/nats-io/nats-server/v2/server.(*Server).processJSAPIRoutedRequests-fm()
      <autogenerated>:1 +0x39

github.com/nats-io/nats-server/v2/server.(*Server).startGoRoutine.func1()
      /go/server/server.go:3604 +0x27d
 
Previous read at 0x00c0014631f8 by goroutine 22995:
  github.com/nats-io/nats-server/v2/server.(*Server).updateAccount()
      /go/server/server.go:1902 +0x6d
  github.com/nats-io/nats-server/v2/server.(*Server).lookupAccount()
      /go/server/server.go:1875 +0x176
  github.com/nats-io/nats-server/v2/server.(*Server).LookupAccount()
      /go/server/server.go:1895 +0x4e

github.com/nats-io/nats-server/v2/server.(*Server).updateInterestForAccountOnGateway()
      /go/server/leafnode.go:2030 +0x3a

github.com/nats-io/nats-server/v2/server.(*client).processGatewayRSub.func1()
      /go/server/gateway.go:1966 +0xc4
  runtime.deferreturn()
      /usr/local/go/src/runtime/panic.go:476 +0x32
  github.com/nats-io/nats-server/v2/server.(*client).parse()
      /go/server/parser.go:664 +0x40b7
  github.com/nats-io/nats-server/v2/server.(*client).readLoop()
      /go/server/client.go:1373 +0x1c98

github.com/nats-io/nats-server/v2/server.(*Server).createGateway.func1()
      /go/server/gateway.go:858 +0x37

github.com/nats-io/nats-server/v2/server.(*Server).startGoRoutine.func1()
      /go/server/server.go:3604 +0x27d
 
Goroutine 22900 (running) created at:
  github.com/nats-io/nats-server/v2/server.(*Server).startGoRoutine()
      /go/server/server.go:3600 +0x2f2

github.com/nats-io/nats-server/v2/server.(*Server).setJetStreamExportSubs()
      /go/server/jetstream_api.go:820 +0x178
  github.com/nats-io/nats-server/v2/server.(*Server).enableJetStream()
      /go/server/jetstream.go:425 +0xcf1
  github.com/nats-io/nats-server/v2/server.(*Server).EnableJetStream()
      /go/server/jetstream.go:217 +0x6f7
  github.com/nats-io/nats-server/v2/server.(*Server).Start()
      /go/server/server.go:2218 +0x1924
  github.com/nats-io/nats-server/v2/server.RunServer()
      /go/server/server_test.go:95 +0x30e
  github.com/nats-io/nats-server/v2/server.RunServerWithConfig()
      /go/server/server_test.go:117 +0x44

github.com/nats-io/nats-server/v2/server.createJetStreamSuperClusterWithTemplateAndModHook()
      /go/server/jetstream_helpers_test.go:449 +0x1331
  github.com/nats-io/nats-server/v2/server.TestJetStreamJWTMove.func1()
      /go/server/jetstream_jwt_test.go:303 +0x204
github.com/nats-io/nats-server/v2/server.TestJetStreamJWTMove.func3.2()
      /go/server/jetstream_jwt_test.go:409 +0x50
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1576 +0x216
  testing.(*T).Run.func1()
      /usr/local/go/src/testing/testing.go:1629 +0x47
 
Goroutine 22995 (running) created at:
  github.com/nats-io/nats-server/v2/server.(*Server).startGoRoutine()
      /go/server/server.go:3600 +0x2f2
  github.com/nats-io/nats-server/v2/server.(*Server).createGateway()
      /go/server/gateway.go:858 +0xf04
  github.com/nats-io/nats-server/v2/server.(*Server).solicitGateway()
      /go/server/gateway.go:707 +0x12e7

github.com/nats-io/nats-server/v2/server.(*Server).solicitGateways.func1()
      /go/server/gateway.go:643 +0x44

github.com/nats-io/nats-server/v2/server.(*Server).startGoRoutine.func1()
      /go/server/server.go:3604 +0x27d
==================
    testing.go:1446: race detected during execution of test
        --- FAIL: TestJetStreamJWTMove/non-tiered/R1 (4.79s)
 
=== FAIL: server TestJetStreamJWTMove/non-tiered (11.03s)
    testing.go:1446: race detected during execution of test
    --- FAIL: TestJetStreamJWTMove/non-tiered (11.03s)
 
=== FAIL: server TestJetStreamJWTMove (23.30s)
    testing.go:1446: race detected during execution of test
```
  • Loading branch information
derekcollison committed Aug 31, 2023
2 parents 0bd4763 + b8200d1 commit 7c8f402
Showing 1 changed file with 3 additions and 0 deletions.
3 changes: 3 additions & 0 deletions server/server.go
Expand Up @@ -1898,11 +1898,14 @@ func (s *Server) LookupAccount(name string) (*Account, error) {
// This will fetch new claims and if found update the account with new claims.
// Lock MUST NOT be held upon entry.
func (s *Server) updateAccount(acc *Account) error {
acc.mu.RLock()
// TODO(dlc) - Make configurable
if !acc.incomplete && time.Since(acc.updated) < time.Second {
acc.mu.RUnlock()
s.Debugf("Requested account update for [%s] ignored, too soon", acc.Name)
return ErrAccountResolverUpdateTooSoon
}
acc.mu.RUnlock()
claimJWT, err := s.fetchRawAccountClaims(acc.Name)
if err != nil {
return err
Expand Down

0 comments on commit 7c8f402

Please sign in to comment.