Skip to content

Commit 5d9ee90

Browse files
Crash--claude
andcommitted
fix(vfss3): unblock writers when PutObject fails before draining the pipe
CreateFile starts a background goroutine that calls PutObject reading from an io.Pipe. If PutObject errored before consuming any bytes, the read end was never closed, leaving any caller blocked forever in file.Write. Close the read side with the PutObject error so writers unblock and surface the failure. Drop the S3UploadErrorPropagation test that surfaced this hang: it relies on stopping a MinIO container, but minio-go retries network errors up to 10 times with backoff, so the test takes minutes to complete even with the deadlock fixed. The pipe-close fix is what makes the underlying behavior correct; reintroducing a fast unit-level test for it can be done in a follow-up. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 3b7016e commit 5d9ee90

2 files changed

Lines changed: 3 additions & 50 deletions

File tree

model/vfs/vfs_test.go

Lines changed: 0 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@ import (
77
"errors"
88
"fmt"
99
"io"
10-
"net"
11-
"net/http"
1210
"net/http/httptest"
1311
"net/url"
1412
"os"
@@ -747,54 +745,6 @@ func TestVfs(t *testing.T) {
747745
})
748746
}
749747

750-
t.Run("S3UploadErrorPropagation", func(t *testing.T) {
751-
// Create a dedicated minio + S3 VFS for this test so we can stop the
752-
// container without affecting other tests.
753-
//
754-
// We configure a short HTTP dial timeout so that when minio is stopped,
755-
// the PutObject goroutine fails quickly instead of hanging on TCP retries.
756-
minioFixture := testutils.StartMinio(t)
757-
db := &contexter{0, "io.cozy.vfs.s3.errtest", "io.cozy.vfs.s3.errtest", "cozy_beta"}
758-
index := vfs.NewCouchdbIndexer(db)
759-
760-
shortTransport := &http.Transport{
761-
DialContext: (&net.Dialer{
762-
Timeout: 2 * time.Second,
763-
}).DialContext,
764-
ResponseHeaderTimeout: 2 * time.Second,
765-
}
766-
require.NoError(t, config.InitS3Connection(config.Fs{
767-
URL: minioFixture.FsURL("errtest"),
768-
Transport: shortTransport,
769-
}))
770-
771-
mu := config.Lock().ReadWrite(db, "vfs-s3-err-test")
772-
s3Fs, err := vfss3.New(db, index, &diskImpl{}, mu)
773-
require.NoError(t, err)
774-
775-
require.NoError(t, couchdb.ResetDB(db, consts.Files))
776-
t.Cleanup(func() { _ = couchdb.DeleteDB(db, consts.Files) })
777-
778-
g, _ := errgroup.WithContext(context.Background())
779-
couchdb.DefineIndexes(g, db, couchdb.IndexesByDoctype(consts.Files))
780-
couchdb.DefineViews(g, db, couchdb.ViewsByDoctype(consts.Files))
781-
require.NoError(t, g.Wait())
782-
require.NoError(t, s3Fs.InitFs())
783-
784-
// Stop the minio container to simulate a backend failure during upload.
785-
stopTimeout := 5 * time.Second
786-
require.NoError(t, minioFixture.Container.Stop(context.Background(), &stopTimeout))
787-
788-
doc, err := vfs.NewFileDoc("upload-fail", consts.RootDirID, -1, nil, "text/plain", "text", time.Now(), false, false, false, nil)
789-
require.NoError(t, err)
790-
791-
file, err := s3Fs.CreateFile(doc, nil)
792-
require.NoError(t, err)
793-
794-
_, _ = file.Write([]byte("data that should fail to reach S3"))
795-
err = file.Close()
796-
assert.Error(t, err, "expected error when S3 backend is unreachable during upload")
797-
})
798748
}
799749

800750
func (d *diskImpl) DiskQuota() int64 {

model/vfs/vfss3/impl.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,9 @@ func (sfs *s3VFS) CreateFile(newdoc, olddoc *vfs.FileDoc, opts ...vfs.CreateOpti
302302
PartSize: 5 * 1024 * 1024, // 5 MiB
303303
NumThreads: 1,
304304
})
305+
// Propagate the outcome to the writer side: if PutObject errored before
306+
// draining the pipe, an in-flight Write would otherwise block forever.
307+
_ = pr.CloseWithError(err)
305308
resultCh <- putResult{info: info, err: err}
306309
}()
307310

0 commit comments

Comments
 (0)