Fix potential race condition in FinishQuerying. Fixes #1341

If TryStartCancel were called between the two 'lock' blocks, the session could be put into an unexpected state. Instead, perform all state updates in the first 'lock' instead of deferring cleanup work. This should also improve efficiency for the common (not cancelled) path.

Signed-off-by: Bradley Grainger <[email protected]>

Author: bgrainger

src/MySqlConnector/Core/ServerSession.cs

@@ -296,14 +296,23 @@ public void StartQuerying(ICancellableCommand command)
 	public void FinishQuerying()
 	{
 		EnteringFinishQuerying(m_logger, Id, m_state);
-		bool clearConnection = false;
+		var clearConnection = false;
 		lock (m_lock)
 		{
+			// entering this method, we expect to be in Querying, CancelingQuery, or Failed state
 			if (m_state == State.CancelingQuery)
 			{
 				m_state = State.ClearingPendingCancellation;
 				clearConnection = true;
 			}
+			else
+			{
+				if (m_state is State.Querying)
+					m_state = State.Connected;
+				else
+					VerifyState(State.Failed);
+				ActiveCommandId = 0;
+			}
 		}
 
 		if (clearConnection)
@@ -318,15 +327,13 @@ public void FinishQuerying()
 			payload = ReceiveReplyAsync(IOBehavior.Synchronous, CancellationToken.None).GetAwaiter().GetResult();
 #pragma warning restore CA2012
 			OkPayload.Verify(payload.Span, this);
-		}
 
-		lock (m_lock)
-		{
-			if (m_state is State.Querying or State.ClearingPendingCancellation)
+			lock (m_lock)
+			{
+				VerifyState(State.ClearingPendingCancellation);
 				m_state = State.Connected;
-			else
-				VerifyState(State.Failed);
-			ActiveCommandId = 0;
+				ActiveCommandId = 0;
+			}
 		}
 	}