Skip to content

fix: close lock file handles to prevent leaks#225

Merged
Yakwilik merged 2 commits into
easyp-tech:mainfrom
yosakoo:fix/lock-file-close-leak
May 18, 2026
Merged

fix: close lock file handles to prevent leaks#225
Yakwilik merged 2 commits into
easyp-tech:mainfrom
yosakoo:fix/lock-file-close-leak

Conversation

@yosakoo
Copy link
Copy Markdown
Contributor

@yosakoo yosakoo commented May 16, 2026

Fixed a few small bugs found while reading the code.

LockFile.New and LockFile.Write opened the lock file but never closed it,
leaking the handle every time those paths ran. LockFile.Write also silently
discarded fp.Write errors now they are returned to the caller.

RemotePluginExecutor.Execute closed the gRPC connection twice once via
defer conn.Close() and once explicitly at the end of the function. The
second close (the deferred one) silently returned an "already closed" error.
Removed the explicit close and moved the error log into the defer, so the
diagnostic is preserved without double-closing.

Note: TestCore_BreakingCheck/broken fails on Windows on main too due to
CRLF line endings shifting hardcoded byte offsets in test expectations.
Unrelated to this PR. Linux CI should be green.

hound672
hound672 previously approved these changes May 16, 2026
@yosakoo
Copy link
Copy Markdown
Contributor Author

yosakoo commented May 16, 2026

@hound672 I should also fix the test for Windows in this pr?

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes resource-handling issues in the adapters layer by ensuring file and gRPC connection handles are properly closed, and by returning previously ignored write errors to callers.

Changes:

  • Close the gRPC client connection exactly once in RemotePluginExecutor.Execute, preserving close diagnostics without double-closing.
  • Ensure LockFile.New closes the lock file after reading.
  • Ensure LockFile.Write closes the created file and returns Write/Close errors instead of discarding them.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
internal/adapters/plugin/remote.go Replace plain defer conn.Close() + explicit close with a single deferred close that logs close errors.
internal/adapters/lock_file/write.go Add deferred Close() (propagating close errors) and return fp.Write errors instead of ignoring them.
internal/adapters/lock_file/lock_file.go Add deferred Close() for the lock file reader to prevent descriptor leaks.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread internal/adapters/lock_file/lock_file.go
@yosakoo
Copy link
Copy Markdown
Contributor Author

yosakoo commented May 17, 2026

@hound672

@yosakoo
Copy link
Copy Markdown
Contributor Author

yosakoo commented May 18, 2026

@Yakwilik can u merge?

@Yakwilik Yakwilik merged commit bfd19dc into easyp-tech:main May 18, 2026
1 check passed
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.

4 participants