chore: add cacheCloser to cleanup all opened files

Emyrk · Emyrk · commit 3721247990ed · 2025-06-20T13:08:48.000-05:00
diff --git a/coderd/dynamicparameters/render.go b/coderd/dynamicparameters/render.go
@@ -131,46 +131,48 @@ func (r *loader) Renderer(ctx context.Context, db database.Store, cache *files.C
 		return r.staticRender(ctx, db)
 	}
 
-	return r.dynamicRenderer(ctx, db, cache)
+	return r.dynamicRenderer(ctx, db, files.NewCacheCloser(cache))
 }
 
 // Renderer caches all the necessary files when rendering a template version's
 // parameters. It must be closed after use to release the cached files.
-func (r *loader) dynamicRenderer(ctx context.Context, db database.Store, cache *files.Cache) (*dynamicRenderer, error) {
+func (r *loader) dynamicRenderer(ctx context.Context, db database.Store, cache *files.CacheCloser) (*dynamicRenderer, error) {
+	closeFiles := true // If the function returns with no error, this will toggle to false.
+	defer func() {
+		if closeFiles {
+			cache.Close()
+		}
+	}()
+
 	// If they can read the template version, then they can read the file for
 	// parameter loading purposes.
 	//nolint:gocritic
 	fileCtx := dbauthz.AsFileReader(ctx)
-	templateFS, err := cache.Acquire(fileCtx, r.job.FileID)
+
+	var templateFS fs.FS
+	var err error
+
+	templateFS, err = cache.Acquire(fileCtx, r.job.FileID)
 	if err != nil {
 		return nil, xerrors.Errorf("acquire template file: %w", err)
 	}
 
-	var terraformFS fs.FS = templateFS
 	var moduleFilesFS *files.CloseFS
 	if r.terraformValues.CachedModuleFiles.Valid {
 		moduleFilesFS, err = cache.Acquire(fileCtx, r.terraformValues.CachedModuleFiles.UUID)
 		if err != nil {
-			templateFS.Close()
 			return nil, xerrors.Errorf("acquire module files: %w", err)
 		}
-		terraformFS = files.NewOverlayFS(templateFS, []files.Overlay{{Path: ".terraform/modules", FS: moduleFilesFS}})
+		templateFS = files.NewOverlayFS(templateFS, []files.Overlay{{Path: ".terraform/modules", FS: moduleFilesFS}})
 	}
 
+	closeFiles = false // Caller will have to call close
 	return &dynamicRenderer{
 		data:        r,
-		templateFS:  terraformFS,
+		templateFS:  templateFS,
 		db:          db,
 		ownerErrors: make(map[uuid.UUID]error),
-		close: func() {
-			// Up to 2 files are cached, and must be released when rendering is complete.
-			// TODO: Might be smart to always call release when the context is
-			//  canceled.
-			templateFS.Close()
-			if moduleFilesFS != nil {
-				moduleFilesFS.Close()
-			}
-		},
+		close:       cache.Close,
 	}, nil
 }
 
diff --git a/coderd/files/cache.go b/coderd/files/cache.go
@@ -19,6 +19,10 @@ import (
 	"github.com/coder/coder/v2/coderd/util/lazy"
 )
 
+type FileAcquirer interface {
+	Acquire(ctx context.Context, fileID uuid.UUID) (*CloseFS, error)
+}
+
 // NewFromStore returns a file cache that will fetch files from the provided
 // database.
 func NewFromStore(store database.Store, registerer prometheus.Registerer, authz rbac.Authorizer) *Cache {
diff --git a/coderd/files/closer.go b/coderd/files/closer.go
@@ -0,0 +1,54 @@
+package files
+
+import (
+	"context"
+	"sync"
+
+	"github.com/google/uuid"
+	"golang.org/x/xerrors"
+)
+
+// CacheCloser is a cache wrapper used to close all acquired files.
+// This is a more simple interface to use if opening multiple files at once.
+type CacheCloser struct {
+	cache FileAcquirer
+
+	close  []*CloseFS
+	mu     sync.Mutex
+	closed bool
+}
+
+func NewCacheCloser(cache FileAcquirer) *CacheCloser {
+	return &CacheCloser{
+		cache: cache,
+		close: make([]*CloseFS, 0),
+	}
+}
+
+func (c *CacheCloser) Close() {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+
+	for _, fs := range c.close {
+		fs.Close()
+	}
+	c.closed = true
+}
+
+func (c *CacheCloser) Acquire(ctx context.Context, fileID uuid.UUID) (*CloseFS, error) {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+
+	if c.closed {
+		return nil, xerrors.New("cache is closed, and cannot acquire new files")
+	}
+
+	f, err := c.cache.Acquire(ctx, fileID)
+	if err != nil {
+		return nil, err
+	}
+
+	c.close = append(c.close, f)
+
+	return f, nil
+}
