fix: double-checked locking fail (#392)

We use double-checked locking to avoid locks on the fast-path, but I modified the behaviour in #390 to cache the result before the lock, removing the second check. Face palm. This fixes #391
cashapp · Nov 15, 2023 · 880e21c · 880e21c
1 parent 44cb709
commit 880e21c
Showing 1 changed file with 6 additions and 9 deletions.
diff --git a/state/state.go b/state/state.go
@@ -284,13 +284,10 @@ func (s *State) removeRecursive(b *ui.Task, dest string) error {
 //
 // If the package has already been extracted, this is a no-op
 func (s *State) CacheAndUnpack(b *ui.Task, p *manifest.Package) error {
-	// Check if the package is up-to-date, and if so, return before acquiring the lock
-	isExtracted := s.isExtracted(p)
-	var areBinariesLinked bool
-	if isExtracted {
-		areBinariesLinked = s.areBinariesLinked(p)
-	}
-	if (isExtracted && areBinariesLinked) || p.Source == "/" {
+	// Double-checked locking. We check without the lock first, and then check
+	// again after acquiring the lock.
+
+	if (s.isExtracted(p) && s.areBinariesLinked(p)) || p.Source == "/" {
 		return nil
 	}
 
@@ -300,13 +297,13 @@ func (s *State) CacheAndUnpack(b *ui.Task, p *manifest.Package) error {
 	}
 	defer lock.Release(b)
 
-	if !isExtracted {
+	if !s.isExtracted(p) {
 		if err := s.extract(b, p); err != nil {
 			return errors.WithStack(err)
 		}
 	}
 
-	if !areBinariesLinked {
+	if !s.areBinariesLinked(p) {
 		if err := s.linkBinaries(p); err != nil {
 			return errors.WithStack(err)
 		}