Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automated cherry pick of #5566 and #5583: Fix deadlock when accessing dirtyRules in fqdn controller #5580

Conversation

luolanzone
Copy link
Contributor

@luolanzone luolanzone commented Oct 16, 2023

Cherry pick of #5566 and #5583 on release-1.12.

#5566: Fix deadlock when accessing dirtyRules in fqdn controller
#5583: Fix data race in FQDN ruleSyncTracker

For details on the cherry pick process, see the cherry pick requests page.

Signed-off-by: Dyanngg <dingyang@vmware.com>
@luolanzone luolanzone added the kind/cherry-pick Categorizes issue or PR as related to the cherry-pick of a bug fix from the main branch to a release label Oct 16, 2023
@luolanzone luolanzone requested review from tnqn and Dyanngg October 16, 2023 04:00
@tnqn
Copy link
Member

tnqn commented Oct 16, 2023

/skip-all

@tnqn
Copy link
Member

tnqn commented Oct 16, 2023

#5566 introduced another data race, hold on backporting it until it's fixed.

Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

revoke approval

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/apimachinery@v0.26.4/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/testify@v1.8.4/assert/assertions.go:65 +0x172
  github.com/stretchr/testify/assert.Equal()
      /root/go/pkg/mod/github.com/stretchr/testify@v1.8.4/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 <qtian@vmware.com>
Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Appended the 2nd fix

@luolanzone luolanzone changed the title Automated cherry pick of #5566: Fix deadlock when accessing dirtyRules in fqdn controller Automated cherry pick of #5566 and #5583: Fix deadlock when accessing dirtyRules in fqdn controller Oct 17, 2023
@tnqn
Copy link
Member

tnqn commented Oct 17, 2023

/test-all

@tnqn
Copy link
Member

tnqn commented Oct 17, 2023

/skip-all

1 similar comment
@tnqn
Copy link
Member

tnqn commented Oct 17, 2023

/skip-all

@tnqn tnqn merged commit 3a25934 into antrea-io:release-1.12 Oct 17, 2023
@luolanzone luolanzone deleted the automated-cherry-pick-of-#5566-upstream-release-1.12 branch January 4, 2024 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/cherry-pick Categorizes issue or PR as related to the cherry-pick of a bug fix from the main branch to a release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants