From c59aaa536e009a6e84ca5ccf9c1869dbf0ec73f0 Mon Sep 17 00:00:00 2001 From: John Floren Date: Mon, 3 May 2021 16:14:19 -0700 Subject: [PATCH 1/2] Handle possible race where WriteStream can mess up the data being read from ReadStream. --- diskv.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/diskv.go b/diskv.go index 9f07b85..4ff6200 100644 --- a/diskv.go +++ b/diskv.go @@ -216,7 +216,16 @@ func (d *Diskv) createKeyFileWithLock(pathKey *PathKey) (*os.File, error) { return f, nil } - mode := os.O_WRONLY | os.O_CREATE | os.O_TRUNC // overwrite if exists + // We call os.Remove before the call to OpenFile because otherwise we can + // mess with existing readers. If someone calls ReadStream, then proceeds + // to read from it slowly, then someone else calls WriteStream, the reader + // will start to get the *new* resource contents part-way through. + // By calling os.Remove, we unlink the existing file, but since the process + // still has a handle on the file, the existing readers can continue to + // read from it. Meanwhile we're free to update what's actually on-disk. + os.Remove(d.completeFilename(pathKey)) + + mode := os.O_WRONLY | os.O_CREATE | os.O_TRUNC // overwrite if exists somehow f, err := os.OpenFile(d.completeFilename(pathKey), mode, d.FilePerm) if err != nil { return nil, fmt.Errorf("open file: %s", err) From 5a1fbbe382821c4f6d9dd2cea7127761e67df66e Mon Sep 17 00:00:00 2001 From: John Floren Date: Tue, 4 May 2021 08:41:20 -0700 Subject: [PATCH 2/2] Add test for issue #63 --- issues_test.go | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/issues_test.go b/issues_test.go index 5bf9e35..4c21de9 100644 --- a/issues_test.go +++ b/issues_test.go @@ -189,3 +189,55 @@ func TestIssue40(t *testing.T) { // is no room in the cache for this entry and it panics. d.Read(k2) } + +// Test issue #63, where a reader obtained from ReadStream will start +// to return invalid data if WriteStream is called before you finish +// reading. +func TestIssue63(t *testing.T) { + var ( + basePath = "test-data" + ) + // Simplest transform function: put all the data files into the base dir. + flatTransform := func(s string) []string { return []string{} } + + // Initialize a new diskv store, rooted at "my-data-dir", + // with no cache. + d := New(Options{ + BasePath: basePath, + Transform: flatTransform, + CacheSizeMax: 0, + }) + + defer d.EraseAll() + + // Write a big entry + k1 := "key1" + d1 := make([]byte, 1024*1024) + rand.Read(d1) + d.Write(k1, d1) + + // Open a reader. We set the direct flag to be sure we're going straight to disk. + s1, err := d.ReadStream(k1, true) + if err != nil { + t.Fatal(err) + } + + // Now generate a second big entry and put it in the *same* key + d2 := make([]byte, 1024*1024) + rand.Read(d2) + d.Write(k1, d2) + + // Now read from that stream we opened + out, err := ioutil.ReadAll(s1) + if err != nil { + t.Fatal(err) + } + if len(out) != len(d1) { + t.Fatalf("Invalid read: got %v bytes expected %v\n", len(out), len(d1)) + } + for i := range out { + if out[i] != d1[i] { + t.Fatalf("Output differs from expected at byte %v", i) + } + } +} \ No newline at end of file