From 6b25bc3003a374bc7539b0c5ec330adf12506972 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 31 Mar 2022 22:52:04 +0200 Subject: [PATCH] fix race condition in TestRemoveForce This test uses two subtests that were sharing the same variable. Subtests run in a goroutine, which could lead to them concurrently accessing the variable, resulting in a panic: === FAIL: cli/command/container TestRemoveForce/without_force (0.00s) Error: Error: No such container: nosuchcontainer --- FAIL: TestRemoveForce/without_force (0.00s) panic: runtime error: invalid memory address or nil pointer dereference [recovered] panic: runtime error: invalid memory address or nil pointer dereference [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x40393f] goroutine 190 [running]: testing.tRunner.func1.2({0xb76380, 0x124c9a0}) /usr/local/go/src/testing/testing.go:1389 +0x24e testing.tRunner.func1() /usr/local/go/src/testing/testing.go:1392 +0x39f panic({0xb76380, 0x124c9a0}) /usr/local/go/src/runtime/panic.go:838 +0x207 sort.StringSlice.Less(...) /usr/local/go/src/sort/sort.go:319 sort.insertionSort({0xd87380, 0xc00051b3b0}, 0x0, 0x2) /usr/local/go/src/sort/sort.go:40 +0xb1 sort.quickSort({0xd87380, 0xc00051b3b0}, 0x18?, 0xb4f060?, 0xc000540e01?) /usr/local/go/src/sort/sort.go:222 +0x171 sort.Sort({0xd87380, 0xc00051b3b0}) /usr/local/go/src/sort/sort.go:231 +0x53 sort.Strings(...) /usr/local/go/src/sort/sort.go:335 github.com/docker/cli/cli/command/container.TestRemoveForce.func2(0xc0005389c0?) /go/src/github.com/docker/cli/cli/command/container/rm_test.go:36 +0x125 testing.tRunner(0xc00053e4e0, 0xc00051b140) /usr/local/go/src/testing/testing.go:1439 +0x102 created by testing.(*T).Run /usr/local/go/src/testing/testing.go:1486 +0x35f === FAIL: cli/command/container TestRemoveForce (0.00s) This patch changes the test to use to separate variables. Signed-off-by: Sebastiaan van Stijn (cherry picked from commit 9688f62d20cc13e1a68fa28ec56cdf03ddf5864e) Signed-off-by: Sebastiaan van Stijn --- cli/command/container/rm_test.go | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/cli/command/container/rm_test.go b/cli/command/container/rm_test.go index 07c3f490f0..c47ea2da5d 100644 --- a/cli/command/container/rm_test.go +++ b/cli/command/container/rm_test.go @@ -14,13 +14,17 @@ import ( ) func TestRemoveForce(t *testing.T) { - var removed []string + var ( + removed1 []string + removed2 []string + ) cli := test.NewFakeCli(&fakeClient{ containerRemoveFunc: func(ctx context.Context, container string, options types.ContainerRemoveOptions) error { - removed = append(removed, container) + removed1 = append(removed1, container) + removed2 = append(removed2, container) if container == "nosuchcontainer" { - return errdefs.NotFound(fmt.Errorf("Error: No such container: " + container)) + return errdefs.NotFound(fmt.Errorf("Error: no such container: " + container)) } return nil }, @@ -31,16 +35,16 @@ func TestRemoveForce(t *testing.T) { t.Run("without force", func(t *testing.T) { cmd.SetArgs([]string{"nosuchcontainer", "mycontainer"}) - removed = []string{} - assert.ErrorContains(t, cmd.Execute(), "No such container") - sort.Strings(removed) - assert.DeepEqual(t, removed, []string{"mycontainer", "nosuchcontainer"}) + removed1 = []string{} + assert.ErrorContains(t, cmd.Execute(), "no such container") + sort.Strings(removed1) + assert.DeepEqual(t, removed1, []string{"mycontainer", "nosuchcontainer"}) }) t.Run("with force", func(t *testing.T) { cmd.SetArgs([]string{"--force", "nosuchcontainer", "mycontainer"}) - removed = []string{} + removed2 = []string{} assert.NilError(t, cmd.Execute()) - sort.Strings(removed) - assert.DeepEqual(t, removed, []string{"mycontainer", "nosuchcontainer"}) + sort.Strings(removed2) + assert.DeepEqual(t, removed2, []string{"mycontainer", "nosuchcontainer"}) }) }