Skip to content

Fix partsReader.WriteTo to close consumed parts on success#61

Merged
sendya merged 2 commits into
fix/issue-57-double-close-part-readerfrom
copilot/fix-code-for-review-comment
Jun 1, 2026
Merged

Fix partsReader.WriteTo to close consumed parts on success#61
sendya merged 2 commits into
fix/issue-57-double-close-part-readerfrom
copilot/fix-code-for-review-comment

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jun 1, 2026

partsReader.WriteTo advanced r.index without closing readers on the successful copy path, which could leave part readers unclosed and let a later Close() skip them. This update aligns WriteTo behavior with the type contract by closing each part as it is fully consumed.

  • Behavioral fix

    • In pkg/iobuf/part_reader.go, WriteTo now closes each part reader after io.Copy, regardless of whether the copy ended with nil or io.EOF.
    • r.index is advanced per consumed part before handling error branches, so reader lifecycle and index progression stay consistent.
  • Targeted test coverage

    • In pkg/iobuf/part_reader_test.go, TestPartsReader_WriteTo_AllSuccess now asserts both readers are closed exactly once after WriteTo.
    • The same test also verifies that a subsequent outer Close() does not double-close already consumed readers.
nn, err = io.Copy(w, reader)
n += nn

r.index++
if closeErr := reader.Close(); closeErr != nil {
	return n, closeErr
}

if err != nil {
	if err != io.EOF {
		return n, err
	}
	return
}

Copilot AI changed the title [WIP] Fix the code for review comment in PR #60 Fix partsReader.WriteTo to close consumed parts on success Jun 1, 2026
Copilot finished work on behalf of sendya June 1, 2026 13:46
Copilot AI requested a review from sendya June 1, 2026 13:46
@sendya sendya marked this pull request as ready for review June 1, 2026 14:55
Copilot AI review requested due to automatic review settings June 1, 2026 14:55
@sendya sendya merged commit d9f9a2e into fix/issue-57-double-close-part-reader Jun 1, 2026
Copy link
Copy Markdown
Contributor

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

Fixes partsReader.WriteTo so that it consistently closes each part reader after it has been copied, preventing already-consumed parts from remaining unclosed (and later being skipped by Close() due to r.index having advanced).

Changes:

  • Update partsReader.WriteTo to advance r.index and Close() each consumed part on the success path (and before error branching), aligning lifecycle management with Read().
  • Extend TestPartsReader_WriteTo_AllSuccess to assert each underlying reader is closed exactly once and that a subsequent outer Close() does not double-close.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
pkg/iobuf/part_reader.go Ensures each part is closed after io.Copy completes, even on the successful copy path.
pkg/iobuf/part_reader_test.go Adds assertions verifying per-part closure on success and no double-close on outer Close().

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

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.

3 participants