Gracefully handle cached files being deleted (#1959)

The global cache in the `apk` package stores cached
`expandapk.APKExpanded` structs in memory, which references files on
disk and contains readers for those files. The in-memory data is
currently never invalidated, which can lead to problems if the files on
disk are deleted. This PR updates the cache to check if the backing file
still exists before returning data from the cache. If it's absent, the
APK data is re-populated.

Author: kevinmdavis

pkg/apk/apk/implementation.go

@@ -640,7 +640,6 @@ func (a *APK) CalculateWorld(ctx context.Context, allpkgs []*RepositoryPackage)
 	for i, pkg := range allpkgs {
 		g.Go(func() error {
 			expanded, err := a.expandPackage(ctx, pkg)
-
 			if err != nil {
 				return fmt.Errorf("expanding %s: %w", pkg.Name, err)
 			}
@@ -1262,31 +1261,33 @@ type apkResult struct {
 }
 
 type apkCache struct {
-	// url -> *sync.Once
+	// url -> func() apkResult (from sync.OnceValue(...))
 	onces sync.Map
-
-	// url -> apkResult
-	resps sync.Map
 }
 
 func (c *apkCache) get(ctx context.Context, a *APK, pkg InstallablePackage) (*expandapk.APKExpanded, error) {
 	u := pkg.URL()
-	// Do all the expensive things inside the once.
-	once, _ := c.onces.LoadOrStore(u, &sync.Once{})
-	once.(*sync.Once).Do(func() {
+	// Do all the expensive things inside sync.OnceValue()
+	fn := func() apkResult {
 		exp, err := expandPackage(ctx, a, pkg)
-		c.resps.Store(u, apkResult{
+		return apkResult{
 			exp: exp,
 			err: err,
-		})
-	})
-
-	v, ok := c.resps.Load(u)
-	if !ok {
-		panic(fmt.Errorf("did not see apk %q after writing it", u))
+		}
+	}
+	once, cached := c.onces.LoadOrStore(u, sync.OnceValue(fn))
+	result := once.(func() apkResult)()
+	if cached && result.exp != nil {
+		// If we find a value in the cache, we should check to make sure the tar file it references still exists.
+		// If it references a non-existent file, we should act as though this was a cache miss and expand the
+		// APK again.
+		if _, err := os.Stat(result.exp.TarFile); os.IsNotExist(err) {
+			newValue := sync.OnceValue(fn)
+			c.onces.Store(u, newValue)
+			result = newValue()
+		}
 	}
 
-	result := v.(apkResult)
 	return result.exp, result.err
 }
 

pkg/apk/apk/implementation_test.go

@@ -753,6 +753,29 @@ func TestFetchPackage(t *testing.T) {
 		require.NoError(t, err, "unable to read previous apk file")
 		require.Equal(t, apk1, apk2, "apk files do not match")
 	})
+	t.Run("handle missing cache files when expanding APK", func(t *testing.T) {
+		tmpDir := t.TempDir()
+		a := prepLayout(t, tmpDir)
+		// Fill the cache
+		exp, err := a.expandPackage(ctx, pkg)
+		require.NoError(t, err, "unable to expand package")
+		_, err = os.Stat(exp.TarFile)
+		require.NoError(t, err, "unable to stat cached tar file")
+		// Delete the tar file from the cache
+		require.NoError(t, os.Remove(exp.TarFile), "unable to delete cached tar file")
+		_, err = os.Stat(exp.TarFile)
+		require.ErrorIs(t, err, os.ErrNotExist, "unexpectedly able to stat cached tar file that should have been deleted")
+
+		// Expand the package again, this should re-populate the cache.
+		exp2, err := a.expandPackage(ctx, pkg)
+		require.NoError(t, err, "unable to expandPackage after deleting cached tar file")
+
+		// We should be able to read the APK contents
+		rc, err := exp2.APK()
+		require.NoError(t, err, "unable to get reader for APK()")
+		_, err = io.ReadAll(rc)
+		require.NoError(t, err, "unable to read APK contents")
+	})
 	t.Run("cache hit no etag", func(t *testing.T) {
 		tmpDir := t.TempDir()
 		a := prepLayout(t, tmpDir)