Skip to content

Commit

Permalink
Fix data race in FQDN ruleSyncTracker (#5583)
Browse files Browse the repository at this point in the history
ruleSyncTracker.Run() can update ruleSyncTracker.dirtyRules in-place,
while ruleSyncTracker.getDirtyRules() returns the pointer of the set
which could be read by other goroutines and leads to a data race like
below:

WARNING: DATA RACE
Write at 0x00c000dd8180 by goroutine 276:
  runtime.mapdelete_faststr()
      /usr/local/go/src/runtime/map_faststr.go:301 +0x0
  k8s.io/apimachinery/pkg/util/sets.Set[go.shape.string].Delete()
      /root/go/pkg/mod/k8s.io/[email protected]/pkg/util/sets/set.go:62 +0x2ae
  antrea.io/antrea/pkg/agent/controller/networkpolicy.(*ruleSyncTracker).Run()
      /root/antrea/pkg/agent/controller/networkpolicy/fqdn.go:570 +0x1d2
  antrea.io/antrea/pkg/agent/controller/networkpolicy.(*fqdnController).runRuleSyncTracker()
      /root/antrea/pkg/agent/controller/networkpolicy/fqdn.go:584 +0x4a
  antrea.io/antrea/pkg/agent/controller/networkpolicy.TestSyncDirtyRules.func1.4()
      /root/antrea/pkg/agent/controller/networkpolicy/fqdn_test.go:495 +0x44

Previous read at 0x00c000dd8180 by goroutine 271:
  reflect.maplen()
      /usr/local/go/src/runtime/map.go:1411 +0x0
  reflect.Value.lenNonSlice()
      /usr/local/go/src/reflect/value.go:1720 +0x324
  reflect.Value.Len()
      /usr/local/go/src/reflect/value.go:1709 +0x158f
  reflect.deepValueEqual()
      /usr/local/go/src/reflect/deepequal.go:139 +0x1571
  reflect.DeepEqual()
      /usr/local/go/src/reflect/deepequal.go:237 +0x38b
  github.com/stretchr/testify/assert.ObjectsAreEqual()
      /root/go/pkg/mod/github.com/stretchr/[email protected]/assert/assertions.go:65 +0x172
  github.com/stretchr/testify/assert.Equal()
      /root/go/pkg/mod/github.com/stretchr/[email protected]/assert/assertions.go:414 +0x1f7
  antrea.io/antrea/pkg/agent/controller/networkpolicy.TestSyncDirtyRules.func1()
      /root/antrea/pkg/agent/controller/networkpolicy/fqdn_test.go:517 +0xb6f
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1595 +0x238
  testing.(*T).Run.func1()
      /usr/local/go/src/testing/testing.go:1648 +0x44

Signed-off-by: Quan Tian <[email protected]>
  • Loading branch information
tnqn committed Oct 17, 2023
1 parent 9f782a6 commit 3a25934
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion pkg/agent/controller/networkpolicy/fqdn.go
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,8 @@ func (rst *ruleSyncTracker) subscribe(waitCh chan error, dirtyRules sets.Set[str
func (rst *ruleSyncTracker) getDirtyRules() sets.Set[string] {
rst.mutex.RLock()
defer rst.mutex.RUnlock()
return rst.dirtyRules
// Must return a copy as the set can be updated in-place by Run func.
return rst.dirtyRules.Clone()
}

func (rst *ruleSyncTracker) Run(stopCh <-chan struct{}) {
Expand Down

0 comments on commit 3a25934

Please sign in to comment.