From 0ad0b7ca7cf6044de04a70d7415666be204712a9 Mon Sep 17 00:00:00 2001
From: Michael Eischer <michael.eischer@fau.de>
Date: Sun, 26 May 2024 12:37:24 +0200
Subject: [PATCH 1/3] bloblru: fix race condition that can compute value
 multiple times

---
 internal/bloblru/cache.go | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/internal/bloblru/cache.go b/internal/bloblru/cache.go
index 4477e37a9..1ff52094b 100644
--- a/internal/bloblru/cache.go
+++ b/internal/bloblru/cache.go
@@ -114,10 +114,19 @@ func (c *Cache) GetOrCompute(id restic.ID, compute func() ([]byte, error)) ([]by
 	if isDownloading {
 		// wait for result of parallel download
 		<-waitForResult
-		blob, ok := c.Get(id)
-		if ok {
-			return blob, nil
-		}
+	}
+
+	// try again. This is necessary independent of whether isDownloading is true or not.
+	// The calls to `c.Get()` and checking/adding the entry in `c.inProgress` are not atomic,
+	// thus the item might have been computed in the meantime.
+	// The following scenario would compute() the value multiple times otherwise:
+	// Goroutine A does not find a value in the initial call to `c.Get`, then goroutine B
+	// takes over, caches the computed value and cleans up its channel in c.inProgress.
+	// Then goroutine A continues, does not detect a parallel computation and would try
+	// to call compute() again.
+	blob, ok = c.Get(id)
+	if ok {
+		return blob, nil
 	}
 
 	// download it

From 21ce03cff27d4460450237a3a52975f97ccf7974 Mon Sep 17 00:00:00 2001
From: Michael Eischer <michael.eischer@fau.de>
Date: Sun, 26 May 2024 12:38:20 +0200
Subject: [PATCH 2/3] bloblru: move defer outside critical section

---
 internal/bloblru/cache.go | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/internal/bloblru/cache.go b/internal/bloblru/cache.go
index 1ff52094b..161f15375 100644
--- a/internal/bloblru/cache.go
+++ b/internal/bloblru/cache.go
@@ -100,7 +100,13 @@ func (c *Cache) GetOrCompute(id restic.ID, compute func() ([]byte, error)) ([]by
 	waitForResult, isDownloading := c.inProgress[id]
 	if !isDownloading {
 		c.inProgress[id] = finish
+	}
+	c.mu.Unlock()
 
+	if isDownloading {
+		// wait for result of parallel download
+		<-waitForResult
+	} else {
 		// remove progress channel once finished here
 		defer func() {
 			c.mu.Lock()
@@ -109,12 +115,6 @@ func (c *Cache) GetOrCompute(id restic.ID, compute func() ([]byte, error)) ([]by
 			close(finish)
 		}()
 	}
-	c.mu.Unlock()
-
-	if isDownloading {
-		// wait for result of parallel download
-		<-waitForResult
-	}
 
 	// try again. This is necessary independent of whether isDownloading is true or not.
 	// The calls to `c.Get()` and checking/adding the entry in `c.inProgress` are not atomic,

From 1c6067d93d61b33778d25f17da12481bdf7a5b84 Mon Sep 17 00:00:00 2001
From: Michael Eischer <michael.eischer@fau.de>
Date: Sun, 26 May 2024 12:38:41 +0200
Subject: [PATCH 3/3] bloblru: variable name cleanup

---
 internal/bloblru/cache.go | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/internal/bloblru/cache.go b/internal/bloblru/cache.go
index 161f15375..9981f8a87 100644
--- a/internal/bloblru/cache.go
+++ b/internal/bloblru/cache.go
@@ -97,13 +97,13 @@ func (c *Cache) GetOrCompute(id restic.ID, compute func() ([]byte, error)) ([]by
 	// check for parallel download or start our own
 	finish := make(chan struct{})
 	c.mu.Lock()
-	waitForResult, isDownloading := c.inProgress[id]
-	if !isDownloading {
+	waitForResult, isComputing := c.inProgress[id]
+	if !isComputing {
 		c.inProgress[id] = finish
 	}
 	c.mu.Unlock()
 
-	if isDownloading {
+	if isComputing {
 		// wait for result of parallel download
 		<-waitForResult
 	} else {
@@ -116,7 +116,7 @@ func (c *Cache) GetOrCompute(id restic.ID, compute func() ([]byte, error)) ([]by
 		}()
 	}
 
-	// try again. This is necessary independent of whether isDownloading is true or not.
+	// try again. This is necessary independent of whether isComputing is true or not.
 	// The calls to `c.Get()` and checking/adding the entry in `c.inProgress` are not atomic,
 	// thus the item might have been computed in the meantime.
 	// The following scenario would compute() the value multiple times otherwise: