Skip to content

fix(doc): resolve file descriptor leaks in documentation generators#992

Open
CygnusMaximillian wants to merge 2 commits into
goharbor:mainfrom
CygnusMaximillian:bug/memory-leak
Open

fix(doc): resolve file descriptor leaks in documentation generators#992
CygnusMaximillian wants to merge 2 commits into
goharbor:mainfrom
CygnusMaximillian:bug/memory-leak

Conversation

@CygnusMaximillian

Copy link
Copy Markdown

Description

This PR fixes a subtle file descriptor leak vulnerability within the CLI documentation generator logic (doc/doc.go and doc/man-docs/man_doc.go).

Previously, defer f.Close() was placed after write operations (like io.WriteString), or handled manually in a way that missed early err != nil return paths. If an I/O operation failed mid-way, the function exited prematurely, leaving the file descriptor open and causing potential resource exhaustion.

Changes Made

  • doc/doc.go: Moved defer f.Close() to execute immediately after checking os.Create errors, guaranteeing closure on subsequent write failures.
  • doc/man-docs/man_doc.go: Replaced manual/conditional closing logic with immediate defer calls for robust resource management.

Verification Results

I have verified these changes locally:

  • go build ./... completes successfully.
  • go test ./... passes without regressions.

Closes

Closes #991

Screenshots of Tests

image

also I ran general go test ./... for man_doc.go as there are no dedicated test for it

Signed-off-by: CygnusMaximillian <dprajjwal11@gmail.com>
@codecov

codecov Bot commented Jun 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 9.34%. Comparing base (60ad0bd) to head (925360c).
⚠️ Report is 186 commits behind head on main.

Files with missing lines Patch % Lines
doc/man-docs/man_doc.go 52.63% 4 Missing and 5 partials ⚠️
doc/doc.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##             main    #992      +/-   ##
=========================================
- Coverage   10.99%   9.34%   -1.65%     
=========================================
  Files         173     321     +148     
  Lines        8671   16089    +7418     
=========================================
+ Hits          953    1504     +551     
- Misses       7612   14442    +6830     
- Partials      106     143      +37     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@bupd bupd requested a review from Copilot June 13, 2026 16:38
@bupd bupd self-assigned this Jun 13, 2026
@bupd bupd added the enhancement New feature or request label Jun 13, 2026

@bupd bupd 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.

/lgtm

@CygnusMaximillian Thanks for your contribution

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 aims to eliminate file descriptor leaks in the CLI documentation generators by ensuring files created/opened during doc generation are reliably closed on all error paths.

Changes:

  • Move defer f.Close() to immediately after os.Create in MarkdownTreeCustom to prevent leaks on write failures.
  • Replace manual Close() calls in man page cleanup with defer to ensure closure on early returns.

Reviewed changes

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

File Description
doc/doc.go Registers defer f.Close() immediately after os.Create so write/generation failures don’t leak the output file descriptor.
doc/man-docs/man_doc.go Switches to deferred closes during man page read/write cleanup to cover early error returns.

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

Comment thread doc/man-docs/man_doc.go Outdated
Comment on lines 82 to 87
@@ -83,8 +83,8 @@ func cleanManPages(docDir string) error {
if err != nil {
return err
}
defer f.Close()
content, err := io.ReadAll(f)

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.

@CygnusMaximillian seems to be valid.

Signed-off-by: Prasanth Baskar <bupdprasanth@gmail.com>
@bupd bupd force-pushed the bug/memory-leak branch from 2cefb48 to 925360c Compare June 14, 2026 01:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: File descriptor leaks in documentation generation logic

3 participants