diff --git a/vfs/vfscache/writeback/writeback.go b/vfs/vfscache/writeback/writeback.go index 54d664baa..5db10c603 100644 --- a/vfs/vfscache/writeback/writeback.go +++ b/vfs/vfscache/writeback/writeback.go @@ -276,13 +276,12 @@ func (wb *WriteBack) Add(id Handle, name string, modified bool, putFn PutFn) Han return wbItem.id } -// Remove should be called when a file should be removed from the +// _remove should be called when a file should be removed from the // writeback queue. This cancels a writeback if there is one and // doesn't return the item to the queue. -func (wb *WriteBack) Remove(id Handle) (found bool) { - wb.mu.Lock() - defer wb.mu.Unlock() - +// +// This should be called with the lock held +func (wb *WriteBack) _remove(id Handle) (found bool) { wbItem, found := wb.lookup[id] if found { fs.Debugf(wbItem.name, "vfs cache: cancelling writeback (uploading %v) %p item %d", wbItem.uploading, wbItem, wbItem.id) @@ -299,6 +298,16 @@ func (wb *WriteBack) Remove(id Handle) (found bool) { return found } +// Remove should be called when a file should be removed from the +// writeback queue. This cancels a writeback if there is one and +// doesn't return the item to the queue. +func (wb *WriteBack) Remove(id Handle) (found bool) { + wb.mu.Lock() + defer wb.mu.Unlock() + + return wb._remove(id) +} + // Rename should be called when a file might be uploading and it gains // a new name. This will cancel the upload and put it back in the // queue. @@ -314,6 +323,15 @@ func (wb *WriteBack) Rename(id Handle, name string) { // We are uploading already so cancel the upload wb._cancelUpload(wbItem) } + + // Check to see if there are any uploads with the existing + // name and remove them + for existingID, existingItem := range wb.lookup { + if existingID != id && existingItem.name == name { + wb._remove(existingID) + } + } + wbItem.name = name // Kick the timer on wb.items._update(wbItem, wb._newExpiry()) diff --git a/vfs/vfscache/writeback/writeback_test.go b/vfs/vfscache/writeback/writeback_test.go index 25a41d9e6..6b0398b50 100644 --- a/vfs/vfscache/writeback/writeback_test.go +++ b/vfs/vfscache/writeback/writeback_test.go @@ -585,6 +585,53 @@ func TestWriteBackRename(t *testing.T) { assert.Equal(t, wbItem.name, "three") } +// TestWriteBackRenameDuplicates checks that if we rename an entry and +// make a duplicate, we remove the duplicate. +func TestWriteBackRenameDuplicates(t *testing.T) { + wb, cancel := newTestWriteBack(t) + defer cancel() + + // add item "one" + pi1 := newPutItem(t) + id1 := wb.Add(0, "one", true, pi1.put) + wbItem1 := wb.lookup[id1] + checkOnHeap(t, wb, wbItem1) + checkInLookup(t, wb, wbItem1) + assert.Equal(t, wbItem1.name, "one") + + <-pi1.started + checkNotOnHeap(t, wb, wbItem1) + checkInLookup(t, wb, wbItem1) + + // add item "two" + pi2 := newPutItem(t) + id2 := wb.Add(0, "two", true, pi2.put) + wbItem2 := wb.lookup[id2] + checkOnHeap(t, wb, wbItem2) + checkInLookup(t, wb, wbItem2) + assert.Equal(t, wbItem2.name, "two") + + <-pi2.started + checkNotOnHeap(t, wb, wbItem2) + checkInLookup(t, wb, wbItem2) + + // rename "two" to "one" + wb.Rename(id2, "one") + + // check "one" is cancelled and removed from heap and lookup + checkNotOnHeap(t, wb, wbItem1) + checkNotInLookup(t, wb, wbItem1) + assert.True(t, pi1.cancelled) + assert.Equal(t, wbItem1.name, "one") + + // check "two" (now called "one"!) has been cancelled and will + // be retried + checkOnHeap(t, wb, wbItem2) + checkInLookup(t, wb, wbItem2) + assert.True(t, pi2.cancelled) + assert.Equal(t, wbItem2.name, "one") +} + func TestWriteBackCancelUpload(t *testing.T) { wb, cancel := newTestWriteBack(t) defer cancel()