Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

NRG: Fix data race in processAppendEntry #5970

Merged
merged 1 commit into from
Oct 7, 2024
Merged

NRG: Fix data race in processAppendEntry #5970

merged 1 commit into from
Oct 7, 2024

Conversation

MauriceVanVeen
Copy link
Member

Fix a data race in processAppendEntry. Since the ae could be cached and returned to the pool in applyCommit, need to make sure we make a copy for ourselves here.

WARNING: DATA RACE
Write at 0x00c0157328c8 by goroutine 160360:
  github.com/nats-io/nats-server/v2/server.(*appendEntry).returnToPool()
      /home/travis/build/nats-io/nats-server/server/raft.go:2194 +0x17a4
  github.com/nats-io/nats-server/v2/server.(*raft).applyCommit()
      /home/travis/build/nats-io/nats-server/server/raft.go:2935 +0x16e8
  github.com/nats-io/nats-server/v2/server.(*raft).ResumeApply()
      /home/travis/build/nats-io/nats-server/server/raft.go:1001 +0x2c4
  github.com/nats-io/nats-server/v2/server.(*stream).processSnapshot.func1()
      /home/travis/build/nats-io/nats-server/server/jetstream_cluster.go:8307 +0x167
  runtime.deferreturn()
      /home/travis/sdk/go1.23.0/src/runtime/panic.go:605 +0x5d
  github.com/nats-io/nats-server/v2/server.(*jetStream).applyStreamEntries()
      /home/travis/build/nats-io/nats-server/server/jetstream_cluster.go:3157 +0x2b57
  github.com/nats-io/nats-server/v2/server.(*jetStream).monitorStream()
      /home/travis/build/nats-io/nats-server/server/jetstream_cluster.go:2448 +0x4492
  github.com/nats-io/nats-server/v2/server.(*jetStream).processClusterCreateStream.func1()
      /home/travis/build/nats-io/nats-server/server/jetstream_cluster.go:3888 +0x55
  github.com/nats-io/nats-server/v2/server.(*Server).startGoRoutine.func1()
      /home/travis/build/nats-io/nats-server/server/server.go:3871 +0x59
Previous read at 0x00c0157328c8 by goroutine 160359:
  github.com/nats-io/nats-server/v2/server.(*raft).processAppendEntry()
      /home/travis/build/nats-io/nats-server/server/raft.go:3577 +0x55c4
  github.com/nats-io/nats-server/v2/server.(*raft).processAppendEntries()
      /home/travis/build/nats-io/nats-server/server/raft.go:2046 +0x1f2
  github.com/nats-io/nats-server/v2/server.(*raft).runAsFollower()
      /home/travis/build/nats-io/nats-server/server/raft.go:2061 +0x446
  github.com/nats-io/nats-server/v2/server.(*raft).run()
      /home/travis/build/nats-io/nats-server/server/raft.go:1961 +0x4c9
  github.com/nats-io/nats-server/v2/server.(*raft).run-fm()
      <autogenerated>:1 +0x33
  github.com/nats-io/nats-server/v2/server.(*Server).startGoRoutine.func1()
      /home/travis/build/nats-io/nats-server/server/server.go:3871 +0x59
Goroutine 160360 (running) created at:
  github.com/nats-io/nats-server/v2/server.(*Server).startGoRoutine()
      /home/travis/build/nats-io/nats-server/server/server.go:3869 +0x1ba
  github.com/nats-io/nats-server/v2/server.(*jetStream).processClusterCreateStream()
      /home/travis/build/nats-io/nats-server/server/jetstream_cluster.go:3887 +0x1ff8
  github.com/nats-io/nats-server/v2/server.newFileStoreWithCreated()
      /home/travis/build/nats-io/nats-server/server/filestore.go:437 +0x10d5
  github.com/nats-io/nats-server/v2/server.(*jetStream).createRaftGroup()
      /home/travis/build/nats-io/nats-server/server/jetstream_cluster.go:2081 +0xbfd
  github.com/nats-io/nats-server/v2/server.(*jetStream).processClusterCreateStream()
      /home/travis/build/nats-io/nats-server/server/jetstream_cluster.go:3743 +0x3ae
  github.com/nats-io/nats-server/v2/server.(*jetStream).processStreamAssignment()
      /home/travis/build/nats-io/nats-server/server/jetstream_cluster.go:3461 +0x88e
  github.com/nats-io/nats-server/v2/server.(*jetStream).applyMetaSnapshot()
      /home/travis/build/nats-io/nats-server/server/jetstream_cluster.go:1637 +0x13a5
  github.com/nats-io/nats-server/v2/server.(*jetStream).applyMetaEntries()
      /home/travis/build/nats-io/nats-server/server/jetstream_cluster.go:1901 +0x124
  github.com/nats-io/nats-server/v2/server.(*jetStream).monitorCluster()
      /home/travis/build/nats-io/nats-server/server/jetstream_cluster.go:1392 +0x10d0
  github.com/nats-io/nats-server/v2/server.(*jetStream).monitorCluster-fm()
      <autogenerated>:1 +0x33
  github.com/nats-io/nats-server/v2/server.(*Server).startGoRoutine.func1()
      /home/travis/build/nats-io/nats-server/server/server.go:3871 +0x59
Goroutine 160359 (running) created at:
  github.com/nats-io/nats-server/v2/server.(*Server).startGoRoutine()
      /home/travis/build/nats-io/nats-server/server/server.go:3869 +0x1ba
  github.com/nats-io/nats-server/v2/server.(*Server).startRaftNode()
      /home/travis/build/nats-io/nats-server/server/raft.go:524 +0x1f58
  github.com/nats-io/nats-server/v2/server.(*jetStream).createRaftGroup()
      /home/travis/build/nats-io/nats-server/server/jetstream_cluster.go:2108 +0x1331
  fmt.Sscanf()
      /home/travis/sdk/go1.23.0/src/fmt/scan.go:114 +0x238f
  github.com/nats-io/nats-server/v2/server.(*fileStore).recoverFullState()
      /home/travis/build/nats-io/nats-server/server/filestore.go:1769 +0x224b
  github.com/nats-io/nats-server/v2/server.newFileStoreWithCreated()
      /home/travis/build/nats-io/nats-server/server/filestore.go:437 +0x10d5
  github.com/nats-io/nats-server/v2/server.(*jetStream).createRaftGroup()
      /home/travis/build/nats-io/nats-server/server/jetstream_cluster.go:2081 +0xbfd
  github.com/nats-io/nats-server/v2/server.(*jetStream).processClusterCreateStream()
      /home/travis/build/nats-io/nats-server/server/jetstream_cluster.go:3743 +0x3ae
  github.com/nats-io/nats-server/v2/server.(*jetStream).processStreamAssignment()
      /home/travis/build/nats-io/nats-server/server/jetstream_cluster.go:3461 +0x88e
  github.com/nats-io/nats-server/v2/server.(*jetStream).applyMetaSnapshot()
      /home/travis/build/nats-io/nats-server/server/jetstream_cluster.go:1637 +0x13a5
  github.com/nats-io/nats-server/v2/server.(*jetStream).applyMetaEntries()
      /home/travis/build/nats-io/nats-server/server/jetstream_cluster.go:1901 +0x124
  github.com/nats-io/nats-server/v2/server.(*jetStream).monitorCluster()
      /home/travis/build/nats-io/nats-server/server/jetstream_cluster.go:1392 +0x10d0
  github.com/nats-io/nats-server/v2/server.(*jetStream).monitorCluster-fm()
      <autogenerated>:1 +0x33
  github.com/nats-io/nats-server/v2/server.(*Server).startGoRoutine.func1()
      /home/travis/build/nats-io/nats-server/server/server.go:3871 +0x59

Signed-off-by: Maurice van Veen github@mauricevanveen.com

@MauriceVanVeen
Fix data race in processAppendEntry
029c2f0 
Signed-off-by: Maurice van Veen <github@mauricevanveen.com>
@MauriceVanVeen MauriceVanVeen changed the title Fix data race in processAppendEntry NRG: Fix data race in processAppendEntry Oct 7, 2024
@MauriceVanVeen MauriceVanVeen marked this pull request as ready for review October 7, 2024 15:46
@MauriceVanVeen MauriceVanVeen requested a review from a team as a code owner October 7, 2024 15:46
derekcollison
derekcollison approved these changes Oct 7, 2024
View reviewed changes
Copy link
Member

@derekcollison derekcollison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@derekcollison derekcollison merged commit 32f0773 into main Oct 7, 2024
5 checks passed
@derekcollison derekcollison deleted the maurice/fix-race-process-append-entry branch October 7, 2024 16:37
@neilalexander neilalexander mentioned this pull request Oct 8, 2024
Merged
neilalexander added a commit that referenced this pull request Oct 9, 2024
@neilalexander
Cherry-picks for 2.10.22-RC.1 (#5979)
7f381e0 
Includes the following:

- #5944
- #5945
- #5939
- #5935
- #5960
- #5970
- #5971
- #5963
- #5973
- #5978

Signed-off-by: Neil Twigg <neil@nats.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@derekcollison derekcollison derekcollison approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@MauriceVanVeen @derekcollison