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

InvalidOperationException when writing a lot of packets asynchronously and using managed SNI implementation #786

Closed
Berreek opened this issue Nov 8, 2020 · 4 comments · Fixed by #796

Comments

@Berreek
Copy link

Berreek commented Nov 8, 2020

Describe the bug

Hey guys.
We are executing stored procedure that sometimes is taking as parameter a data table with a little bit of data (in the prepared repro case it is arround 0.5mb). When we try to do that we sometimes get following exception:

System.InvalidOperationException: Invalid operation. The connection is closed.
   at Microsoft.Data.SqlClient.TdsParserStateObject.WritePacket(Byte flushMode, Boolean canAccumulate)
   at Microsoft.Data.SqlClient.TdsParserStateObject.WriteBytes(ReadOnlySpan`1 b, Int32 len, Int32 offsetBuffer, Boolean canAccumulate, TaskCompletionSource`1 completion, Byte[] array)
   at Microsoft.Data.SqlClient.TdsParserStateObject.WriteByteArray(Byte[] b, Int32 len, Int32 offsetBuffer, Boolean canAccumulate, TaskCompletionSource`1 completion)
   at Microsoft.Data.SqlClient.TdsParser.WriteString(String s, Int32 length, Int32 offset, TdsParserStateObject stateObj, Boolean canAccumulate)
   at Microsoft.Data.SqlClient.TdsValueSetter.SetString(String value, Int32 offset, Int32 length)
   at Microsoft.Data.SqlClient.TdsRecordBufferSetter.SetString(SmiEventSink sink, Int32 ordinal, String value, Int32 offset, Int32 length)
   at Microsoft.Data.SqlClient.Server.ValueUtilsSmi.SetString_Unchecked(SmiEventSink_Default sink, ITypedSettersV3 setters, Int32 ordinal, String value, Int32 offset, Int32 length)
   at Microsoft.Data.SqlClient.Server.ValueUtilsSmi.SetString_LengthChecked(SmiEventSink_Default sink, ITypedSettersV3 setters, Int32 ordinal, SmiMetaData metaData, String value, Int32 offset)
   at Microsoft.Data.SqlClient.Server.ValueUtilsSmi.SetCompatibleValue(SmiEventSink_Default sink, ITypedSettersV3 setters, Int32 ordinal, SmiMetaData metaData, Object value, ExtendedClrTypeCode typeCode, Int32 offset)
   at Microsoft.Data.SqlClient.Server.ValueUtilsSmi.SetCompatibleValueV200(SmiEventSink_Default sink, SmiTypedGetterSetter setters, Int32 ordinal, SmiMetaData metaData, Object value, ExtendedClrTypeCode typeCode, Int32 offset, Int32 length, ParameterPeekAheadValue peekAhead)
   at Microsoft.Data.SqlClient.Server.ValueUtilsSmi.SetDataTable_Unchecked(SmiEventSink_Default sink, SmiTypedGetterSetter setters, Int32 ordinal, SmiMetaData metaData, DataTable value)
   at Microsoft.Data.SqlClient.Server.ValueUtilsSmi.SetCompatibleValueV200(SmiEventSink_Default sink, SmiTypedGetterSetter setters, Int32 ordinal, SmiMetaData metaData, Object value, ExtendedClrTypeCode typeCode, Int32 offset, Int32 length, ParameterPeekAheadValue peekAhead)
   at Microsoft.Data.SqlClient.TdsParser.WriteSmiParameter(SqlParameter param, Int32 paramIndex, Boolean sendDefault, TdsParserStateObject stateObj, Boolean advancedTraceIsOn)
   at Microsoft.Data.SqlClient.TdsParser.TdsExecuteRPC(SqlCommand cmd, _SqlRPC[] rpcArray, Int32 timeout, Boolean inSchema, SqlNotificationRequest notificationRequest, TdsParserStateObject stateObj, Boolean isCommandProc, Boolean sync, TaskCompletionSource`1 completion, Int32 startRpc, Int32 startParam)
   at Microsoft.Data.SqlClient.SqlCommand.RunExecuteReaderTds(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, Boolean isAsync, Int32 timeout, Task& task, Boolean asyncWrite, Boolean inRetry, SqlDataReader ds, Boolean describeParameterEncryptionRequest)
   at Microsoft.Data.SqlClient.SqlCommand.RunExecuteReader(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, TaskCompletionSource`1 completion, Int32 timeout, Task& task, Boolean& usedCache, Boolean asyncWrite, Boolean inRetry, String method)
   at Microsoft.Data.SqlClient.SqlCommand.BeginExecuteReaderInternal(CommandBehavior behavior, AsyncCallback callback, Object stateObject, Int32 timeout, Boolean inRetry, Boolean asyncWrite)
   at Microsoft.Data.SqlClient.SqlCommand.BeginExecuteReaderAsync(CommandBehavior behavior, AsyncCallback callback, Object stateObject)
   at Microsoft.Data.SqlClient.SqlCommand.<>c__DisplayClass173_0.<ExecuteReaderAsync>b__0(CommandBehavior commandBehavior, AsyncCallback callback, Object stateObject)
   at System.Threading.Tasks.TaskFactory`1.FromAsyncImpl[TArg1](Func`4 beginMethod, Func`2 endFunction, Action`1 endAction, TArg1 arg1, Object state, TaskCreationOptions creationOptions)
   at System.Threading.Tasks.TaskFactory`1.FromAsync[TArg1](Func`4 beginMethod, Func`2 endMethod, TArg1 arg1, Object state)
   at Microsoft.Data.SqlClient.SqlCommand.ExecuteReaderAsync(CommandBehavior behavior, CancellationToken cancellationToken)

When I digged into the code I saw that connection is being closed here:


And the exception received in that method is:

Microsoft.Data.SqlClient.SqlException (0x80131904): A transport-level error has occurred when sending the request to the server. (provider: TCP Provider, error: 35 - An internal exception was caught)
 ---> System.NotSupportedException:  The WriteAsync method cannot be called when another write operation is pending.
   at System.Net.Security.SslStream.WriteAsyncInternal[TIOAdapter](TIOAdapter writeAdapter, ReadOnlyMemory`1 buffer)
   at Microsoft.Data.SqlClient.SNI.SNIPacket.WriteToStreamAsync(Stream stream, SNIAsyncCallback callback, SNIProviders provider)
ClientConnectionId:fc17a791-7e6e-4ba9-aeed-5d086c8206c4

That led me to following methods:

public override uint SendAsync(SNIPacket packet, SNIAsyncCallback callback = null)

public async void WriteToStreamAsync(Stream stream, SNIAsyncCallback callback, SNIProviders provider)

So SendAsync returns just uint and inside that method we have following call of WriteToStreamAsync which is a async void:

packet.WriteToStreamAsync(_stream, cb, SNIProviders.TCP_PROV);

Since it's a async void it means it's fire and forget and there might be another call of WriteToStreamAsync before previous one completes which lead to writing into the same stream by multiple threads concurrently at the same time that is causing the NotSupportedException (there is also a lock in SendAsync which I don't know what is a purpose of it but it does not defend against that case).

If I use sync version of ExecuteReader or non-managed SNI on Windows it works fine. I also tried to play with packetSize property in connection string to increase/decrease but it caused the exception anyway.

To reproduce

Here is a solution project with reproduction.

Expected behavior

Execute without error like in non-managed SNI implementation or sync version.

Further technical details

Microsoft.Data.SqlClient version: 1.1.3/2.0.1
.NET target: Net Core 3.1/.net5
SQL Server version: Azure Sql Server
Operating system: Windows(with managed SNI)/ Alpine docker container

@Berreek Berreek changed the title Race condition when writing a lot of packets asynchronously and using managed SNI implementation InvalidOperationExcpetion when writing a lot of packets asynchronously and using managed SNI implementation Nov 8, 2020
@Berreek Berreek changed the title InvalidOperationExcpetion when writing a lot of packets asynchronously and using managed SNI implementation InvalidOperationException when writing a lot of packets asynchronously and using managed SNI implementation Nov 8, 2020
@cheenamalhotra
Copy link
Member

Hi @Berreek

You may wanna try #579 and validate the fix for your case.
Just FYI, it's still work in progress as there're some concerns we need to address.

@Berreek
Copy link
Author

Berreek commented Nov 9, 2020

Hey @cheenamalhotra

The fix works but as it is mentioned in the PR by @roji it might lead to thread pool starvation with that _currentTask.Wait(cancellationToken).

Is there any way to get the fix for it into 2.1 version? Looking at work arrounds in other issues:

  • turning on MARS - we turned it off beacuse of this
  • switching to non-managed SNI - we are running everything on Linux
  • switching to sync version - that's what we had to do for queries that are experiencing that but it's far from ideal since we are forced to do sync IO.

Is there any other work arround that could help us? I just feel like the asynchronous API is too much unreliable in such cases beacuse of it but from the other hand using sync API does not scale :(

@cheenamalhotra
Copy link
Member

@Berreek

Please try out #796 and let us know!

SqlClient Triage Board automation moved this from Under Investigation to Closed Nov 19, 2020
@Berreek
Copy link
Author

Berreek commented Nov 23, 2020

@cheenamalhotra Works great in 2.1. Thanks for your help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
2 participants