From eaf5de05e974d3b173e4bd5b1a7a73f21d232f10 Mon Sep 17 00:00:00 2001 From: Ivan Kozlovic Date: Fri, 25 Mar 2022 12:51:52 -0600 Subject: [PATCH] Fixed data race caused by moving some code inside startGoRoutine startGoRoutine will execute the closed function as a go routine, so passing copyBytes(msg) as the argument caused a race. The copy needs to be done before startGoRoutine, as it was before being changed in https://github.com/nats-io/nats-server/pull/2925 Here is the race observed: ``` ================== WARNING: DATA RACE Write at 0x00c0001dd930 by goroutine 367: runtime.racewriterange() :1 +0x29 internal/poll.ignoringEINTRIO() /home/travis/.gimme/versions/go1.17.8.linux.amd64/src/internal/poll/fd_unix.go:582 +0x454 internal/poll.(*FD).Read() /home/travis/.gimme/versions/go1.17.8.linux.amd64/src/internal/poll/fd_unix.go:163 +0x26 net.(*netFD).Read() /home/travis/.gimme/versions/go1.17.8.linux.amd64/src/net/fd_posix.go:56 +0x50 net.(*conn).Read() /home/travis/.gimme/versions/go1.17.8.linux.amd64/src/net/net.go:183 +0xb0 net.(*TCPConn).Read() :1 +0x64 github.com/nats-io/nats-server/v2/server.(*client).readLoop() /home/travis/gopath/src/github.com/nats-io/nats-server/server/client.go:1188 +0x8f7 github.com/nats-io/nats-server/v2/server.(*Server).createLeafNode.func1() /home/travis/gopath/src/github.com/nats-io/nats-server/server/leafnode.go:904 +0x5d Previous read at 0x00c0001dd930 by goroutine 93: runtime.slicecopy() /home/travis/.gimme/versions/go1.17.8.linux.amd64/src/runtime/slice.go:284 +0x0 github.com/nats-io/nats-server/v2/server.copyBytes() /home/travis/gopath/src/github.com/nats-io/nats-server/server/util.go:282 +0x10b github.com/nats-io/nats-server/v2/server.(*Server).jsStreamListRequest.func1() /home/travis/gopath/src/github.com/nats-io/nats-server/server/jetstream_api.go:1613 +0x26 Goroutine 367 (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:3017 +0x86 github.com/nats-io/nats-server/v2/server.(*Server).createLeafNode() /home/travis/gopath/src/github.com/nats-io/nats-server/server/leafnode.go:904 +0x1b08 github.com/nats-io/nats-server/v2/server.(*Server).startLeafNodeAcceptLoop.func1() /home/travis/gopath/src/github.com/nats-io/nats-server/server/leafnode.go:604 +0x4b github.com/nats-io/nats-server/v2/server.(*Server).acceptConnections.func1() /home/travis/gopath/src/github.com/nats-io/nats-server/server/server.go:2122 +0x58 Goroutine 93 (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:3017 +0x86 github.com/nats-io/nats-server/v2/server.(*Server).jsStreamListRequest() /home/travis/gopath/src/github.com/nats-io/nats-server/server/jetstream_api.go:1613 +0xbf1 github.com/nats-io/nats-server/v2/server.(*Server).jsStreamListRequest-fm() /home/travis/gopath/src/github.com/nats-io/nats-server/server/jetstream_api.go:1554 +0xcc github.com/nats-io/nats-server/v2/server.(*jetStream).apiDispatch() /home/travis/gopath/src/github.com/nats-io/nats-server/server/jetstream_api.go:680 +0xcf0 github.com/nats-io/nats-server/v2/server.(*jetStream).apiDispatch-fm() /home/travis/gopath/src/github.com/nats-io/nats-server/server/jetstream_api.go:652 +0xcc github.com/nats-io/nats-server/v2/server.(*client).deliverMsg() /home/travis/gopath/src/github.com/nats-io/nats-server/server/client.go:3181 +0xbde github.com/nats-io/nats-server/v2/server.(*client).processMsgResults() /home/travis/gopath/src/github.com/nats-io/nats-server/server/client.go:4164 +0xe1e github.com/nats-io/nats-server/v2/server.(*client).processInboundLeafMsg() /home/travis/gopath/src/github.com/nats-io/nats-server/server/leafnode.go:2183 +0x7eb github.com/nats-io/nats-server/v2/server.(*client).processInboundMsg() /home/travis/gopath/src/github.com/nats-io/nats-server/server/client.go:3498 +0xb1 github.com/nats-io/nats-server/v2/server.(*client).parse() /home/travis/gopath/src/github.com/nats-io/nats-server/server/parser.go:497 +0x3886 github.com/nats-io/nats-server/v2/server.(*client).readLoop() /home/travis/gopath/src/github.com/nats-io/nats-server/server/client.go:1228 +0x1669 github.com/nats-io/nats-server/v2/server.(*Server).createLeafNode.func1() /home/travis/gopath/src/github.com/nats-io/nats-server/server/leafnode.go:904 +0x5d ================== testing.go:1152: race detected during execution of test ``` Signed-off-by: Ivan Kozlovic --- server/jetstream_api.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/server/jetstream_api.go b/server/jetstream_api.go index 19278353..51044b17 100644 --- a/server/jetstream_api.go +++ b/server/jetstream_api.go @@ -1609,8 +1609,9 @@ func (s *Server) jsStreamListRequest(sub *subscription, c *client, _ *Account, s // Clustered mode will invoke a scatter and gather. if s.JetStreamIsClustered() { - // Need to copy the msg before sending.. - s.startGoRoutine(func() { s.jsClusteredStreamListRequest(acc, ci, filter, offset, subject, reply, copyBytes(msg)) }) + // Need to copy these off before sending.. don't move this inside startGoRoutine!!! + msg = copyBytes(msg) + s.startGoRoutine(func() { s.jsClusteredStreamListRequest(acc, ci, filter, offset, subject, reply, msg) }) return } @@ -3464,8 +3465,10 @@ func (s *Server) jsConsumerListRequest(sub *subscription, c *client, _ *Account, // Clustered mode will invoke a scatter and gather. if s.JetStreamIsClustered() { + // Need to copy these off before sending.. don't move this inside startGoRoutine!!! + msg = copyBytes(msg) s.startGoRoutine(func() { - s.jsClusteredConsumerListRequest(acc, ci, offset, streamName, subject, reply, copyBytes(msg)) + s.jsClusteredConsumerListRequest(acc, ci, offset, streamName, subject, reply, msg) }) return }