libn/networkdb: fix data race in GetTableByNetwork

corhere · corhere · commit ec65f2d21b83 · 2025-05-07T11:28:17.000-04:00
The function was accessing the index map without holding the mutex, so
it would race any mutation to the database indexes. Fetch the reference
to the tree's root while holding a read lock. Since the radix tree is
immutable, taking a reference to the root is equivalent to starting a
read-only database transaction, providing a consistent view of the data
at a snapshot in time, even as the live state is mutated concurrently.

Also optimize the WalkTable function by leveraging the immutability of
the radix tree.

Signed-off-by: Cory Snider &lt;csnider@mirantis.com&gt;
diff --git a/libnetwork/networkdb/networkdb.go b/libnetwork/networkdb/networkdb.go
@@ -429,8 +429,11 @@ type TableElem struct {
 // GetTableByNetwork walks the networkdb by the give table and network id and
 // returns a map of keys and values
 func (nDB *NetworkDB) GetTableByNetwork(tname, nid string) map[string]*TableElem {
+	nDB.RLock()
+	root := nDB.indexes[byNetwork].Root()
+	nDB.RUnlock()
 	entries := make(map[string]*TableElem)
-	nDB.indexes[byTable].Root().WalkPrefix([]byte(fmt.Sprintf("/%s/%s", tname, nid)), func(k []byte, v *entry) bool {
+	root.WalkPrefix([]byte(fmt.Sprintf("/%s/%s", tname, nid)), func(k []byte, v *entry) bool {
 		if v.deleting {
 			return false
 		}
@@ -585,21 +588,14 @@ func (nDB *NetworkDB) deleteNodeTableEntries(node string) {
 // value. The walk stops if the passed function returns a true.
 func (nDB *NetworkDB) WalkTable(tname string, fn func(string, string, []byte, bool) bool) error {
 	nDB.RLock()
-	values := make(map[string]*entry)
-	nDB.indexes[byTable].Root().WalkPrefix([]byte("/"+tname), func(path []byte, v *entry) bool {
-		values[string(path)] = v
-		return false
-	})
+	root := nDB.indexes[byTable].Root()
 	nDB.RUnlock()
-
-	for k, v := range values {
-		params := strings.Split(k[1:], "/")
+	root.WalkPrefix([]byte("/"+tname), func(path []byte, v *entry) bool {
+		params := strings.Split(string(path[1:]), "/")
 		nid := params[1]
 		key := params[2]
-		if fn(nid, key, v.value, v.deleting) {
-			return nil
-		}
-	}
+		return fn(nid, key, v.value, v.deleting)
+	})
 
 	return nil
 }
diff --git a/libnetwork/networkdb/networkdb_test.go b/libnetwork/networkdb/networkdb_test.go
@@ -428,6 +428,22 @@ func TestNetworkDBCRUDMediumCluster(t *testing.T) {
 
 	dbs := createNetworkDBInstances(t, n, "node", DefaultConfig())
 
+	// Shake out any data races.
+	done := make(chan struct{})
+	defer close(done)
+	for _, db := range dbs {
+		go func(db *NetworkDB) {
+			for {
+				select {
+				case <-done:
+					return
+				default:
+				}
+				_ = db.GetTableByNetwork("test_table", "network1")
+			}
+		}(db)
+	}
+
 	for i := 0; i < n; i++ {
 		for j := 0; j < n; j++ {
 			if i == j {
