Skip to content

Commit

Permalink
test(storage): add test for Writer race
Browse files Browse the repository at this point in the history
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 googleapis#6817.

Updates googleapis#6816
  • Loading branch information
tritone committed Oct 13, 2022
1 parent ae7441a commit 612a695
Showing 1 changed file with 27 additions and 1 deletion.
28 changes: 27 additions & 1 deletion storage/writer_test.go
Expand Up @@ -18,6 +18,7 @@ import (
"context"
"crypto/sha256"
"encoding/base64"
"errors"
"net/http"
"strings"
"testing"
Expand Down Expand Up @@ -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)
Expand All @@ -136,6 +137,31 @@ 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 should not return an error because data does not fill the
// buffer. Since the Writer is lazy about sending a request to GCS, nothing
// happens until the buffer fills or Writer.Close is called.
_, err := writer.Write([]byte("data"))
if err != nil {
t.Errorf("Writer.Write: %v", err)
}
// We expect Close to return a context cancelled error, and the Writer should
// not open the pipe and avoid triggering a race condition.
err = writer.Close()
if !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"
Expand Down

0 comments on commit 612a695

Please sign in to comment.