Skip to content

Fix data race in cell.Config due to shared FlagSet#4

Merged
joamaki merged 1 commit intomainfrom
pr/joamaki/cell-config-parallel
Apr 25, 2024
Merged

Fix data race in cell.Config due to shared FlagSet#4
joamaki merged 1 commit intomainfrom
pr/joamaki/cell-config-parallel

Conversation

@joamaki
Copy link
Copy Markdown
Contributor

@joamaki joamaki commented Apr 24, 2024

This fixes and adds a regression test for a data race due to using a shared FlagSet in cell.Config that was mutated due to e.g. (*FlagSet).AddFlag modifying the passed in flag. This race occured in cases where a single Config cell was used in multiple hives in parallel (e.g. by tests).

Previous write at 0x00c000111ea0 by goroutine 10:
  github.com/spf13/pflag.(*FlagSet).AddFlag()
      /home/jussi/go/pkg/mod/github.com/spf13/pflag@v1.0.5/flag.go:854 +0x153

The issue is fixed by creating the FlagSet during Apply() so each invocation gets its own unshared set.

Reported-by: David Bimmler david.bimmler@isovalent.com

This fixes and adds a regression test for a data race due to using a shared FlagSet
in cell.Config that was mutated due to e.g. (*FlagSet).AddFlag modifying the passed
in flag. This race occured in cases where a single Config cell was used in multiple
hives in parallel (e.g. by tests).

Previous write at 0x00c000111ea0 by goroutine 10:
  github.com/spf13/pflag.(*FlagSet).AddFlag()
      /home/jussi/go/pkg/mod/github.com/spf13/pflag@v1.0.5/flag.go:854 +0x153

The issue is fixed by creating the FlagSet during Apply() so each invocation
gets its own unshared set.

Reported-by: David Bimmler <david.bimmler@isovalent.com>
Signed-off-by: Jussi Maki <jussi@isovalent.com>
@joamaki joamaki requested a review from bimmlerd April 24, 2024 11:23
@joamaki joamaki merged commit f6d2f20 into main Apr 25, 2024
@joamaki joamaki deleted the pr/joamaki/cell-config-parallel branch April 25, 2024 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants