Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

SqlClient minor fix and changes#35344

Merged
AfsanehR-zz merged 5 commits intodotnet:masterfrom
Wraith2:sqlfix-tweaks1
Mar 15, 2019
Merged

SqlClient minor fix and changes#35344
AfsanehR-zz merged 5 commits intodotnet:masterfrom
Wraith2:sqlfix-tweaks1

Conversation

@Wraith2
Copy link
Contributor

@Wraith2 Wraith2 commented Feb 15, 2019

Fixes https://github.com/dotnet/corefx/issues/30650

3 tiny changes:

  • fix https://github.com/dotnet/corefx/issues/30650 by using Interlocked.CompareExchange so if two threads cause initialization of the synonyms dictionary the first writer wins and the write instruction is thread/cache aware.
  • change a byte[] allocation in TryProcessLoginAck to a stackalloc span, tiny memory saving.
  • change ProcessSSPI to use a second rented buffer

function and manual tests run in native mode.
/cc @afsanehr, @tarikulsabbir, @David-Engel

_physicalStateObj.SniContext = SniContext.Snix_ProcessSspi;
// allocate received buffer based on length from SSPI message
byte[] receivedBuff = new byte[receivedLength];
byte[] receivedBuff = ArrayPool<byte>.Shared.Rent(receivedLength);
Copy link
Member

Choose a reason for hiding this comment

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

I assume throw SQL.SynchronousCallMayNotPend(); is not commonly taken, otherwise we should return the array in that path.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a real error if it occurs, it means someone has tried to do asynchronous things in a synchronous context. It's a programming error if that happens and It'll blow up the unit tests all over the place.

@karelz
Copy link
Member

karelz commented Mar 4, 2019

@afsanehr @tarikulsabbir @Gary-Zh @David-Engel what are next steps here? Can you please code review? (it's been 2 weeks since the PR was created ...)

@AfsanehR-zz AfsanehR-zz added this to the 3.0 milestone Mar 5, 2019
@AfsanehR-zz
Copy link
Contributor

@Wraith2 As mentioned, we are working towards an internal deadline and we will start reviewing these PRs starting next week. Thanks @Wraith2 for your contribution and patience.

@Gary-Zh
Copy link
Contributor

Gary-Zh commented Mar 13, 2019

Hi @Wraith2 , we are reviewing this PR but CI fails because to run it properly, we need a newer version of code. Could you please update this PR to the latest?

@Wraith2
Copy link
Contributor Author

Wraith2 commented Mar 13, 2019

Updated.

@Wraith2
Copy link
Contributor Author

Wraith2 commented Mar 14, 2019

The CI failure is a catastrohic failure of the entire ubuntu run, all other legs ran successfully. It could be retriggered but i don't think it's worth it since everything else passed and nothing is linux ditrro specific in this library.

Copy link
Contributor

@Gary-Zh Gary-Zh left a comment

Choose a reason for hiding this comment

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

LGTM

@AfsanehR-zz AfsanehR-zz merged commit 9fef9c9 into dotnet:master Mar 15, 2019
@Wraith2 Wraith2 deleted the sqlfix-tweaks1 branch March 17, 2019 13:24
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
SqlClient minor fix and changes

Commit migrated from dotnet/corefx@9fef9c9
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Potential race condition in SqlConnectionString.cs

7 participants