Skip to content

chore: add cacheCloser to cleanup all opened files #18473

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 20, 2025
Merged

chore: add cacheCloser to cleanup all opened files #18473

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3721247
chore: add cacheCloser to cleanup all opened files
Emyrk Jun 20, 2025
be1247a
remove some references
Emyrk Jun 20, 2025
0315359
set cache to nil to signal closed
Emyrk Jun 20, 2025
616f4e3
nil out the closers too
Emyrk Jun 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 18 additions & 16 deletions coderd/dynamicparameters/render.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
4 changes: 4 additions & 0 deletions coderd/files/cache.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
57 changes: 57 additions & 0 deletions coderd/files/closer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
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

closers []func()
mu sync.Mutex
}

func NewCacheCloser(cache FileAcquirer) *CacheCloser {
return &CacheCloser{
cache: cache,
closers: make([]func(), 0),
}
}

func (c *CacheCloser) Close() {
c.mu.Lock()
defer c.mu.Unlock()

for _, doClose := range c.closers {
doClose()
}

// Prevent further acquisitions
c.cache = nil
// Remove any references
c.closers = nil
}

func (c *CacheCloser) Acquire(ctx context.Context, fileID uuid.UUID) (*CloseFS, error) {
c.mu.Lock()
defer c.mu.Unlock()

if c.cache == nil {
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.closers = append(c.closers, f.close)

return f, nil
}
Loading