Skip to content

fix(store/posixage): always close file lock#488

Merged
joe0BAB merged 1 commit intomainfrom
fix/posixage
Mar 12, 2026
Merged

fix(store/posixage): always close file lock#488
joe0BAB merged 1 commit intomainfrom
fix/posixage

Conversation

@joe0BAB
Copy link
Collaborator

@joe0BAB joe0BAB commented Mar 12, 2026

This is a very subtle leak of not closing the file lock correctly in certain circumstances.

At the beginning of the function, we have:

	var (
		err error
		fl  *os.File
	)

	defer func() {
		// we must always close the file on any error returned
		if err != nil && fl != nil {
			_ = fl.Close()
		}
	}()

This code relies on re-using the same err variable. And we missed that / created a new one that would shadow it on one code path.

Signed-off-by: Johannes Großmann <grossmann.johannes@t-online.de>
@joe0BAB joe0BAB marked this pull request as ready for review March 12, 2026 09:42
@joe0BAB joe0BAB requested a review from Benehiko March 12, 2026 09:42
Copy link

@docker-agent docker-agent bot left a comment

Choose a reason for hiding this comment

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

Review Summary

Assessment: 🟢 APPROVE

The fix correctly addresses the variable shadowing issue described in the PR. By splitting the short variable declaration (:=) into a separate assignment (=) at line 92, the code now properly reuses the outer err variable. This allows the defer function to correctly close the file lock on error paths.

Analysis:

  • ✅ The variable shadowing bug is fixed correctly
  • ✅ The defer function will now trigger as intended when errors occur
  • ✅ No new bugs introduced by the changes

Note: During review, a potential file handle leak was identified at line 89 (where a second openFile call could replace the first file handle without closing it). However, this issue exists in the unchanged surrounding code, not in the lines modified by this PR, so it's outside the scope of this review.

@joe0BAB joe0BAB merged commit f580386 into main Mar 12, 2026
24 checks passed
@joe0BAB joe0BAB deleted the fix/posixage branch March 12, 2026 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants