Fixing race condition in ux.TaskList when accessing the completed count - #7076
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes a data race in the ux.TaskList render/update loop by making reads of the completed counter atomic, matching the existing atomic writes.
Changes:
- Replace non-atomic read of
t.completedwithatomic.LoadInt32inTaskList.isCompleted().
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
…nt (Azure#7076) Just a little unprotected access to a variable. This should just happen if you run `go test -race` This is the test I used to repro it, but I'm unsure if you already have `-race` turned on in testing, etc... Let me know, I'll move this into the right spot. ```go package bug_test import ( "testing" "github.com/azure/azure-dev/cli/azd/pkg/ux" "github.com/stretchr/testify/require" ) func TestUseUXTaskList_RaceCondition(t *testing.T) { taskList := ux.NewTaskList(&ux.TaskListOptions{}) taskList.AddTask(ux.TaskOptions{ Title: "hello", Action: func(spf ux.SetProgressFunc) (ux.TaskState, error) { return ux.Success, nil }, }) taskList.AddTask(ux.TaskOptions{ Title: "hello2", Action: func(spf ux.SetProgressFunc) (ux.TaskState, error) { return ux.Success, nil }, }) err := taskList.Run() require.NoError(t, err) } ```
…nt (Azure#7076) Just a little unprotected access to a variable. This should just happen if you run `go test -race` This is the test I used to repro it, but I'm unsure if you already have `-race` turned on in testing, etc... Let me know, I'll move this into the right spot. ```go package bug_test import ( "testing" "github.com/azure/azure-dev/cli/azd/pkg/ux" "github.com/stretchr/testify/require" ) func TestUseUXTaskList_RaceCondition(t *testing.T) { taskList := ux.NewTaskList(&ux.TaskListOptions{}) taskList.AddTask(ux.TaskOptions{ Title: "hello", Action: func(spf ux.SetProgressFunc) (ux.TaskState, error) { return ux.Success, nil }, }) taskList.AddTask(ux.TaskOptions{ Title: "hello2", Action: func(spf ux.SetProgressFunc) (ux.TaskState, error) { return ux.Success, nil }, }) err := taskList.Run() require.NoError(t, err) } ```
…nt (Azure#7076) Just a little unprotected access to a variable. This should just happen if you run `go test -race` This is the test I used to repro it, but I'm unsure if you already have `-race` turned on in testing, etc... Let me know, I'll move this into the right spot. ```go package bug_test import ( "testing" "github.com/azure/azure-dev/cli/azd/pkg/ux" "github.com/stretchr/testify/require" ) func TestUseUXTaskList_RaceCondition(t *testing.T) { taskList := ux.NewTaskList(&ux.TaskListOptions{}) taskList.AddTask(ux.TaskOptions{ Title: "hello", Action: func(spf ux.SetProgressFunc) (ux.TaskState, error) { return ux.Success, nil }, }) taskList.AddTask(ux.TaskOptions{ Title: "hello2", Action: func(spf ux.SetProgressFunc) (ux.TaskState, error) { return ux.Success, nil }, }) err := taskList.Run() require.NoError(t, err) } ```
…nt (Azure#7076) Just a little unprotected access to a variable. This should just happen if you run `go test -race` This is the test I used to repro it, but I'm unsure if you already have `-race` turned on in testing, etc... Let me know, I'll move this into the right spot. ```go package bug_test import ( "testing" "github.com/azure/azure-dev/cli/azd/pkg/ux" "github.com/stretchr/testify/require" ) func TestUseUXTaskList_RaceCondition(t *testing.T) { taskList := ux.NewTaskList(&ux.TaskListOptions{}) taskList.AddTask(ux.TaskOptions{ Title: "hello", Action: func(spf ux.SetProgressFunc) (ux.TaskState, error) { return ux.Success, nil }, }) taskList.AddTask(ux.TaskOptions{ Title: "hello2", Action: func(spf ux.SetProgressFunc) (ux.TaskState, error) { return ux.Success, nil }, }) err := taskList.Run() require.NoError(t, err) } ```
…nt (Azure#7076) Just a little unprotected access to a variable. This should just happen if you run `go test -race` This is the test I used to repro it, but I'm unsure if you already have `-race` turned on in testing, etc... Let me know, I'll move this into the right spot. ```go package bug_test import ( "testing" "github.com/azure/azure-dev/cli/azd/pkg/ux" "github.com/stretchr/testify/require" ) func TestUseUXTaskList_RaceCondition(t *testing.T) { taskList := ux.NewTaskList(&ux.TaskListOptions{}) taskList.AddTask(ux.TaskOptions{ Title: "hello", Action: func(spf ux.SetProgressFunc) (ux.TaskState, error) { return ux.Success, nil }, }) taskList.AddTask(ux.TaskOptions{ Title: "hello2", Action: func(spf ux.SetProgressFunc) (ux.TaskState, error) { return ux.Success, nil }, }) err := taskList.Run() require.NoError(t, err) } ```
Just a little unprotected access to a variable. This should just happen if you run
go test -raceThis is the test I used to repro it, but I'm unsure if you already have
-raceturned on in testing, etc... Let me know, I'll move this into the right spot.