From d85f836b582407b9af0a0d93430e55fe25a192b3 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Tue, 4 Aug 2020 14:56:50 +0200 Subject: [PATCH 1/2] Improved reliability of Task.GetResultOrDefault --- .../Extensions/TaskExtensions.cs | 30 +++++++------------ 1 file changed, 11 insertions(+), 19 deletions(-) diff --git a/Microsoft.Toolkit/Extensions/TaskExtensions.cs b/Microsoft.Toolkit/Extensions/TaskExtensions.cs index 9254f3c6290..204a74fe1c7 100644 --- a/Microsoft.Toolkit/Extensions/TaskExtensions.cs +++ b/Microsoft.Toolkit/Extensions/TaskExtensions.cs @@ -2,7 +2,6 @@ // 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.Diagnostics.CodeAnalysis; using System.Diagnostics.Contracts; using System.Reflection; @@ -41,28 +40,21 @@ public static class TaskExtensions #endif ) { - Type taskType = task.GetType(); - - // Check if the task is actually some Task - if ( -#if NETSTANDARD1_4 - taskType.GetTypeInfo().IsGenericType && -#else - taskType.IsGenericType && -#endif - taskType.GetGenericTypeDefinition() == typeof(Task<>)) - { - // Get the Task.Result property - PropertyInfo propertyInfo = + // Try to get the Task.Result property. This method would've + // been called anyway after the type checks, but using that to + // validate the input type saves some additional reflection calls. + // Furthermore, doing this also makes the method flexible enough to + // cases whether the input Task is actually an instance of some + // runtime-specific type that inherits from Task. + PropertyInfo? propertyInfo = #if NETSTANDARD1_4 - taskType.GetRuntimeProperty(nameof(Task.Result)); + task.GetType().GetRuntimeProperty(nameof(Task.Result)); #else - taskType.GetProperty(nameof(Task.Result)); + task.GetType().GetProperty(nameof(Task.Result)); #endif - // Finally retrieve the result - return propertyInfo!.GetValue(task); - } + // Return the result, if possible + return propertyInfo?.GetValue(task); } return null; From e469f9e23f012a7b790ada106f4df517aed4d226 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Wed, 5 Aug 2020 00:22:38 +0200 Subject: [PATCH 2/2] Added unit test for AsyncTaskMethodBuilder --- .../Extensions/Test_TaskExtensions.cs | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/UnitTests/UnitTests.Shared/Extensions/Test_TaskExtensions.cs b/UnitTests/UnitTests.Shared/Extensions/Test_TaskExtensions.cs index 6754f5214a6..8e1d529186e 100644 --- a/UnitTests/UnitTests.Shared/Extensions/Test_TaskExtensions.cs +++ b/UnitTests/UnitTests.Shared/Extensions/Test_TaskExtensions.cs @@ -37,6 +37,25 @@ public void Test_TaskExtensions_ResultOrDefault() Assert.AreEqual(42, ((Task)tcs.Task).GetResultOrDefault()); } + [TestCategory("TaskExtensions")] + [TestMethod] + public async Task Test_TaskExtensions_ResultOrDefault_FromAsyncTaskMethodBuilder() + { + var tcs = new TaskCompletionSource(); + + Task taskFromBuilder = GetTaskFromAsyncMethodBuilder("Test", tcs); + + Assert.IsNull(((Task)taskFromBuilder).GetResultOrDefault()); + Assert.IsNull(taskFromBuilder.GetResultOrDefault()); + + tcs.SetResult(null); + + await taskFromBuilder; + + Assert.AreEqual(((Task)taskFromBuilder).GetResultOrDefault(), "Test"); + Assert.AreEqual(taskFromBuilder.GetResultOrDefault(), "Test"); + } + [TestCategory("TaskExtensions")] [TestMethod] public void Test_TaskExtensions_ResultOrDefault_OfT_Int32() @@ -86,5 +105,16 @@ public void Test_TaskExtensions_ResultOrDefault_OfT_String() Assert.AreEqual("Hello world", tcs.Task.GetResultOrDefault()); } + + // Creates a Task of a given type which is actually an instance of + // System.Runtime.CompilerServices.AsyncTaskMethodBuilder.AsyncStateMachineBox. + // See https://source.dot.net/#System.Private.CoreLib/AsyncTaskMethodBuilderT.cs,f8f35fd356112b30. + // This is needed to verify that the extension also works when the input Task is of a derived type. + private static async Task GetTaskFromAsyncMethodBuilder(T result, TaskCompletionSource tcs) + { + await tcs.Task; + + return result; + } } }