Fix RegisterNotifier to use a copy of the tables to prevent data races (

#14716)

Signed-off-by: Manan Gupta <[email protected]>

go/vt/vttablet/tabletserver/schema/engine.go

@@ -21,6 +21,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"maps"
 	"net/http"
 	"strings"
 	"sync"
@@ -706,7 +707,8 @@ func (se *Engine) RegisterNotifier(name string, f notifier, runNotifier bool) {
 		created = append(created, table)
 	}
 	if runNotifier {
-		f(se.tables, created, nil, nil)
+		s := maps.Clone(se.tables)
+		f(s, created, nil, nil)
 	}
 }
 
@@ -734,10 +736,7 @@ func (se *Engine) broadcast(created, altered, dropped []*Table) {
 
 	se.notifierMu.Lock()
 	defer se.notifierMu.Unlock()
-	s := make(map[string]*Table, len(se.tables))
-	for k, v := range se.tables {
-		s[k] = v
-	}
+	s := maps.Clone(se.tables)
 	for _, f := range se.notifiers {
 		f(s, created, altered, dropped)
 	}
@@ -755,10 +754,7 @@ func (se *Engine) GetTable(tableName sqlparser.IdentifierCS) *Table {
 func (se *Engine) GetSchema() map[string]*Table {
 	se.mu.Lock()
 	defer se.mu.Unlock()
-	tables := make(map[string]*Table, len(se.tables))
-	for k, v := range se.tables {
-		tables[k] = v
-	}
+	tables := maps.Clone(se.tables)
 	return tables
 }
 

go/vt/vttablet/tabletserver/schema/engine_test.go

@@ -705,6 +705,29 @@ func AddFakeInnoDBReadRowsResult(db *fakesqldb.DB, value int) *fakesqldb.Expecte
 	))
 }
 
+// TestRegisterNotifier tests the functionality of RegisterNotifier
+// It also makes sure that writing to the tables map in the schema engine doesn't change the tables received by the notifiers.
+func TestRegisterNotifier(t *testing.T) {
+	// Create a new engine for testing
+	se := NewEngineForTests()
+	se.notifiers = map[string]notifier{}
+	se.tables = map[string]*Table{
+		"t1": nil,
+		"t2": nil,
+		"t3": nil,
+	}
+
+	var tablesReceived map[string]*Table
+	// Register a notifier and make it run immediately.
+	se.RegisterNotifier("TestRegisterNotifier", func(full map[string]*Table, created, altered, dropped []*Table) {
+		tablesReceived = full
+	}, true)
+
+	// Change the se.tables and make sure it doesn't affect the tables received by the notifier.
+	se.tables["t4"] = nil
+	require.Len(t, tablesReceived, 3)
+}
+
 // TestEngineMysqlTime tests the functionality of Engine.mysqlTime function
 func TestEngineMysqlTime(t *testing.T) {
 	tests := []struct {