Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add CA2024: Do not use 'StreamReader.EndOfStream' in async methods #7390

Merged
merged 8 commits into from
Nov 9, 2024

Conversation

mpidash
Copy link
Contributor

@mpidash mpidash commented Aug 25, 2024

Fixes dotnet/runtime#98834.

Rule ID CA2024
Title Do not use 'StreamReader.EndOfStream' in async methods
Message Do not use '{0}' in an async method
Description The property 'StreamReader.EndOfStream' can cause unintended synchronous blocking when no data is buffered. Instead, use 'StreamReader.ReadLineAsync' directly, which returns 'null' when reaching the end of the stream.
Category Reliability
Severity Warning

This analyzer detects when StreamReader.EndOfStream is used in an async method, which can prevent I/O from being done asynchronously.


I've tested against dotnet/runtime, dotnet/roslyn and dotnet/aspnetcore and found no violations.

I've also added a reference to EndOfStream in an async method in runtime for testing, which leads to the following message: warning CA2024: Do not use 'sr.EndOfStream' in an async method.

This analyzer detects when 'StreamReader.EndOfStream' is used in an
async method, which can prevent I/O from being done asynchronously.
@mpidash mpidash requested a review from a team as a code owner August 25, 2024 17:21
Copy link

codecov bot commented Aug 25, 2024

Codecov Report

Attention: Patch coverage is 98.34906% with 7 lines in your changes missing coverage. Please review.

Project coverage is 96.50%. Comparing base (d6e7d82) to head (e59b83a).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff            @@
##             main    #7390    +/-   ##
========================================
  Coverage   96.50%   96.50%            
========================================
  Files        1448     1450     +2     
  Lines      346895   347319   +424     
  Branches    11404    11409     +5     
========================================
+ Hits       334755   335186   +431     
+ Misses       9250     9242     -8     
- Partials     2890     2891     +1     

Copy link
Member

@stephentoub stephentoub left a comment

Choose a reason for hiding this comment

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

Other than some changes for the tests, this LGTM. Thanks for working on this.

Using the Stream type, or more specifically, the EndOfStream property of
the Stream type, should be much rarer than async methods in general, so
we check the symbols first.
@mpidash
Copy link
Contributor Author

mpidash commented Nov 9, 2024

I've addressed your feedback, thanks for the review!

@stephentoub stephentoub merged commit 5435ba7 into dotnet:main Nov 9, 2024
11 checks passed
@stephentoub
Copy link
Member

thanks!

@mpidash mpidash deleted the issue-98834 branch November 9, 2024 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Analyzer Proposal]: Warn on use of StreamReader.EndOfStream in async methods
2 participants