Address MARS TDS header contained errors #490
Conversation
- Added support for enclave simulator bypassing the actual attestation. - Added BuildSimulator property to enable the simulator support. How to use the enclave simulator: Given Attestation Protocol = SIM and Enclave Attestation Url= SomeDummyURL in the connection string and run the AE enclave-enabled tests. NOTE: This change is for internal testing only. The simulator support should be disabled in the official nuget package releases.
src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs
Show resolved
Hide resolved
|
This is a candidate to back port to 1.1 but I'd like to run the EF tests against it and get some people testing it to be more confident of the fix before putting it into a GA hotfix. |
|
I can test the 2.0 version against EF Core - where can I get a daily build? |
|
Hi @ErikEJ Since the PR hasn't been merged yet, nightly builds don't contain this change. You may use the Nuget package I attached under to validate changes: Microsoft.Data.SqlClient.2.0.0-dev-pr490.nupkg.zip <PackageReference Include="Microsoft.Data.SqlClient" Version="2.0.0-dev-pr490" /> |
|
Maybe you should consider automatically running the 18.000 EF Core SQL Server functional tests against any PR? |
|
I tested against latest EF Core master, and all tests passed (single run) |
Hi @ErikEJ The issue was reproducible with their "release/3.0" tag only and not in master as I mentioned here:
Yes that is in our backlog of things. Will be done soon! |
|
@cheenamalhotra I was unable to build that tag. |
|
@ErikEJ I too faced issues with running tests with NuGet package, tested with direct project reference and everything looks good! |
Trying to address #85
Here are my findings:
It seems that TryProcessDone in fact was running twice in certain cases. Once in the context of the first ReadAsync call, and again in the context of a continuation task when Cancel was called on the SqlCommand. I'm guessing Cancel happened really fast in these cases because both threads appeared to be executing nearly in parallel, hitting
stateObj.DecrementOpenResultCount();close enough together that the count incorrectly went to -1 (which gets passed in the MARS header and is invalid).This underlying problem was exposed by dotnet/corefx#26595.
I think this is a valid fix without reverting the above PR. It fixes the repro case and makes sense from what I've been able to digest from working with the async code (for what that's worth).
Note: The netfx code has the same issue, but it is hidden by the fact that
LocalAppContextSwitches.MakeReadAsyncBlockingis true by default which setsstateObj._syncOverAsync = true;. I did not change the behavior of netfx as part of this change but I did make the same fix to the netfx code as is applied to the netcore code.