Skip to content

Commit 4b4c3ba

Browse files
benrr101benrr101
authored
Cleanup | Remove Abort/Doom From AsyncHelpers (#3683)
* Condense duplicate cases in ContinueTaskWithState * Remove connectionToDoom and connectionToAbort from AsyncHelper.ContinueTask * Remove connection to abort AsyncHelpers.CreateContinuationTask * Remove connectionToDoom from CreateContinuationTask * Unify netcore/netfx exception converter - it was only used in one place and what made them different wasn't even utilized! * Missed one more reference
1 parent fd2aac8 commit 4b4c3ba

File tree

5 files changed

+75
-152
lines changed

5 files changed

+75
-152
lines changed

‎src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlBulkCopy.cs‎

Lines changed: 42 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -2361,7 +2361,10 @@ private Task CopyColumnsAsync(int col, TaskCompletionSource<object> source = nul
23612361
// This is in its own method to avoid always allocating the lambda in CopyColumnsAsync
23622362
private void CopyColumnsAsyncSetupContinuation(TaskCompletionSource<object> source, Task task, int i)
23632363
{
2364-
AsyncHelper.ContinueTaskWithState(task, source, this,
2364+
AsyncHelper.ContinueTaskWithState(
2365+
task,
2366+
source,
2367+
state: this,
23652368
onSuccess: (object state) =>
23662369
{
23672370
SqlBulkCopy sqlBulkCopy = (SqlBulkCopy)state;
@@ -2373,9 +2376,7 @@ private void CopyColumnsAsyncSetupContinuation(TaskCompletionSource<object> sour
23732376
{
23742377
source.SetResult(null);
23752378
}
2376-
},
2377-
connectionToDoom: _connection.GetOpenTdsConnection()
2378-
);
2379+
});
23792380
}
23802381

23812382
// The notification logic.
@@ -2510,10 +2511,11 @@ private Task CopyRowsAsync(int rowsSoFar, int totalRows, CancellationToken cts,
25102511
}
25112512
resultTask = source.Task;
25122513

2513-
AsyncHelper.ContinueTaskWithState(readTask, source, this,
2514-
onSuccess: (object state) => ((SqlBulkCopy)state).CopyRowsAsync(i + 1, totalRows, cts, source),
2515-
connectionToDoom: _connection.GetOpenTdsConnection()
2516-
);
2514+
AsyncHelper.ContinueTaskWithState(
2515+
readTask,
2516+
source,
2517+
state: this,
2518+
onSuccess: (object state) => ((SqlBulkCopy)state).CopyRowsAsync(i + 1, totalRows, cts, source));
25172519
return resultTask; // Associated task will be completed when all rows are copied to server/exception/cancelled.
25182520
}
25192521
}
@@ -2535,14 +2537,13 @@ private Task CopyRowsAsync(int rowsSoFar, int totalRows, CancellationToken cts,
25352537
}
25362538
else
25372539
{
2538-
AsyncHelper.ContinueTaskWithState(readTask, source, sqlBulkCopy,
2539-
onSuccess: (object state2) => ((SqlBulkCopy)state2).CopyRowsAsync(i + 1, totalRows, cts, source),
2540-
connectionToDoom: _connection.GetOpenTdsConnection()
2541-
);
2540+
AsyncHelper.ContinueTaskWithState(
2541+
readTask,
2542+
source,
2543+
state: sqlBulkCopy,
2544+
onSuccess: (object state2) => ((SqlBulkCopy)state2).CopyRowsAsync(i + 1, totalRows, cts, source));
25422545
}
2543-
},
2544-
connectionToDoom: _connection.GetOpenTdsConnection()
2545-
);
2546+
});
25462547
return resultTask;
25472548
}
25482549
}
@@ -2611,7 +2612,10 @@ private Task CopyBatchesAsync(BulkCopySimpleResultSet internalResults, string up
26112612
source = new TaskCompletionSource<object>();
26122613
}
26132614

2614-
AsyncHelper.ContinueTaskWithState(commandTask, source, this,
2615+
AsyncHelper.ContinueTaskWithState(
2616+
commandTask,
2617+
source,
2618+
state: this,
26152619
onSuccess: (object state) =>
26162620
{
26172621
SqlBulkCopy sqlBulkCopy = (SqlBulkCopy)state;
@@ -2621,9 +2625,7 @@ private Task CopyBatchesAsync(BulkCopySimpleResultSet internalResults, string up
26212625
// Continuation finished sync, recall into CopyBatchesAsync to continue
26222626
sqlBulkCopy.CopyBatchesAsync(internalResults, updateBulkCommandText, cts, source);
26232627
}
2624-
},
2625-
connectionToDoom: _connection.GetOpenTdsConnection()
2626-
);
2628+
});
26272629
return source.Task;
26282630
}
26292631
}
@@ -2677,7 +2679,10 @@ private Task CopyBatchesAsyncContinued(BulkCopySimpleResultSet internalResults,
26772679
{ // First time only
26782680
source = new TaskCompletionSource<object>();
26792681
}
2680-
AsyncHelper.ContinueTaskWithState(task, source, this,
2682+
AsyncHelper.ContinueTaskWithState(
2683+
task,
2684+
source,
2685+
state: this,
26812686
onSuccess: (object state) =>
26822687
{
26832688
SqlBulkCopy sqlBulkCopy = (SqlBulkCopy)state;
@@ -2689,9 +2694,7 @@ private Task CopyBatchesAsyncContinued(BulkCopySimpleResultSet internalResults,
26892694
}
26902695
},
26912696
onFailure: static (Exception _, object state) => ((SqlBulkCopy)state).CopyBatchesAsyncContinuedOnError(cleanupParser: false),
2692-
onCancellation: static (object state) => ((SqlBulkCopy)state).CopyBatchesAsyncContinuedOnError(cleanupParser: true),
2693-
connectionToDoom: _connection.GetOpenTdsConnection()
2694-
);
2697+
onCancellation: static (object state) => ((SqlBulkCopy)state).CopyBatchesAsyncContinuedOnError(cleanupParser: true));
26952698

26962699
return source.Task;
26972700
}
@@ -2738,7 +2741,10 @@ private Task CopyBatchesAsyncContinuedOnSuccess(BulkCopySimpleResultSet internal
27382741
source = new TaskCompletionSource<object>();
27392742
}
27402743

2741-
AsyncHelper.ContinueTaskWithState(writeTask, source, this,
2744+
AsyncHelper.ContinueTaskWithState(
2745+
writeTask,
2746+
source,
2747+
state: this,
27422748
onSuccess: (object state) =>
27432749
{
27442750
SqlBulkCopy sqlBulkCopy = (SqlBulkCopy)state;
@@ -2756,9 +2762,7 @@ private Task CopyBatchesAsyncContinuedOnSuccess(BulkCopySimpleResultSet internal
27562762
// Always call back into CopyBatchesAsync
27572763
sqlBulkCopy.CopyBatchesAsync(internalResults, updateBulkCommandText, cts, source);
27582764
},
2759-
onFailure: static (Exception _, object state) => ((SqlBulkCopy)state).CopyBatchesAsyncContinuedOnError(cleanupParser: false),
2760-
connectionToDoom: _connection.GetOpenTdsConnection()
2761-
);
2765+
onFailure: static (Exception _, object state) => ((SqlBulkCopy)state).CopyBatchesAsyncContinuedOnError(cleanupParser: false));
27622766
return source.Task;
27632767
}
27642768
}
@@ -2859,7 +2863,10 @@ private void WriteToServerInternalRestContinuedAsync(BulkCopySimpleResultSet int
28592863
{
28602864
source = new TaskCompletionSource<object>();
28612865
}
2862-
AsyncHelper.ContinueTaskWithState(task, source, this,
2866+
AsyncHelper.ContinueTaskWithState(
2867+
task,
2868+
source,
2869+
state: this,
28632870
onSuccess: (object state) =>
28642871
{
28652872
SqlBulkCopy sqlBulkCopy = (SqlBulkCopy)state;
@@ -2902,9 +2909,7 @@ private void WriteToServerInternalRestContinuedAsync(BulkCopySimpleResultSet int
29022909
}
29032910
}
29042911
}
2905-
},
2906-
connectionToDoom: _connection.GetOpenTdsConnection()
2907-
);
2912+
});
29082913
return;
29092914
}
29102915
else
@@ -3029,14 +3034,9 @@ private void WriteToServerInternalRestAsync(CancellationToken cts, TaskCompletio
30293034
_parserLock.Wait(canReleaseFromAnyThread: true);
30303035
WriteToServerInternalRestAsync(cts, source);
30313036
},
3032-
connectionToAbort: _connection,
30333037
onFailure: static (_, state) => ((StrongBox<CancellationTokenRegistration>)state).Value.Dispose(),
30343038
onCancellation: static state => ((StrongBox<CancellationTokenRegistration>)state).Value.Dispose(),
3035-
#if NET
30363039
exceptionConverter: ex => SQL.BulkLoadInvalidDestinationTable(_destinationTableName, ex)
3037-
#else
3038-
exceptionConverter: (ex, _) => SQL.BulkLoadInvalidDestinationTable(_destinationTableName, ex)
3039-
#endif
30403040
);
30413041
return;
30423042
}
@@ -3085,10 +3085,11 @@ private void WriteToServerInternalRestAsync(CancellationToken cts, TaskCompletio
30853085

30863086
if (internalResultsTask != null)
30873087
{
3088-
AsyncHelper.ContinueTaskWithState(internalResultsTask, source, this,
3089-
onSuccess: (object state) => ((SqlBulkCopy)state).WriteToServerInternalRestContinuedAsync(internalResultsTask.Result, cts, source),
3090-
connectionToDoom: _connection.GetOpenTdsConnection()
3091-
);
3088+
AsyncHelper.ContinueTaskWithState(
3089+
internalResultsTask,
3090+
source,
3091+
state: this,
3092+
onSuccess: (object state) => ((SqlBulkCopy)state).WriteToServerInternalRestContinuedAsync(internalResultsTask.Result, cts, source));
30923093
}
30933094
else
30943095
{
@@ -3169,9 +3170,7 @@ private Task WriteToServerInternalAsync(CancellationToken ctoken)
31693170
{
31703171
sqlBulkCopy.WriteToServerInternalRestAsync(ctoken, source); // Passing the same completion which will be completed by the Callee.
31713172
}
3172-
},
3173-
connectionToDoom: _connection.GetOpenTdsConnection()
3174-
);
3173+
});
31753174
return resultTask;
31763175
}
31773176
}

‎src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Reader.cs‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1749,11 +1749,7 @@ private SqlDataReader RunExecuteReaderTdsWithTransparentParameterEncryption(
17491749
onCancellation: static state =>
17501750
{
17511751
((SqlCommand)state).CachedAsyncState?.ResetAsyncState();
1752-
}
1753-
#if NETFRAMEWORK
1754-
, connectionToAbort: _activeConnection
1755-
#endif
1756-
);
1752+
});
17571753

17581754
task = completion.Task;
17591755
return ds;

‎src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlUtil.cs‎

Lines changed: 20 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -51,11 +51,10 @@ internal static ArgumentOutOfRangeException InvalidMinAndMaxPair(string minParam
5151

5252
internal static class AsyncHelper
5353
{
54-
internal static Task CreateContinuationTask(Task task, Action onSuccess,
55-
#if NETFRAMEWORK
56-
SqlInternalConnectionTds connectionToDoom = null,
57-
#endif
58-
Action<Exception> onFailure = null)
54+
internal static Task CreateContinuationTask(
55+
Task task,
56+
Action onSuccess,
57+
Action<Exception> onFailure = null)
5958
{
6059
if (task == null)
6160
{
@@ -65,8 +64,9 @@ internal static Task CreateContinuationTask(Task task, Action onSuccess,
6564
else
6665
{
6766
TaskCompletionSource<object> completion = new TaskCompletionSource<object>();
68-
#if NET
69-
ContinueTaskWithState(task, completion,
67+
ContinueTaskWithState(
68+
task,
69+
completion,
7070
state: Tuple.Create(onSuccess, onFailure, completion),
7171
onSuccess: static (object state) =>
7272
{
@@ -82,16 +82,6 @@ internal static Task CreateContinuationTask(Task task, Action onSuccess,
8282
Action<Exception> failure = parameters.Item2;
8383
failure?.Invoke(exception);
8484
}
85-
#else
86-
ContinueTask(task, completion,
87-
onSuccess: () =>
88-
{
89-
onSuccess();
90-
completion.SetResult(null);
91-
},
92-
onFailure: onFailure,
93-
connectionToDoom: connectionToDoom
94-
#endif
9585
);
9686
return completion.Task;
9787
}
@@ -119,32 +109,23 @@ internal static Task CreateContinuationTaskWithState(Task task, object state, Ac
119109
}
120110
}
121111

122-
internal static Task CreateContinuationTask<T1, T2>(Task task, Action<T1, T2> onSuccess, T1 arg1, T2 arg2, SqlInternalConnectionTds connectionToDoom = null, Action<Exception> onFailure = null)
112+
internal static Task CreateContinuationTask<T1, T2>(
113+
Task task,
114+
Action<T1, T2> onSuccess,
115+
T1 arg1,
116+
T2 arg2,
117+
Action<Exception> onFailure = null)
123118
{
124-
return CreateContinuationTask(task, () => onSuccess(arg1, arg2),
125-
#if NETFRAMEWORK
126-
connectionToDoom,
127-
#endif
128-
onFailure);
119+
return CreateContinuationTask(task, () => onSuccess(arg1, arg2), onFailure);
129120
}
130121

131122
internal static void ContinueTask(Task task,
132123
TaskCompletionSource<object> completion,
133124
Action onSuccess,
134125
Action<Exception> onFailure = null,
135126
Action onCancellation = null,
136-
Func<Exception, Exception> exceptionConverter = null,
137-
#if NET
138-
SqlInternalConnectionTds connectionToDoom = null
139-
#else
140-
SqlInternalConnectionTds connectionToDoom = null,
141-
SqlConnection connectionToAbort = null
142-
#endif
143-
)
127+
Func<Exception, Exception> exceptionConverter = null)
144128
{
145-
#if NETFRAMEWORK
146-
Debug.Assert((connectionToAbort == null) || (connectionToDoom == null), "Should not specify both connectionToDoom and connectionToAbort");
147-
#endif
148129
task.ContinueWith(
149130
tsk =>
150131
{
@@ -177,42 +158,16 @@ internal static void ContinueTask(Task task,
177158
}
178159
else
179160
{
180-
#if NETFRAMEWORK
181-
if (connectionToDoom != null || connectionToAbort != null)
182-
{
183-
try
184-
{
185-
onSuccess();
186-
}
187-
// @TODO: CER Exception Handling was removed here (see GH#3581)
188-
catch (Exception e)
189-
{
190-
completion.SetException(e);
191-
}
192-
}
193-
else
194-
{ // no connection to doom - reliability section not required
195-
try
196-
{
197-
onSuccess();
198-
}
199-
catch (Exception e)
200-
{
201-
completion.SetException(e);
202-
}
203-
}
204-
}
205-
#else
206161
try
207162
{
208163
onSuccess();
209164
}
165+
// @TODO: CER Exception Handling was removed here (see GH#3581)
210166
catch (Exception e)
211167
{
212168
completion.SetException(e);
213169
}
214170
}
215-
#endif
216171
}, TaskScheduler.Default
217172
);
218173
}
@@ -225,18 +180,8 @@ internal static void ContinueTaskWithState(Task task,
225180
Action<object> onSuccess,
226181
Action<Exception, object> onFailure = null,
227182
Action<object> onCancellation = null,
228-
#if NET
229-
Func<Exception, Exception> exceptionConverter = null,
230-
#else
231-
Func<Exception, object, Exception> exceptionConverter = null,
232-
#endif
233-
SqlInternalConnectionTds connectionToDoom = null,
234-
SqlConnection connectionToAbort = null
235-
)
183+
Func<Exception, Exception> exceptionConverter = null)
236184
{
237-
#if NETFRAMEWORK
238-
Debug.Assert((connectionToAbort == null) || (connectionToDoom == null), "Should not specify both connectionToDoom and connectionToAbort");
239-
#endif
240185
task.ContinueWith(
241186
(Task tsk, object state2) =>
242187
{
@@ -245,12 +190,9 @@ internal static void ContinueTaskWithState(Task task,
245190
Exception exc = tsk.Exception.InnerException;
246191
if (exceptionConverter != null)
247192
{
248-
exc = exceptionConverter(exc
249-
#if NETFRAMEWORK
250-
, state2
251-
#endif
252-
);
193+
exc = exceptionConverter(exc);
253194
}
195+
254196
try
255197
{
256198
onFailure?.Invoke(exc, state2);
@@ -271,24 +213,13 @@ internal static void ContinueTaskWithState(Task task,
271213
completion.TrySetCanceled();
272214
}
273215
}
274-
else if (connectionToDoom != null || connectionToAbort != null)
275-
{
276-
try
277-
{
278-
onSuccess(state2);
279-
}
280-
// @TODO: CER Exception Handling was removed here (see GH#3581)
281-
catch (Exception e)
282-
{
283-
completion.SetException(e);
284-
}
285-
}
286216
else
287217
{
288218
try
289219
{
290220
onSuccess(state2);
291221
}
222+
// @TODO: CER Exception Handling was removed here (see GH#3581)
292223
catch (Exception e)
293224
{
294225
completion.SetException(e);

0 commit comments

Comments
 (0)