From ad763b8fa344d863c7389dc0b7c98cfb48f18bca Mon Sep 17 00:00:00 2001 From: Jack Mills Date: Thu, 13 Aug 2020 11:07:06 +0100 Subject: [PATCH 01/10] Create an example of how TaskHelper.WaitForCompletion leaks unobserved exceptions --- .../src/Microsoft/Data/SqlClient/SqlUtil.cs | 7 ++- .../Microsoft.Data.SqlClient.Tests.csproj | 1 + .../tests/FunctionalTests/SqlHelperTests.cs | 49 +++++++++++++++++++ 3 files changed, 55 insertions(+), 2 deletions(-) create mode 100644 src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlHelperTests.cs diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlUtil.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlUtil.cs index ebfe96b769..68662183cb 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlUtil.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlUtil.cs @@ -19,7 +19,8 @@ namespace Microsoft.Data.SqlClient { - internal static class AsyncHelper +#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member + public static class AsyncHelper { internal static Task CreateContinuationTask(Task task, Action onSuccess, Action onFailure = null) { @@ -188,7 +189,7 @@ internal static Task CreateContinuationTaskWithState(Task task, object state, Ac ); } - internal static void WaitForCompletion(Task task, int timeout, Action onTimeout = null, bool rethrowExceptions = true) + public static void WaitForCompletion(Task task, int timeout, Action onTimeout = null, bool rethrowExceptions = true) { try { @@ -204,6 +205,7 @@ internal static void WaitForCompletion(Task task, int timeout, Action onTimeout } if (!task.IsCompleted) { + task.ContinueWith(_ => { }); //We're handling the error with onTimeout if (onTimeout != null) { onTimeout(); @@ -225,6 +227,7 @@ internal static void SetTimeoutException(TaskCompletionSource completion } } } +#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member internal static class SQL { diff --git a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/Microsoft.Data.SqlClient.Tests.csproj b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/Microsoft.Data.SqlClient.Tests.csproj index ed7e701212..757260f56e 100644 --- a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/Microsoft.Data.SqlClient.Tests.csproj +++ b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/Microsoft.Data.SqlClient.Tests.csproj @@ -40,6 +40,7 @@ + diff --git a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlHelperTests.cs b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlHelperTests.cs new file mode 100644 index 0000000000..3f4b7c9a36 --- /dev/null +++ b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlHelperTests.cs @@ -0,0 +1,49 @@ +using System; +using System.Threading; +using System.Threading.Tasks; +using Xunit; +using Xunit.Abstractions; + +namespace Microsoft.Data.SqlClient.Tests +{ + public class SqlHelperTests + { + private readonly ITestOutputHelper output; + public SqlHelperTests(ITestOutputHelper output) + { + this.output = output; + } + + AutoResetEvent UnobservedExceptionHappened = new AutoResetEvent(false); + + private void UnobservedExceptionHanlder(object sender, UnobservedTaskExceptionEventArgs eventArgs) + { + output.WriteLine("Exception unobserved"); + UnobservedExceptionHappened.Set(); + } + + private void HandleTimeout() + { + output.WriteLine("Timeout handled"); + } + + private void DoWork() + { + TaskScheduler.UnobservedTaskException += UnobservedExceptionHanlder; + TaskCompletionSource tcs = new TaskCompletionSource(); + AsyncHelper.WaitForCompletion(tcs.Task, 1, HandleTimeout); //Will time out as task uncompleted + tcs.SetException(new TimeoutException()); //Our task now completes as timed out + } + + [Fact] + public void ReproduceUnobservedException() + { + DoWork(); //Do the main work in another function so the task has no reference remaining + GC.Collect(); //Force collection of unobserved task + GC.WaitForPendingFinalizers(); + + bool unobservedHappend = UnobservedExceptionHappened.WaitOne(1); + Assert.False(unobservedHappend, "Did not expect an exception to be unobserved"); + } + } +} From c4d6ed23bcf6dcb6b92ed6e00600ac904f567dc5 Mon Sep 17 00:00:00 2001 From: Jack Mills Date: Thu, 13 Aug 2020 11:37:13 +0100 Subject: [PATCH 02/10] Use internalsvisibleto rather than making things public --- .../netcore/src/Microsoft/Data/SqlClient/SqlUtil.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlUtil.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlUtil.cs index 68662183cb..e257c381f4 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlUtil.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlUtil.cs @@ -17,10 +17,11 @@ using System.Transactions; using Microsoft.Data.Common; +[assembly: InternalsVisibleTo("FunctionalTests")] + namespace Microsoft.Data.SqlClient { -#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member - public static class AsyncHelper + internal static class AsyncHelper { internal static Task CreateContinuationTask(Task task, Action onSuccess, Action onFailure = null) { @@ -189,7 +190,7 @@ internal static Task CreateContinuationTaskWithState(Task task, object state, Ac ); } - public static void WaitForCompletion(Task task, int timeout, Action onTimeout = null, bool rethrowExceptions = true) + internal static void WaitForCompletion(Task task, int timeout, Action onTimeout = null, bool rethrowExceptions = true) { try { @@ -227,7 +228,6 @@ internal static void SetTimeoutException(TaskCompletionSource completion } } } -#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member internal static class SQL { From 2618c65da243436c405f521020c43d118222c132 Mon Sep 17 00:00:00 2001 From: Jack Mills Date: Fri, 14 Aug 2020 15:49:08 +0100 Subject: [PATCH 03/10] Tidy up pre PR --- .../src/Microsoft/Data/SqlClient/SqlUtil.cs | 2 +- .../Microsoft.Data.SqlClient.Tests.csproj | 2 +- .../tests/FunctionalTests/SqlHelperTest.cs | 45 +++++++++++++++++ .../tests/FunctionalTests/SqlHelperTests.cs | 49 ------------------- 4 files changed, 47 insertions(+), 51 deletions(-) create mode 100644 src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlHelperTest.cs delete mode 100644 src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlHelperTests.cs diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlUtil.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlUtil.cs index e257c381f4..a1a297ed99 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlUtil.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlUtil.cs @@ -206,7 +206,7 @@ internal static void WaitForCompletion(Task task, int timeout, Action onTimeout } if (!task.IsCompleted) { - task.ContinueWith(_ => { }); //We're handling the error with onTimeout + task.ContinueWith(_ => { }); //Ensure the task does not leave an unobserved exception if (onTimeout != null) { onTimeout(); diff --git a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/Microsoft.Data.SqlClient.Tests.csproj b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/Microsoft.Data.SqlClient.Tests.csproj index 757260f56e..cf8fd9ca04 100644 --- a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/Microsoft.Data.SqlClient.Tests.csproj +++ b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/Microsoft.Data.SqlClient.Tests.csproj @@ -40,7 +40,7 @@ - + diff --git a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlHelperTest.cs b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlHelperTest.cs new file mode 100644 index 0000000000..0d4dcded37 --- /dev/null +++ b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlHelperTest.cs @@ -0,0 +1,45 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Threading; +using System.Threading.Tasks; +using Xunit; + +namespace Microsoft.Data.SqlClient.Tests +{ + public class SqlHelperTest + { + private void TimeOutATask() + { + TaskCompletionSource tcs = new TaskCompletionSource(); + AsyncHelper.WaitForCompletion(tcs.Task, 1); //Will time out as task uncompleted + tcs.SetException(new TimeoutException()); //Our task now completes with an error + } + + [Fact] + public void WaitForCompletion_DoesNotCreateUnobservedException() + { + var unobservedExceptionHappenedEvent = new AutoResetEvent(false); + EventHandler handleUnobservedException = + (o, a) => { unobservedExceptionHappenedEvent.Set(); }; + + TaskScheduler.UnobservedTaskException += handleUnobservedException; + + try + { + TimeOutATask(); //Create the task in another function so the task has no reference remaining + GC.Collect(); //Force collection of unobserved task + GC.WaitForPendingFinalizers(); + + bool unobservedExceptionHappend = unobservedExceptionHappenedEvent.WaitOne(1); + Assert.False(unobservedExceptionHappend, "Did not expect an unobserved exception"); + } + finally + { + TaskScheduler.UnobservedTaskException -= handleUnobservedException; + } + } + } +} diff --git a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlHelperTests.cs b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlHelperTests.cs deleted file mode 100644 index 3f4b7c9a36..0000000000 --- a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlHelperTests.cs +++ /dev/null @@ -1,49 +0,0 @@ -using System; -using System.Threading; -using System.Threading.Tasks; -using Xunit; -using Xunit.Abstractions; - -namespace Microsoft.Data.SqlClient.Tests -{ - public class SqlHelperTests - { - private readonly ITestOutputHelper output; - public SqlHelperTests(ITestOutputHelper output) - { - this.output = output; - } - - AutoResetEvent UnobservedExceptionHappened = new AutoResetEvent(false); - - private void UnobservedExceptionHanlder(object sender, UnobservedTaskExceptionEventArgs eventArgs) - { - output.WriteLine("Exception unobserved"); - UnobservedExceptionHappened.Set(); - } - - private void HandleTimeout() - { - output.WriteLine("Timeout handled"); - } - - private void DoWork() - { - TaskScheduler.UnobservedTaskException += UnobservedExceptionHanlder; - TaskCompletionSource tcs = new TaskCompletionSource(); - AsyncHelper.WaitForCompletion(tcs.Task, 1, HandleTimeout); //Will time out as task uncompleted - tcs.SetException(new TimeoutException()); //Our task now completes as timed out - } - - [Fact] - public void ReproduceUnobservedException() - { - DoWork(); //Do the main work in another function so the task has no reference remaining - GC.Collect(); //Force collection of unobserved task - GC.WaitForPendingFinalizers(); - - bool unobservedHappend = UnobservedExceptionHappened.WaitOne(1); - Assert.False(unobservedHappend, "Did not expect an exception to be unobserved"); - } - } -} From 60cea014ec04e97ee9f06cf563736b866f9c519c Mon Sep 17 00:00:00 2001 From: Jack Mills Date: Fri, 14 Aug 2020 16:38:21 +0100 Subject: [PATCH 04/10] Add more detailed info on the unhandled exception to diagnose build failures --- .../tests/FunctionalTests/SqlHelperTest.cs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlHelperTest.cs b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlHelperTest.cs index 0d4dcded37..89b9228f7a 100644 --- a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlHelperTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlHelperTest.cs @@ -15,15 +15,21 @@ private void TimeOutATask() { TaskCompletionSource tcs = new TaskCompletionSource(); AsyncHelper.WaitForCompletion(tcs.Task, 1); //Will time out as task uncompleted - tcs.SetException(new TimeoutException()); //Our task now completes with an error + tcs.SetException(new TimeoutException("Dummy timeout exception")); //Our task now completes with an error + } + + private Exception UnwrapException(Exception e) + { + return e?.InnerException != null ? UnwrapException(e.InnerException) : e; } [Fact] public void WaitForCompletion_DoesNotCreateUnobservedException() { var unobservedExceptionHappenedEvent = new AutoResetEvent(false); + Exception unhandledException = null; EventHandler handleUnobservedException = - (o, a) => { unobservedExceptionHappenedEvent.Set(); }; + (o, a) => { unhandledException = a.Exception; unobservedExceptionHappenedEvent.Set(); }; TaskScheduler.UnobservedTaskException += handleUnobservedException; @@ -34,7 +40,11 @@ public void WaitForCompletion_DoesNotCreateUnobservedException() GC.WaitForPendingFinalizers(); bool unobservedExceptionHappend = unobservedExceptionHappenedEvent.WaitOne(1); - Assert.False(unobservedExceptionHappend, "Did not expect an unobserved exception"); + if (unobservedExceptionHappend) //Save doing string interpolation in the happy case + { + var e = UnwrapException(unhandledException); + Assert.False(true, $"Did not expect an unobserved exception, but found a {e?.GetType()} with message \"{e?.Message}\""); + } } finally { From 4a8b3e94673ed50e0ed4596bd7a7e89146aeaf0d Mon Sep 17 00:00:00 2001 From: Jack Mills Date: Fri, 14 Aug 2020 16:59:29 +0100 Subject: [PATCH 05/10] Patch fix to netfx version of SqlUtil too --- .../netfx/src/Microsoft/Data/SqlClient/SqlUtil.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlUtil.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlUtil.cs index 6318a1491f..3929d1422f 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlUtil.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlUtil.cs @@ -17,6 +17,8 @@ using System.Threading.Tasks; using SysTx = System.Transactions; +[assembly: InternalsVisibleTo("FunctionalTests")] + namespace Microsoft.Data.SqlClient { using Microsoft.Data.Common; @@ -189,6 +191,7 @@ internal static void WaitForCompletion(Task task, int timeout, Action onTimeout } if (!task.IsCompleted) { + task.ContinueWith(_ => { }); //Ensure the task does not leave an unobserved exception if (onTimeout != null) { onTimeout(); From 3b0939283a8514cb3ac9ac62e640f2d57f6d9792 Mon Sep 17 00:00:00 2001 From: Jack Mills Date: Sat, 15 Aug 2020 11:41:28 +0100 Subject: [PATCH 06/10] Stop other unit tests from leaving unobserved exceptions --- .../BaseProviderAsyncTest.cs | 43 +++++++++++-------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/BaseProviderAsyncTest/BaseProviderAsyncTest.cs b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/BaseProviderAsyncTest/BaseProviderAsyncTest.cs index 1c340f771b..07e7461c7b 100644 --- a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/BaseProviderAsyncTest/BaseProviderAsyncTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/BaseProviderAsyncTest/BaseProviderAsyncTest.cs @@ -13,6 +13,11 @@ namespace Microsoft.Data.SqlClient.ManualTesting.Tests { public static class BaseProviderAsyncTest { + private static void AssertTaskFaults(Task t) + { + Assert.ThrowsAny(() => t.Wait(TimeSpan.FromMilliseconds(1))); + } + [Fact] public static void TestDbConnection() { @@ -37,8 +42,8 @@ public static void TestDbConnection() { Fail = true }; - connectionFail.OpenAsync().ContinueWith((t) => { }, TaskContinuationOptions.OnlyOnFaulted).Wait(); - connectionFail.OpenAsync(source.Token).ContinueWith((t) => { }, TaskContinuationOptions.OnlyOnFaulted).Wait(); + AssertTaskFaults(connectionFail.OpenAsync()); + AssertTaskFaults(connectionFail.OpenAsync(source.Token)); // Verify base implementation does not call Open when passed an already cancelled cancellation token source.Cancel(); @@ -90,14 +95,14 @@ public static void TestDbCommand() { Fail = true }; - commandFail.ExecuteNonQueryAsync().ContinueWith((t) => { }, TaskContinuationOptions.OnlyOnFaulted).Wait(); - commandFail.ExecuteNonQueryAsync(source.Token).ContinueWith((t) => { }, TaskContinuationOptions.OnlyOnFaulted).Wait(); - commandFail.ExecuteReaderAsync().ContinueWith((t) => { }, TaskContinuationOptions.OnlyOnFaulted).Wait(); - commandFail.ExecuteReaderAsync(CommandBehavior.SequentialAccess).ContinueWith((t) => { }, TaskContinuationOptions.OnlyOnFaulted).Wait(); - commandFail.ExecuteReaderAsync(source.Token).ContinueWith((t) => { }, TaskContinuationOptions.OnlyOnFaulted).Wait(); - commandFail.ExecuteReaderAsync(CommandBehavior.SequentialAccess, source.Token).ContinueWith((t) => { }, TaskContinuationOptions.OnlyOnFaulted).Wait(); - commandFail.ExecuteScalarAsync().ContinueWith((t) => { }, TaskContinuationOptions.OnlyOnFaulted).Wait(); - commandFail.ExecuteScalarAsync(source.Token).ContinueWith((t) => { }, TaskContinuationOptions.OnlyOnFaulted).Wait(); + AssertTaskFaults(commandFail.ExecuteNonQueryAsync()); + AssertTaskFaults(commandFail.ExecuteNonQueryAsync(source.Token)); + AssertTaskFaults(commandFail.ExecuteReaderAsync()); + AssertTaskFaults(commandFail.ExecuteReaderAsync(CommandBehavior.SequentialAccess)); + AssertTaskFaults(commandFail.ExecuteReaderAsync(source.Token)); + AssertTaskFaults(commandFail.ExecuteReaderAsync(CommandBehavior.SequentialAccess, source.Token)); + AssertTaskFaults(commandFail.ExecuteScalarAsync()); + AssertTaskFaults(commandFail.ExecuteScalarAsync(source.Token)); // Verify base implementation does not call Open when passed an already cancelled cancellation token source.Cancel(); @@ -155,9 +160,9 @@ public static void TestDbDataReader() GetFieldValueAsync(reader, 2, DBNull.Value); GetFieldValueAsync(reader, 2, DBNull.Value); - reader.GetFieldValueAsync(2).ContinueWith((t) => { }, TaskContinuationOptions.OnlyOnFaulted).Wait(); - reader.GetFieldValueAsync(2).ContinueWith((t) => { }, TaskContinuationOptions.OnlyOnFaulted).Wait(); - reader.GetFieldValueAsync(2).ContinueWith((t) => { }, TaskContinuationOptions.OnlyOnFaulted).Wait(); + AssertTaskFaults(reader.GetFieldValueAsync(2)); + AssertTaskFaults(reader.GetFieldValueAsync(2)); + AssertTaskFaults(reader.GetFieldValueAsync(2)); AssertEqualsWithDescription("GetValue", reader.LastCommand, "Last command was not as expected"); result = reader.ReadAsync(); @@ -174,12 +179,12 @@ public static void TestDbDataReader() Assert.False(result.Result, "Should NOT have received a Result from NextResultAsync"); MockDataReader readerFail = new MockDataReader { Results = query.GetEnumerator(), Fail = true }; - readerFail.ReadAsync().ContinueWith((t) => { }, TaskContinuationOptions.OnlyOnFaulted).Wait(); - readerFail.ReadAsync(source.Token).ContinueWith((t) => { }, TaskContinuationOptions.OnlyOnFaulted).Wait(); - readerFail.NextResultAsync().ContinueWith((t) => { }, TaskContinuationOptions.OnlyOnFaulted).Wait(); - readerFail.NextResultAsync(source.Token).ContinueWith((t) => { }, TaskContinuationOptions.OnlyOnFaulted).Wait(); - readerFail.GetFieldValueAsync(0).ContinueWith((t) => { }, TaskContinuationOptions.OnlyOnFaulted).Wait(); - readerFail.GetFieldValueAsync(0, source.Token).ContinueWith((t) => { }, TaskContinuationOptions.OnlyOnFaulted).Wait(); + AssertTaskFaults(readerFail.ReadAsync()); + AssertTaskFaults(readerFail.ReadAsync(source.Token)); + AssertTaskFaults(readerFail.NextResultAsync()); + AssertTaskFaults(readerFail.NextResultAsync(source.Token)); + AssertTaskFaults(readerFail.GetFieldValueAsync(0)); + AssertTaskFaults(readerFail.GetFieldValueAsync(0, source.Token)); source.Cancel(); reader.LastCommand = "Nothing"; From a2ea13627080505d673fe38b4988ce1922dbd492 Mon Sep 17 00:00:00 2001 From: Jack Mills Date: Sat, 15 Aug 2020 12:19:50 +0100 Subject: [PATCH 07/10] Leak even fewer unobserved exceptions --- .../BaseProviderAsyncTest/BaseProviderAsyncTest.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/BaseProviderAsyncTest/BaseProviderAsyncTest.cs b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/BaseProviderAsyncTest/BaseProviderAsyncTest.cs index 07e7461c7b..0d13ad5f77 100644 --- a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/BaseProviderAsyncTest/BaseProviderAsyncTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/BaseProviderAsyncTest/BaseProviderAsyncTest.cs @@ -121,17 +121,17 @@ public static void TestDbCommand() source = new CancellationTokenSource(); Task.Factory.StartNew(() => { command.WaitForWaitingForCancel(); source.Cancel(); }); Task result = command.ExecuteNonQueryAsync(source.Token); - Assert.True(result.IsFaulted, "Task result should be faulted"); + Assert.True(result.Exception != null, "Task result should be faulted"); source = new CancellationTokenSource(); Task.Factory.StartNew(() => { command.WaitForWaitingForCancel(); source.Cancel(); }); result = command.ExecuteReaderAsync(source.Token); - Assert.True(result.IsFaulted, "Task result should be faulted"); + Assert.True(result.Exception != null, "Task result should be faulted"); source = new CancellationTokenSource(); Task.Factory.StartNew(() => { command.WaitForWaitingForCancel(); source.Cancel(); }); result = command.ExecuteScalarAsync(source.Token); - Assert.True(result.IsFaulted, "Task result should be faulted"); + Assert.True(result.Exception != null, "Task result should be faulted"); } [Fact] From 08b54d4add856659dd4c62515ec979450a462589 Mon Sep 17 00:00:00 2001 From: Jack Mills Date: Tue, 1 Sep 2020 17:23:07 +0100 Subject: [PATCH 08/10] View the exception from the continuation --- .../netcore/src/Microsoft/Data/SqlClient/SqlUtil.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlUtil.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlUtil.cs index a1a297ed99..e54361bc31 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlUtil.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlUtil.cs @@ -206,7 +206,7 @@ internal static void WaitForCompletion(Task task, int timeout, Action onTimeout } if (!task.IsCompleted) { - task.ContinueWith(_ => { }); //Ensure the task does not leave an unobserved exception + task.ContinueWith(t => { var ignored = t.Exception; }); //Ensure the task does not leave an unobserved exception if (onTimeout != null) { onTimeout(); From f3aed3b5bcae8e25562c9a6405a79dae3e10954b Mon Sep 17 00:00:00 2001 From: Jack Mills Date: Tue, 1 Sep 2020 17:38:02 +0100 Subject: [PATCH 09/10] Observe the exception from _the other_ SqlUtil --- .../netfx/src/Microsoft/Data/SqlClient/SqlUtil.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlUtil.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlUtil.cs index 3929d1422f..d3c57d1122 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlUtil.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlUtil.cs @@ -191,7 +191,7 @@ internal static void WaitForCompletion(Task task, int timeout, Action onTimeout } if (!task.IsCompleted) { - task.ContinueWith(_ => { }); //Ensure the task does not leave an unobserved exception + task.ContinueWith(t => { var ignored = t.Exception; }); //Ensure the task does not leave an unobserved exception if (onTimeout != null) { onTimeout(); From 07ad6ec4e6ac5e25214c1f98c522d3658dcf1713 Mon Sep 17 00:00:00 2001 From: Jack Mills Date: Tue, 1 Sep 2020 17:52:45 +0100 Subject: [PATCH 10/10] Swap lambda to local function Co-authored-by: Cheena Malhotra --- .../tests/FunctionalTests/SqlHelperTest.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlHelperTest.cs b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlHelperTest.cs index 89b9228f7a..30152bd2c3 100644 --- a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlHelperTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlHelperTest.cs @@ -28,8 +28,8 @@ public void WaitForCompletion_DoesNotCreateUnobservedException() { var unobservedExceptionHappenedEvent = new AutoResetEvent(false); Exception unhandledException = null; - EventHandler handleUnobservedException = - (o, a) => { unhandledException = a.Exception; unobservedExceptionHappenedEvent.Set(); }; + void handleUnobservedException(object o, UnobservedTaskExceptionEventArgs a) + { unhandledException = a.Exception; unobservedExceptionHappenedEvent.Set(); } TaskScheduler.UnobservedTaskException += handleUnobservedException;