Skip to content

🤖🤖🤖 Close WriteSnapshotToDir snapshot file handles#38534

Open
mike-lmctl wants to merge 2 commits into
hashicorp:mainfrom
mike-lmctl:fix-modsdir-snapshot-file-close
Open

🤖🤖🤖 Close WriteSnapshotToDir snapshot file handles#38534
mike-lmctl wants to merge 2 commits into
hashicorp:mainfrom
mike-lmctl:fix-modsdir-snapshot-file-close

Conversation

@mike-lmctl
Copy link
Copy Markdown

@mike-lmctl mike-lmctl commented May 7, 2026

Fixes #38302.

This closes the modules.json snapshot file on all WriteSnapshotToDir return paths and returns close errors after a successful encode when appropriate.

The regression tests cover repeated writes and file-handle release behavior in internal/modsdir.

AI usage disclosure: AI assistance was used to inspect the affected code path and draft the initial patch/tests. I reviewed the change, kept the scope to internal/modsdir, and tested it locally.

Tested:

  • go test ./internal/modsdir in a Go 1.25.8 Docker container

Ensure `WriteSnapshotToDir` always closes the created snapshot file and
returns close errors when write succeeds but close fails.

Add focused regression coverage in `internal/modsdir` for repeated
snapshot writes and file-handle release behavior.

Tested: go test ./internal/modsdir in a Go 1.25.8 Docker container
Tested: git diff --check -- internal/modsdir/manifest.go internal/modsdir/manifest_test.go '.changes/v1.16/BUG FIXES-20260506-195131.yaml' pr-request-dd-026-terraform-38302.txt
@mike-lmctl mike-lmctl requested a review from a team as a code owner May 7, 2026 01:08
@hashicorp-cla-app
Copy link
Copy Markdown

CLA assistant check

Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement

Learn more about why HashiCorp requires a CLA and what the CLA includes

Have you signed the CLA already but the status is still pending? Recheck it.

@crw
Copy link
Copy Markdown
Contributor

crw commented May 7, 2026

Thanks for this submission. Please see our Contributing.md document regarding submission guidelines, particularly Proposing a Change and rules on disclosure of usage of GenAI. So that this can be considered for review, please also sign the CLA as per this comment: #38534 (comment). Thanks!

@mike-lmctl mike-lmctl changed the title Close WriteSnapshotToDir snapshot file handles 🤖🤖🤖 Close WriteSnapshotToDir snapshot file handles May 7, 2026
@mike-lmctl
Copy link
Copy Markdown
Author

@crw Thanks for pointing me to those guidelines. I updated the PR title/body to include the issue reference and the AI usage disclosure requested in CONTRIBUTING.md.

The remaining blocker is the CLA check; I’ll get that handled before expecting review.

@mike-lmctl
Copy link
Copy Markdown
Author

@crw about CLA, I signed it, though I could not find a page to confirm I have signed it. The bot still shows that I am not signed.

Empty commit to check whether the CLA bot re-runs after the CLA was signed.

Signed-off-by: Mike Ma <mike.ma@lmctl.com>
@crw
Copy link
Copy Markdown
Contributor

crw commented Jun 4, 2026

@crw about CLA, I signed it, though I could not find a page to confirm I have signed it. The bot still shows that I am not signed.

The CLA signing mechanism is based on matching the email of the account doing the signing to the email doing the commit. Sometimes those are not the same email. Do you know if you committed with the same email as you signed? If so I'll do some deeper digging on this side with regards to what email we have signed for you. Thanks!

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.

bug: file descriptor leak in modsdir.WriteSnapshotToDir

2 participants