From ca2a961fa728404c90cb8faee2210940e230fc2f Mon Sep 17 00:00:00 2001 From: Ivan Kozlovic Date: Mon, 25 Sep 2023 16:11:09 -0600 Subject: [PATCH] [FIXED] JetStream: stream assignment data race Two go routines could possibly execute the stream assignment at the same time. A WaitGroup was used to prevent that, but an issue caused the data race and possible concurrent execution. Signed-off-by: Ivan Kozlovic --- server/stream.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/server/stream.go b/server/stream.go index 2180dc96..b907e8de 100644 --- a/server/stream.go +++ b/server/stream.go @@ -404,12 +404,21 @@ func (a *Account) addStreamWithAssignment(config *StreamConfig, fsConfig *FileSt } // Make sure we are ok when these are done in parallel. - v, loaded := jsa.inflight.LoadOrStore(cfg.Name, &sync.WaitGroup{}) + // We used to call Add(1) in the "else" clause of the "if loaded" + // statement. This caused a data race because it was possible + // that one go routine stores (with count==0) and another routine + // gets "loaded==true" and calls wg.Wait() while the other routine + // then calls wg.Add(1). It also could mean that two routines execute + // the rest of the code concurrently. + swg := &sync.WaitGroup{} + swg.Add(1) + v, loaded := jsa.inflight.LoadOrStore(cfg.Name, swg) wg := v.(*sync.WaitGroup) if loaded { wg.Wait() + // This waitgroup is "thrown away" (since there was an existing one). + swg.Done() } else { - wg.Add(1) defer func() { jsa.inflight.Delete(cfg.Name) wg.Done()