← Back to PRs

#22596: fix: use defer for file handle cleanup in TranslationMemory.Save()

by hobostay open 2026-02-21 10:53 View on GitHub →
scripts size: XS
Fix resource leak in `scripts/docs-i18n/tm.go` by using deferred file cleanup while ensuring the file is closed before `os.Rename()`. ## Problem The `TranslationMemory.Save()` method has resource leak and code duplication issues: 1. **Resource leak**: If an error occurs after opening the file but before the first manual `Close()`, the file handle leaks 2. **Code duplication**: The same `file.Close()` pattern is repeated 5 times throughout the function 3. **Windows compatibility**: The original PR #12318 used simple `defer file.Close()` which keeps the file open during `os.Rename()`, causing failures on Windows ## Solution - Use `defer` with a flag-based guard to ensure file is closed on all error paths - Remove 5 manual `file.Close()` calls that were duplicated across error paths - Explicitly close file before `os.Rename()` (required for Windows compatibility) ```go // Ensure file is closed on error paths, but we'll explicitly close before rename closed := false defer func() { if !closed { _ = file.Close() } }() // ... write operations ... // Close the file explicitly before rename (required on Windows) if err := file.Close(); err != nil { return err } closed = true return os.Rename(tmpPath, tm.path) ``` ## Benefits 1. **No resource leaks**: File is always closed, even on early error returns 2. **Windows compatible**: File is closed before rename (Windows can't rename open files) 3. **Cleaner code**: Eliminates repetitive manual cleanup code 4. **Follows Go idioms**: Uses defer for resource cleanup while handling platform-specific needs ## Technical Details The `closed` flag prevents the deferred cleanup from attempting to close an already-closed file, while ensuring that any early return will still clean up the file handle. ## Addresses Review Feedback This PR addresses the feedback on PR #12318 from Greptile: > "The only behavioral change is file lifetime, but `os.Rename` now runs before the file is closed, which is a known failure mode on Windows." The fix ensures the file is closed **before** `os.Rename()` while keeping `defer` as a safety net for error paths. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> <!-- greptile_comment --> <h3>Greptile Summary</h3> Fixed resource leak in `TranslationMemory.Save()` by using deferred cleanup with explicit close before rename for Windows compatibility. - Prevented file handle leaks on error paths by adding defer guard - Removed 5 redundant manual `file.Close()` calls - Maintained Windows compatibility by explicitly closing before `os.Rename()` <h3>Confidence Score: 5/5</h3> - Safe to merge - textbook Go resource management pattern - The change correctly implements deferred cleanup with a flag-based guard to handle platform-specific requirements. Logic is sound, addresses the stated resource leak, and follows Go idioms precisely. - No files require special attention <sub>Last reviewed commit: 2b41a93</sub> <!-- greptile_other_comments_section --> <sub>(5/5) You can turn off certain types of comments like style [here](https://app.greptile.com/review/github)!</sub> <!-- /greptile_comment -->

Most Similar PRs