From ac554bf663a3971681c7d35de7ccc503fbda837c Mon Sep 17 00:00:00 2001 From: Krishna Srinivas Date: Sat, 10 Dec 2016 05:40:18 +0530 Subject: [PATCH] =?UTF-8?q?FS/Multipart:=20Fix=20race=20between=20PutObjec?= =?UTF-8?q?tPart=20and=20Complete/Abort=20multi=E2=80=A6=20(#3419)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FS/Multipart: Fix race between PutObjectPart and Complete/Abort multipart. close(timeoutCh) on complete/abort so that a racing PutObjectPart does not leave a dangling go-routine. Fixes #3351 --- cmd/fs-v1-background-append.go | 7 ++++--- cmd/fs-v1-multipart.go | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/cmd/fs-v1-background-append.go b/cmd/fs-v1-background-append.go index 83634d680..1db467010 100644 --- a/cmd/fs-v1-background-append.go +++ b/cmd/fs-v1-background-append.go @@ -91,9 +91,9 @@ func (b *backgroundAppend) append(disk StorageAPI, bucket, object, uploadID stri // Called on complete-multipart-upload. Returns nil if the required parts have been appended. func (b *backgroundAppend) complete(disk StorageAPI, bucket, object, uploadID string, meta fsMetaV1) error { b.Lock() + defer b.Unlock() info, ok := b.infoMap[uploadID] delete(b.infoMap, uploadID) - b.Unlock() if !ok { return errPartsMissing } @@ -115,13 +115,12 @@ func (b *backgroundAppend) complete(disk StorageAPI, bucket, object, uploadID st // Called after complete-multipart-upload or abort-multipart-upload so that the appendParts go-routine is not left dangling. func (b *backgroundAppend) abort(uploadID string) { b.Lock() + defer b.Unlock() info, ok := b.infoMap[uploadID] if !ok { - b.Unlock() return } delete(b.infoMap, uploadID) - b.Unlock() info.abortCh <- struct{}{} } @@ -164,9 +163,11 @@ func (b *backgroundAppend) appendParts(disk StorageAPI, bucket, object, uploadID case <-info.abortCh: // abort-multipart-upload closed abortCh to end the appendParts go-routine. disk.DeleteFile(minioMetaTmpBucket, uploadID) + close(info.timeoutCh) // So that any racing PutObjectPart does not leave a dangling go-routine. return case <-info.completeCh: // complete-multipart-upload closed completeCh to end the appendParts go-routine. + close(info.timeoutCh) // So that any racing PutObjectPart does not leave a dangling go-routine. return case <-time.After(appendPartsTimeout): // Timeout the goroutine to garbage collect its resources. This would happen if the client initiates diff --git a/cmd/fs-v1-multipart.go b/cmd/fs-v1-multipart.go index c4718a3ad..8ae6d554c 100644 --- a/cmd/fs-v1-multipart.go +++ b/cmd/fs-v1-multipart.go @@ -361,9 +361,9 @@ func (fs fsObjects) PutObjectPart(bucket, object, uploadID string, partID int, s return "", toObjectErr(err, minioMetaMultipartBucket, uploadIDPath) } + // Append the part in background. + errCh := fs.bgAppend.append(fs.storage, bucket, object, uploadID, fsMeta) go func() { - // Append the part in background. - errCh := fs.bgAppend.append(fs.storage, bucket, object, uploadID, fsMeta) // Also receive the error so that the appendParts go-routine does not block on send. // But the error received is ignored as fs.PutObjectPart() would have already // returned success to the client.