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

Fix NullReferenceException in GetBytesAsync #1906

Merged
merged 1 commit into from Jan 30, 2023

Conversation

Wraith2
Copy link
Contributor

@Wraith2 Wraith2 commented Jan 25, 2023

fixes #1903

When calling GetBytesAsyns with no timeout no cancellation token source is created. The source value is captured by the async context and is being unconditionally disposed in DisposeCore(). This fix adds a null check before disposal.

Covering test is added.

/cc @NickKerensky

@codecov
Copy link

codecov bot commented Jan 26, 2023

Codecov Report

Base: 70.79% // Head: 69.62% // Decreases project coverage by -1.17% ⚠️

Coverage data is based on head (bb81ab4) compared to base (0156df2).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1906      +/-   ##
==========================================
- Coverage   70.79%   69.62%   -1.17%     
==========================================
  Files         292      292              
  Lines       61732    61732              
==========================================
- Hits        43701    42983     -718     
- Misses      18031    18749     +718     
Flag Coverage Δ
addons 92.38% <ø> (ø)
netcore 73.62% <100.00%> (-0.64%) ⬇️
netfx 67.97% <ø> (-1.20%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...core/src/Microsoft/Data/SqlClient/SqlDataReader.cs 72.88% <100.00%> (-0.46%) ⬇️
...c/Microsoft/Data/SqlClient/TdsParserStateObject.cs 26.18% <0.00%> (-40.63%) ⬇️
...c/Microsoft/Data/SqlClient/TdsParserStateObject.cs 23.30% <0.00%> (-21.08%) ⬇️
...icrosoft/Data/ProviderBase/DbConnectionInternal.cs 78.22% <0.00%> (-2.42%) ⬇️
...e/src/Microsoft/Data/SqlClient/SNI/SNITcpHandle.cs 62.62% <0.00%> (-2.32%) ⬇️
...soft/Data/SqlClient/TdsParserStateObjectManaged.cs 84.02% <0.00%> (-1.39%) ⬇️
...core/src/Microsoft/Data/SqlClient/SqlConnection.cs 77.31% <0.00%> (-1.21%) ⬇️
...SqlClient/src/Microsoft/Data/SqlClient/TdsEnums.cs 95.91% <0.00%> (-1.03%) ⬇️
.../src/Microsoft/Data/SqlClient/SNI/SNIMarsHandle.cs 92.37% <0.00%> (-0.90%) ⬇️
...c/Microsoft/Data/SqlClient/SqlConnectionFactory.cs 68.85% <0.00%> (-0.82%) ⬇️
... and 7 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@JRahnama
Copy link
Member

@Wraith2 thanks. Failure is not regarding the changes made in this PR. The intermittent failure that happens is regarding named pipes. Any idea on PR #1880? There are couple of things in that draft, but mainly what is your idea on passing a flag to distinguish between sync and async calls?

Copy link
Contributor

@lcheunglci lcheunglci left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @Wraith2 for the prompt fix. I tried to re-trigger the pipeline to run the tests a few times yesterday as I'm fairly sure the failures are not due to your changes.

@DavoudEshtehari DavoudEshtehari added netcore 🐛 Bug! Something isn't right ! labels Jan 30, 2023
@DavoudEshtehari DavoudEshtehari modified the milestones: 5.1.0, 5.2.0 Jan 30, 2023
@DavoudEshtehari DavoudEshtehari merged commit 518869e into dotnet:main Jan 30, 2023
@Wraith2 Wraith2 deleted the fix-1903 branch January 30, 2023 21:25
lcheunglci pushed a commit to lcheunglci/SqlClient that referenced this pull request Mar 23, 2023
@lcheunglci lcheunglci mentioned this pull request Mar 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 Bug! Something isn't right ! netcore
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NullReferenceException when streaming XML asynchronously from DB
4 participants