Skip to content

Call FlushAsync before Dispose in DisposeAsync polyfills for Stream and TextWriter#84

Merged
Tyrrrz merged 3 commits intomasterfrom
copilot/update-disposeasync-implementation
Feb 27, 2026
Merged

Call FlushAsync before Dispose in DisposeAsync polyfills for Stream and TextWriter#84
Tyrrrz merged 3 commits intomasterfrom
copilot/update-disposeasync-implementation

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Feb 27, 2026

  • Update PolyShim/NetCore30/Stream.cs – call FlushAsync before Dispose in the DisposeAsync fallback path (with try/finally to guarantee disposal)
  • Update PolyShim/NetCore30/TextWriter.cs – call FlushAsync before Dispose in the DisposeAsync fallback path (with try/finally to guarantee disposal)
  • Simplify PolyShim.Tests/NetCore30/StreamTests.cs – single focused DisposeAsync_Test using a BufferedStream with WriteAsync, verifying data is flushed before disposal
  • Remove PolyShim.Tests/NetCore30/TextWriterTests.cs per review feedback
Original prompt

This section details on the original issue you should resolve

<issue_title>DisposeAsync should call FlushAsync on types with asynchronous IO before Dispose</issue_title>
<issue_description>### Details

Currently, PolyShim implements DisposeAsync for several types by simply wrapping Dispose. However, for types where Dispose may synchronously finalize IO operations (such as streams or writers), DisposeAsync should call FlushAsync or an equivalent asynchronous method before Dispose. This ensures that any remaining data is flushed asynchronously, avoiding blocking synchronous operations in Dispose and providing more consistent async IO semantics.

Proposal:

  • Update DisposeAsync implementations to invoke FlushAsync or similar methods where available, before calling Dispose.
  • This change should apply to any types in PolyShim where FlushAsync is relevant (streams, writers, etc.).
  • If a type does not support FlushAsync or an equivalent, the logic can remain unchanged.

This will enhance the correctness and performance of DisposeAsync in scenarios that involve asynchronous IO, aligning with expected .NET async patterns.

Checklist

  • I have looked through existing issues to make sure that this feature has not been requested before
  • I have provided a descriptive title for this issue
  • I am aware that even valid feature requests may be rejected if they do not align with the project's goals
  • I have sponsored this project</issue_description>

<agent_instructions>Also look for any other DisposeAsync() polyfills that may benefit from similar treatment</agent_instructions>

Comments on the Issue (you are @copilot in this section)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

…d TextWriter

Co-authored-by: Tyrrrz <1935960+Tyrrrz@users.noreply.github.com>
Copilot AI changed the title [WIP] Update DisposeAsync to call FlushAsync before Dispose Call FlushAsync before Dispose in DisposeAsync polyfills for Stream and TextWriter Feb 27, 2026
@Tyrrrz Tyrrrz added the enhancement New feature or request label Feb 27, 2026
@Tyrrrz Tyrrrz marked this pull request as ready for review February 27, 2026 09:55
Copilot AI review requested due to automatic review settings February 27, 2026 09:55
Copy link
Copy Markdown
Owner

@Tyrrrz Tyrrrz left a comment

Choose a reason for hiding this comment

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

… review feedback

Co-authored-by: Tyrrrz <1935960+Tyrrrz@users.noreply.github.com>
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

This PR updates the NetCore30 DisposeAsync polyfills for Stream and TextWriter to perform an async flush before synchronous disposal in the fallback path (when IAsyncDisposable isn’t available), and adds tests intended to validate the behavior.

Changes:

  • Update Stream.DisposeAsync polyfill to await FlushAsync() and then Dispose() in finally.
  • Update TextWriter.DisposeAsync polyfill similarly.
  • Add new unit tests for DisposeAsync on Stream and TextWriter.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
PolyShim/NetCore30/Stream.cs Replace Task.Run(Dispose) fallback with FlushAsync + Dispose in finally.
PolyShim/NetCore30/TextWriter.cs Replace Task.Run(Dispose) fallback with FlushAsync + Dispose in finally.
PolyShim.Tests/NetCore30/StreamTests.cs Adds tests for DisposeAsync and “flushes data” scenario.
PolyShim.Tests/NetCore30/TextWriterTests.cs Adds tests for DisposeAsync and “flushes data” scenario.

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

@Tyrrrz Tyrrrz changed the title Call FlushAsync before Dispose in DisposeAsync polyfills for Stream and TextWriter Call FlushAsync before Dispose in DisposeAsync polyfills for Stream and TextWriter Feb 27, 2026
@Tyrrrz Tyrrrz merged commit 834b221 into master Feb 27, 2026
5 checks passed
@Tyrrrz Tyrrrz deleted the copilot/update-disposeasync-implementation branch February 27, 2026 10:15
@codecov
Copy link
Copy Markdown

codecov bot commented Feb 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0.00%. Comparing base (0c7f42a) to head (d08724c).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@     Coverage Diff      @@
##   master   #84   +/-   ##
============================
============================

☔ View full report in Codecov by Sentry.
📢 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.

Copilot AI review requested due to automatic review settings March 23, 2026 23:37
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.

DisposeAsync should call FlushAsync on types with asynchronous IO before Dispose

3 participants