Fix data race in genny list Remove (#3233)

m3db · Feb 16, 2021 · e31d6dc · e31d6dc
1 parent d4ff5a6
commit e31d6dc
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 5 deletions.
diff --git a/src/dbnode/storage/id_list_gen.go b/src/dbnode/storage/id_list_gen.go
diff --git a/src/x/context/finalizeable_list_gen.go b/src/x/context/finalizeable_list_gen.go
diff --git a/src/x/generics/list/list.go b/src/x/generics/list/list.go
@@ -199,13 +199,16 @@ func (l *List) remove(e *Element) *Element {
 // It returns the element value e.Value.
 // The element must not be nil.
 func (l *List) Remove(e *Element) ValueType {
+	// read the value before returning to the pool to avoid a data race with another goroutine getting access to the
+	// list after it has been put back into the pool.
+	v := e.Value
 	if e.list == l {
 		// if e.list == l, l must have been initialized when e was inserted
 		// in l or l == nil (e is a zero Element) and l.remove will crash.
 		l.remove(e)
 		l.Pool.put(e)
 	}
-	return e.Value
+	return v
 }
 
 // PushFront inserts a new element e with value v at the front of list l and returns e.