From f9682976a4d4fed5c20ceaba34a6ac2f2d30c1d7 Mon Sep 17 00:00:00 2001 From: Chris Cotter Date: Mon, 17 Oct 2022 14:08:27 -0400 Subject: [PATCH] test(storage): add test for Writer race (#6862) Adds a test to detect a potential race condition that can arise if the context is cancelled before writing to GCS. We already have a test that detects a similar condition after we have already started writing the object, but this test looks at the context being cancelled before any writes occur. When run with -race, this test fails in storage/v1.27.0, but it passes when run with the fix in #6817. Updates #6816 --- storage/writer_test.go | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/storage/writer_test.go b/storage/writer_test.go index 2537da9a633..e6e40e199d2 100644 --- a/storage/writer_test.go +++ b/storage/writer_test.go @@ -18,6 +18,7 @@ import ( "context" "crypto/sha256" "encoding/base64" + "errors" "net/http" "strings" "testing" @@ -119,7 +120,7 @@ func TestEncryption(t *testing.T) { // This test demonstrates the data race on Writer.err that can happen when the // Writer's context is cancelled. To see the race, comment out the w.mu.Lock/Unlock // lines in writer.go and run this test with -race. -func TestRaceOnCancel(t *testing.T) { +func TestRaceOnCancelAfterWrite(t *testing.T) { client := mockClient(t, &mockTransport{}) cctx, cancel := context.WithCancel(context.Background()) w := client.Bucket("b").Object("o").NewWriter(cctx) @@ -136,6 +137,24 @@ func TestRaceOnCancel(t *testing.T) { w.Write([]byte(nil)) } +// This test checks for the read-write race condition that can occur if +// monitorCancel is launched before the pipe is opened. See +// https://github.com/googleapis/google-cloud-go/issues/6816 +func TestRaceOnCancelBeforeWrite(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + client := mockClient(t, &mockTransport{}) + cancel() + writer := client.Bucket("foo").Object("bar").NewWriter(ctx) + writer.ChunkSize = googleapi.MinUploadChunkSize + writer.Write([]byte("data")) + + // We expect Close to return a context cancelled error, and the Writer should + // not open the pipe and avoid triggering a race condition. + if err := writer.Close(); !errors.Is(err, context.Canceled) { + t.Errorf("Writer.Close: got %v, want %v", err, context.Canceled) + } +} + func TestCancelDoesNotLeak(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) const contents = "hello world"