Skip to content

Conversation

jrajahalme
Copy link
Member

@jrajahalme jrajahalme commented Oct 3, 2024

Call queueNotifiedUsersCommit(..., wg) before starting go routing waiting on wg, as queueNotifiedUsersCommit() does wg.Add(1). This makes it impossible to hit this bug:

2024-10-03T08:45:07.562242086Z panic: sync: WaitGroup is reused before previous Wait has returned
2024-10-03T08:45:07.562251423Z
2024-10-03T08:45:07.562358242Z goroutine 11033 [running]:
2024-10-03T08:45:07.562532947Z sync.(*WaitGroup).Wait(0xc0041bfe90?)
2024-10-03T08:45:07.562538587Z 	/usr/local/go/src/sync/waitgroup.go:120 +0x74
2024-10-03T08:45:07.562541644Z github.com/cilium/cilium/pkg/policy.(*SelectorCache).UpdateIdentities.func1(0xc01829a4c0)
2024-10-03T08:45:07.562544629Z 	/go/src/github.com/cilium/cilium/pkg/policy/selectorcache.go:540 +0x3a
2024-10-03T08:45:07.562547444Z created by github.com/cilium/cilium/pkg/policy.(*SelectorCache).UpdateIdentities in goroutine 357
2024-10-03T08:45:07.562550470Z 	/go/src/github.com/cilium/cilium/pkg/policy/selectorcache.go:539 +0xd54

Fixes: #34205

@jrajahalme jrajahalme added kind/bug This is a bug in the Cilium logic. release-note/misc This PR makes changes that have no direct user impact. release-blocker/1.17 This issue will prevent the release of the next version of Cilium. labels Oct 3, 2024
@jrajahalme jrajahalme requested a review from a team as a code owner October 3, 2024 11:03
@jrajahalme jrajahalme requested a review from doniacld October 3, 2024 11:03
@jrajahalme
Copy link
Member Author

/test

@github-actions github-actions bot added the sig/policy Impacts whether traffic is allowed or denied based on user-defined policies. label Oct 3, 2024
@jrajahalme jrajahalme enabled auto-merge October 3, 2024 12:28
@jrajahalme
Copy link
Member Author

ci-runtime hit by #35055 and #35196, rebasing after the latter merges.

Call queueNotifiedUsersCommit() before starting go routing waiting on
`wg`, as queueNotifiedUsersCommit() does `wg.Add(1)`. This makes it
impossible to hit this bug:

```
2024-10-03T08:45:07.562242086Z panic: sync: WaitGroup is reused before previous Wait has returned
2024-10-03T08:45:07.562251423Z
2024-10-03T08:45:07.562358242Z goroutine 11033 [running]:
2024-10-03T08:45:07.562532947Z sync.(*WaitGroup).Wait(0xc0041bfe90?)
2024-10-03T08:45:07.562538587Z 	/usr/local/go/src/sync/waitgroup.go:120 +0x74
2024-10-03T08:45:07.562541644Z github.com/cilium/cilium/pkg/policy.(*SelectorCache).UpdateIdentities.func1(0xc01829a4c0)
2024-10-03T08:45:07.562544629Z 	/go/src/github.com/cilium/cilium/pkg/policy/selectorcache.go:540 +0x3a
2024-10-03T08:45:07.562547444Z created by github.com/cilium/cilium/pkg/policy.(*SelectorCache).UpdateIdentities in goroutine 357
2024-10-03T08:45:07.562550470Z 	/go/src/github.com/cilium/cilium/pkg/policy/selectorcache.go:539 +0xd54
```

Fixes: cilium#34205

Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
@jrajahalme jrajahalme force-pushed the policy-fix-waitgroup-use branch from 5976df1 to f966abf Compare October 4, 2024 11:32
@jrajahalme
Copy link
Member Author

Rebased to get integration test fix.

@jrajahalme
Copy link
Member Author

/test

@jrajahalme jrajahalme added this pull request to the merge queue Oct 4, 2024
Merged via the queue into cilium:main with commit 3f7360a Oct 4, 2024
63 checks passed
@jrajahalme jrajahalme deleted the policy-fix-waitgroup-use branch October 4, 2024 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug This is a bug in the Cilium logic. release-blocker/1.17 This issue will prevent the release of the next version of Cilium. release-note/misc This PR makes changes that have no direct user impact. sig/policy Impacts whether traffic is allowed or denied based on user-defined policies.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants