Major data corruption issue with PR #956 and connection pooling #1344
Description
We have identified a major bug that appears to have been introduced in PR #956. When using connection pooling under high concurrency, cancelled SqlCommand objects may return the connection to the pool with data still in flight. In the reproduction example below, which occurs in 4.0.0-preview2 but is not reproducible prior to the PR in 4.0.0-preview1, we spawn 100 threads that are opening connections, executing a simple SQL statement that passes in a parameter set to a unique GUID and gets it echoed back through a second parameter, and closes the connection, with a low cancellation timeout so that some will fail and some will succeed. We are finding that the parameter values from a previous connection are being returned in a new, unrelated command object. This suggests that the changes made in PR #956 are no longer fully resetting the connection and in-flight data may come into a new command using a connection retrieved from the pool. This could have significant data corruption implications where output parameters from an unrelated command are returned.
Reproduction Code:
private static void Exec()
{
var cancellationToken = new System.Threading.CancellationTokenSource(50);
var connection = new Microsoft.Data.SqlClient.SqlConnection("Data Source=your-data-source;Integrated Security=SSPI;Trust Server Certificate=true;Max Pool Size=256;Min Pool Size=0;");
var expectedGuid = System.Guid.NewGuid();
var cm = new Microsoft.Data.SqlClient.SqlCommand();
cm.Connection = connection;
cm.CommandType = System.Data.CommandType.Text;
cm.CommandText = "select @id2 = @id;";
cm.Parameters.Add(new Microsoft.Data.SqlClient.SqlParameter("@id", System.Data.SqlDbType.UniqueIdentifier) { Value = expectedGuid });
cm.Parameters.Add(new Microsoft.Data.SqlClient.SqlParameter("@id2", System.Data.SqlDbType.UniqueIdentifier) { Direction = System.Data.ParameterDirection.Output });
cm.CommandTimeout = 2;
try
{
connection.Open();
var task = cm.ExecuteNonQueryAsync(cancellationToken.Token);
task.Wait();
}
catch (System.Exception)
{
//ignore cancellations
}
finally
{
connection.Close();
}
if (cm.Parameters["@id2"].Value == null) return;
else if ((System.Guid)cm.Parameters["@id2"].Value != expectedGuid) throw new System.InvalidOperationException(string.Format("OH NO: Expected {0}, Got {1}", expectedGuid, ((System.Guid)cm.Parameters["@id2"].Value).ToString()));
}
public static void Main(string[] args)
{
var threads = new System.Collections.Generic.List<System.Threading.Thread>();
for (int i = 0; i < 100; i++)
{
var t = new System.Threading.Thread(() => { for (int j = 0; j < 1000; j++) Exec(); });
t.Start();
threads.Add(t);
}
for (int i = 0; i < threads.Count; i++) threads[i].Join();
System.Console.WriteLine("END");
System.Console.ReadKey();
}Just a few seconds into execution, you'll see it throw the InvalidOperationException with something like:
System.InvalidOperationException: 'OH NO: Expected f90b93e5-e651-4543-a663-562998fca20d, Got 50f1c311-9e34-4573-9637-e3e3f0a4737b'
The output parameter is clearing being populated with data from another past iteration of the connection.