Skip to content

policy: close memoized policy filesystems#3943

Open
crazy-max wants to merge 1 commit into
docker:masterfrom
crazy-max:policy-handle-leak
Open

policy: close memoized policy filesystems#3943
crazy-max wants to merge 1 commit into
docker:masterfrom
crazy-max:policy-handle-leak

Conversation

@crazy-max

@crazy-max crazy-max commented Jul 7, 2026

Copy link
Copy Markdown
Member

This fixes policy filesystem cleanup so a policy filesystem opened during policy loading is fully closed after use.

Here is a repro with simple test:

func TestPolicyFSHandleLeakRepro(t *testing.T) {
	dir := t.TempDir()

	require.NoError(t, os.WriteFile(
		filepath.Join(dir, "policy.rego"),
		[]byte("package docker\n"),
		0600,
	))

	provider := newPolicyPathFS(context.Background(), nil, policyOpt{
		ContextDir: dir,
	})

	_, ok, err := loadPolicyData(provider, "policy.rego")
	require.NoError(t, err)
	require.True(t, ok)
}
go test -count=1 ./build -run TestPolicyFSHandleLeakRepro
...
TempDir RemoveAll cleanup: unlinkat ...: The process cannot access the file because it is being used by another process.

That happens because loadPolicyData does both Stat and ReadFile, and each call resolves the memoized policy filesystem. The old close path only decremented one reference, so the opened filesystem root stayed alive on Windows.

The policy path filesystem now closes all memoized filesystem references when the wrapper is closed, instead of decrementing only one reference.

Policy loading can resolve the same memoized filesystem more than once while reading a policy file. Closing only one reference can leave the underlying filesystem open, which is especially visible on Windows when temporary policy directories cannot be removed.

Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
@crazy-max crazy-max added this to the v0.36.0 milestone Jul 7, 2026
@crazy-max crazy-max requested a review from tonistiigi July 7, 2026 09:03
@crazy-max crazy-max marked this pull request as ready for review July 7, 2026 09:03

@tonistiigi tonistiigi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get this. The previous function does reference counting. The new one just closes directly, so should be unsafe for multiple users. I also don't see what is left using the previous function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants