From 51477a8a741985488372caaec5aeeef23f168e57 Mon Sep 17 00:00:00 2001 From: Alexander Sher Date: Tue, 18 Aug 2020 11:07:17 -0500 Subject: [PATCH 1/4] Revert "Fix #10904: Task extensions synchronously complete ValueTasks (#13719)" This reverts commit 49bcc3e63c8953cec6ab10cd6a552799abbba13d. --- .../Azure.Core/src/Shared/TaskExtensions.cs | 36 +--- .../Azure.Core/tests/TaskExtensionsTest.cs | 181 ------------------ 2 files changed, 7 insertions(+), 210 deletions(-) diff --git a/sdk/core/Azure.Core/src/Shared/TaskExtensions.cs b/sdk/core/Azure.Core/src/Shared/TaskExtensions.cs index 61b215054ec0..a22738192f8b 100644 --- a/sdk/core/Azure.Core/src/Shared/TaskExtensions.cs +++ b/sdk/core/Azure.Core/src/Shared/TaskExtensions.cs @@ -25,11 +25,6 @@ public static T EnsureCompleted(this Task task) { #if DEBUG VerifyTaskCompleted(task.IsCompleted); -#else - if (HasSynchronizationContext()) - { - throw new InvalidOperationException("Synchronously waiting on non-completed task isn't allowed."); - } #endif #pragma warning disable AZC0102 // Do not use GetAwaiter().GetResult(). Use the TaskExtensions.EnsureCompleted() extension method instead. return task.GetAwaiter().GetResult(); @@ -40,11 +35,6 @@ public static void EnsureCompleted(this Task task) { #if DEBUG VerifyTaskCompleted(task.IsCompleted); -#else - if (HasSynchronizationContext()) - { - throw new InvalidOperationException("Synchronously waiting on non-completed task isn't allowed."); - } #endif #pragma warning disable AZC0102 // Do not use GetAwaiter().GetResult(). Use the TaskExtensions.EnsureCompleted() extension method instead. task.GetAwaiter().GetResult(); @@ -53,12 +43,9 @@ public static void EnsureCompleted(this Task task) public static T EnsureCompleted(this ValueTask task) { - if (!task.IsCompleted) - { -#pragma warning disable AZC0107 // public asynchronous method shouldn't be called in synchronous scope. Use synchronous version of the method if it is available. - return EnsureCompleted(task.AsTask()); -#pragma warning restore AZC0107 // public asynchronous method shouldn't be called in synchronous scope. Use synchronous version of the method if it is available. - } +#if DEBUG + VerifyTaskCompleted(task.IsCompleted); +#endif #pragma warning disable AZC0102 // Do not use GetAwaiter().GetResult(). Use the TaskExtensions.EnsureCompleted() extension method instead. return task.GetAwaiter().GetResult(); #pragma warning restore AZC0102 // Do not use GetAwaiter().GetResult(). Use the TaskExtensions.EnsureCompleted() extension method instead. @@ -66,18 +53,12 @@ public static T EnsureCompleted(this ValueTask task) public static void EnsureCompleted(this ValueTask task) { - if (!task.IsCompleted) - { -#pragma warning disable AZC0107 // public asynchronous method shouldn't be called in synchronous scope. Use synchronous version of the method if it is available. - EnsureCompleted(task.AsTask()); -#pragma warning restore AZC0107 // public asynchronous method shouldn't be called in synchronous scope. Use synchronous version of the method if it is available. - } - else - { +#if DEBUG + VerifyTaskCompleted(task.IsCompleted); +#endif #pragma warning disable AZC0102 // Do not use GetAwaiter().GetResult(). Use the TaskExtensions.EnsureCompleted() extension method instead. - task.GetAwaiter().GetResult(); + task.GetAwaiter().GetResult(); #pragma warning restore AZC0102 // Do not use GetAwaiter().GetResult(). Use the TaskExtensions.EnsureCompleted() extension method instead. - } } public static Enumerable EnsureSyncEnumerable(this IAsyncEnumerable asyncEnumerable) => new Enumerable(asyncEnumerable); @@ -120,9 +101,6 @@ private static void VerifyTaskCompleted(bool isCompleted) } } - private static bool HasSynchronizationContext() - => SynchronizationContext.Current != null && SynchronizationContext.Current.GetType() != typeof(SynchronizationContext) || TaskScheduler.Current != TaskScheduler.Default; - /// /// Both and are defined as public structs so that foreach can use duck typing /// to call and avoid heap memory allocation. diff --git a/sdk/core/Azure.Core/tests/TaskExtensionsTest.cs b/sdk/core/Azure.Core/tests/TaskExtensionsTest.cs index 9fe2dd399949..4ce2839ef361 100644 --- a/sdk/core/Azure.Core/tests/TaskExtensionsTest.cs +++ b/sdk/core/Azure.Core/tests/TaskExtensionsTest.cs @@ -4,7 +4,6 @@ using Azure.Core.Pipeline; using NUnit.Framework; using System; -using System.Collections.Concurrent; using System.Threading; using System.Threading.Tasks; @@ -12,136 +11,6 @@ namespace Azure.Core.Tests { public class TaskExtensionsTest { - [Test] - public void TaskExtensions_TaskEnsureCompleted() - { - var task = Task.CompletedTask; - task.EnsureCompleted(); - } - - [Test] - public void TaskExtensions_TaskOfTEnsureCompleted() - { - var task = Task.FromResult(42); - Assert.AreEqual(42, task.EnsureCompleted()); - } - - [Test] - public void TaskExtensions_ValueTaskEnsureCompleted() - { - var task = new ValueTask(); - task.EnsureCompleted(); - } - - [Test] - public void TaskExtensions_ValueTaskOfTEnsureCompleted() - { - var task = new ValueTask(42); - Assert.AreEqual(42, task.EnsureCompleted()); - } - - [Test] - public async Task TaskExtensions_TaskEnsureCompleted_NotCompletedNoSyncContext() - { - var tcs = new TaskCompletionSource(); - Task task = tcs.Task; -#if DEBUG - Assert.Catch(() => task.EnsureCompleted()); - await Task.CompletedTask; -#else - Task runningTask = Task.Run(() => task.EnsureCompleted()); - Assert.IsFalse(runningTask.IsCompleted); - tcs.SetResult(0); - await runningTask; -#endif - } - - [Test] - public async Task TaskExtensions_TaskOfTEnsureCompleted_NotCompletedNoSyncContext() - { - var tcs = new TaskCompletionSource(); -#if DEBUG - Assert.Catch(() => tcs.Task.EnsureCompleted()); - await Task.CompletedTask; -#else - Task runningTask = Task.Run(() => tcs.Task.EnsureCompleted()); - Assert.IsFalse(runningTask.IsCompleted); - tcs.SetResult(42); - Assert.AreEqual(42, await runningTask); -#endif - } - - [Test] - public async Task TaskExtensions_ValueTaskEnsureCompleted_NotCompletedNoSyncContext() - { - var tcs = new TaskCompletionSource(); - ValueTask task = new ValueTask(tcs.Task); -#if DEBUG - Assert.Catch(() => task.EnsureCompleted()); - await Task.CompletedTask; -#else - Task runningTask = Task.Run(() => task.EnsureCompleted()); - Assert.IsFalse(runningTask.IsCompleted); - tcs.SetResult(0); - await runningTask; -#endif - } - - [Test] - public async Task TaskExtensions_ValueTaskOfTEnsureCompleted_NotCompletedNoSyncContext() - { - var tcs = new TaskCompletionSource(); - ValueTask task = new ValueTask(tcs.Task); -#if DEBUG - Assert.Catch(() => task.EnsureCompleted()); - await Task.CompletedTask; -#else - Task runningTask = Task.Run(() => task.EnsureCompleted()); - Assert.IsFalse(runningTask.IsCompleted); - tcs.SetResult(42); - Assert.AreEqual(42, await runningTask); -#endif - } - - [Test] - public void TaskExtensions_TaskEnsureCompleted_NotCompletedInSyncContext() - { - using SingleThreadedSynchronizationContext syncContext = new SingleThreadedSynchronizationContext(); - var tcs = new TaskCompletionSource(); - Task task = tcs.Task; - - syncContext.Post(t => { Assert.Catch(() => task.EnsureCompleted()); }, null); - } - - [Test] - public void TaskExtensions_TaskOfTEnsureCompleted_NotCompletedInSyncContext() - { - using SingleThreadedSynchronizationContext syncContext = new SingleThreadedSynchronizationContext(); - var tcs = new TaskCompletionSource(); - - syncContext.Post(t => { Assert.Catch(() => tcs.Task.EnsureCompleted()); }, null); - } - - [Test] - public void TaskExtensions_ValueTaskEnsureCompleted_NotCompletedInSyncContext() - { - using SingleThreadedSynchronizationContext syncContext = new SingleThreadedSynchronizationContext(); - var tcs = new TaskCompletionSource(); - ValueTask task = new ValueTask(tcs.Task); - - syncContext.Post(t => { Assert.Catch(() => task.EnsureCompleted()); }, null); - } - - [Test] - public void TaskExtensions_ValueTaskOfTEnsureCompleted_NotCompletedInSyncContext() - { - using SingleThreadedSynchronizationContext syncContext = new SingleThreadedSynchronizationContext(); - var tcs = new TaskCompletionSource(); - var task = new ValueTask(tcs.Task); - - syncContext.Post(t => { Assert.Catch(() => task.EnsureCompleted()); }, null); - } - [Test] public void TaskExtensions_TaskWithCancellationDefault() { @@ -323,55 +192,5 @@ public void TaskExtensions_ValueTaskWithCancellationFailedAfterContinuationSched Assert.AreEqual(true, awaiter.IsCompleted); Assert.Catch(() => awaiter.GetResult(), "Error"); } - - private sealed class SingleThreadedSynchronizationContext : SynchronizationContext, IDisposable - { - private readonly Task _task; - private readonly BlockingCollection _queue; - private readonly ConcurrentQueue _exceptions; - - public SingleThreadedSynchronizationContext() - { - _queue = new BlockingCollection(); - _exceptions = new ConcurrentQueue(); - _task = Task.Run(RunLoop); - } - - private void RunLoop() - { - try - { - SetSynchronizationContext(this); - while (!_queue.IsCompleted) - { - Action action = _queue.Take(); - try - { - action(); - } - catch (Exception e) - { - _exceptions.Enqueue(e); - } - } - } - catch (InvalidOperationException) { } - catch (OperationCanceledException) { } - finally - { - SetSynchronizationContext(null); - } - } - - public override void Post(SendOrPostCallback d, object state) => _queue.Add(() => d(state)); - - public void Dispose() - { - _queue.CompleteAdding(); - _task.Wait(); - } - - public AggregateException Exceptions => new AggregateException(_exceptions); - } } } From 9b6c21a8bdc30c16d9647c636eeba2b202056780 Mon Sep 17 00:00:00 2001 From: Alexander Sher Date: Tue, 18 Aug 2020 11:17:56 -0500 Subject: [PATCH 2/4] Fix changelog and csproj --- sdk/core/Azure.Core/CHANGELOG.md | 4 +++- sdk/core/Azure.Core/src/Azure.Core.csproj | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/sdk/core/Azure.Core/CHANGELOG.md b/sdk/core/Azure.Core/CHANGELOG.md index e22272b858e4..02cbdbcd10a1 100644 --- a/sdk/core/Azure.Core/CHANGELOG.md +++ b/sdk/core/Azure.Core/CHANGELOG.md @@ -1,7 +1,9 @@ # Release History -## 1.5.0-preview.1 (Unreleased) +## 1.4.1 (2020-08-18) +### Fixed +- Bug in TaskExtensions.EnsureCompleted method that causes it to unconditionally throw an exception in the environments with synchronization context ## 1.4.0 (2020-08-06) diff --git a/sdk/core/Azure.Core/src/Azure.Core.csproj b/sdk/core/Azure.Core/src/Azure.Core.csproj index f8460d0de7f9..ee815ce7929d 100644 --- a/sdk/core/Azure.Core/src/Azure.Core.csproj +++ b/sdk/core/Azure.Core/src/Azure.Core.csproj @@ -2,7 +2,7 @@ This is the implementation of the Azure Client Pipeline Microsoft Azure Client Pipeline - 1.5.0-preview.1 + 1.4.1 1.4.0 Microsoft Azure Client Pipeline enable From 1f11d0c756987a827c0c0c6bbdf10a0b766b6029 Mon Sep 17 00:00:00 2001 From: Alexander Sher Date: Tue, 18 Aug 2020 11:21:17 -0500 Subject: [PATCH 3/4] Revert csproj change --- sdk/core/Azure.Core/src/Azure.Core.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/core/Azure.Core/src/Azure.Core.csproj b/sdk/core/Azure.Core/src/Azure.Core.csproj index ee815ce7929d..f8460d0de7f9 100644 --- a/sdk/core/Azure.Core/src/Azure.Core.csproj +++ b/sdk/core/Azure.Core/src/Azure.Core.csproj @@ -2,7 +2,7 @@ This is the implementation of the Azure Client Pipeline Microsoft Azure Client Pipeline - 1.4.1 + 1.5.0-preview.1 1.4.0 Microsoft Azure Client Pipeline enable From 1f3598e64684387d05e77d49f78c8024aa028369 Mon Sep 17 00:00:00 2001 From: Alexander Sher Date: Tue, 18 Aug 2020 12:13:31 -0500 Subject: [PATCH 4/4] Update changelog --- sdk/core/Azure.Core/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/core/Azure.Core/CHANGELOG.md b/sdk/core/Azure.Core/CHANGELOG.md index 02cbdbcd10a1..1191cd887611 100644 --- a/sdk/core/Azure.Core/CHANGELOG.md +++ b/sdk/core/Azure.Core/CHANGELOG.md @@ -1,6 +1,6 @@ # Release History -## 1.4.1 (2020-08-18) +## 1.5.0-preview.1 (Unreleased) ### Fixed - Bug in TaskExtensions.EnsureCompleted method that causes it to unconditionally throw an exception in the environments with synchronization context