Skip to content

Commit

Permalink
s3blob: fix data race
Browse files Browse the repository at this point in the history
w.pr cannot be used to indicate that the pipe has been closed. Instead, we use an atomic.Bool.
The problem with using w.pr is that a write to it in the goroutine launched by writer.open
can race with a read on w.pr in Close.

Note that it's still possible that w.pr.CloseWithError runs concurrently with w.pr.Close. But this is fine.
  • Loading branch information
arjunnair1997 committed Aug 29, 2024
1 parent bb796f8 commit 02ad5d2
Showing 1 changed file with 6 additions and 4 deletions.
10 changes: 6 additions & 4 deletions blob/s3blob/s3blob.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ import (
"sort"
"strconv"
"strings"
"sync/atomic"

s3managerv2 "github.com/aws/aws-sdk-go-v2/feature/s3/manager"
s3v2 "github.com/aws/aws-sdk-go-v2/service/s3"
Expand Down Expand Up @@ -312,8 +313,9 @@ func (r *reader) Attributes() *driver.ReaderAttributes {
// writer writes an S3 object, it implements io.WriteCloser.
type writer struct {
// Ends of an io.Pipe, created when the first byte is written.
pw *io.PipeWriter
pr *io.PipeReader
pw *io.PipeWriter
pr *io.PipeReader
prClosed atomic.Bool

// Alternatively, upload is set to true when Upload was
// used to upload data.
Expand Down Expand Up @@ -380,7 +382,7 @@ func (w *writer) open(r io.Reader, closePipeOnError bool) {
if err != nil {
if closePipeOnError {
w.pr.CloseWithError(err)
w.pr = nil
w.prClosed.Store(true)
}
w.err = err
}
Expand All @@ -392,7 +394,7 @@ func (w *writer) open(r io.Reader, closePipeOnError bool) {
// will create an empty file at the given key.
func (w *writer) Close() error {
if !w.upload {
if w.pr != nil {
if !w.prClosed.Load() {
defer w.pr.Close()
}
if w.pw == nil {
Expand Down

0 comments on commit 02ad5d2

Please sign in to comment.