Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Patch TaskExtensions bug in Azure.Storage #14313

Merged
merged 3 commits into from
Aug 18, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions eng/Packages.Data.props
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@
<PackageReference Update="ApprovalUtilities" Version="3.0.22" />
<PackageReference Update="Azure.AI.TextAnalytics" Version="1.0.0" />
<PackageReference Update="Azure.Data.AppConfiguration" Version="1.0.0" />
<PackageReference Update="Azure.Core" Version="1.4.0" />
<PackageReference Update="Azure.Core.Experimental" Version="0.1.0-preview.3" />
<PackageReference Update="Azure.Identity" Version="1.1.1" />
<PackageReference Update="Azure.Core" Version="1.4.1" />
<PackageReference Update="Azure.Core.Experimental" Version="0.1.0-preview.4" />
<PackageReference Update="Azure.Identity" Version="1.1.2" />
<PackageReference Update="Azure.Security.KeyVault.Secrets" Version="4.0.1" />
<PackageReference Update="Azure.Security.KeyVault.Certificates" Version="4.0.0" />
<PackageReference Update="Azure.Security.KeyVault.Keys" Version="4.0.1" />
Expand Down
36 changes: 7 additions & 29 deletions sdk/core/Azure.Core/src/Shared/TaskExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,6 @@ public static T EnsureCompleted<T>(this Task<T> 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();
Expand All @@ -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();
Expand All @@ -53,31 +43,22 @@ public static void EnsureCompleted(this Task task)

public static T EnsureCompleted<T>(this ValueTask<T> 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.
}

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<T> EnsureSyncEnumerable<T>(this IAsyncEnumerable<T> asyncEnumerable) => new Enumerable<T>(asyncEnumerable);
Expand Down Expand Up @@ -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;

/// <summary>
/// Both <see cref="Enumerable{T}"/> and <see cref="Enumerator{T}"/> are defined as public structs so that foreach can use duck typing
/// to call <see cref="Enumerable{T}.GetEnumerator"/> and avoid heap memory allocation.
Expand Down
181 changes: 0 additions & 181 deletions sdk/core/Azure.Core/tests/TaskExtensionsTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,144 +4,13 @@
using Azure.Core.Pipeline;
using NUnit.Framework;
using System;
using System.Collections.Concurrent;
using System.Threading;
using System.Threading.Tasks;

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<int>(42);
Assert.AreEqual(42, task.EnsureCompleted());
}

[Test]
public async Task TaskExtensions_TaskEnsureCompleted_NotCompletedNoSyncContext()
{
var tcs = new TaskCompletionSource<int>();
Task task = tcs.Task;
#if DEBUG
Assert.Catch<InvalidOperationException>(() => 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<int>();
#if DEBUG
Assert.Catch<InvalidOperationException>(() => tcs.Task.EnsureCompleted());
await Task.CompletedTask;
#else
Task<int> 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<int>();
ValueTask task = new ValueTask(tcs.Task);
#if DEBUG
Assert.Catch<InvalidOperationException>(() => 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<int>();
ValueTask<int> task = new ValueTask<int>(tcs.Task);
#if DEBUG
Assert.Catch<InvalidOperationException>(() => task.EnsureCompleted());
await Task.CompletedTask;
#else
Task<int> 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<int>();
Task task = tcs.Task;

syncContext.Post(t => { Assert.Catch<InvalidOperationException>(() => task.EnsureCompleted()); }, null);
}

[Test]
public void TaskExtensions_TaskOfTEnsureCompleted_NotCompletedInSyncContext()
{
using SingleThreadedSynchronizationContext syncContext = new SingleThreadedSynchronizationContext();
var tcs = new TaskCompletionSource<int>();

syncContext.Post(t => { Assert.Catch<InvalidOperationException>(() => tcs.Task.EnsureCompleted()); }, null);
}

[Test]
public void TaskExtensions_ValueTaskEnsureCompleted_NotCompletedInSyncContext()
{
using SingleThreadedSynchronizationContext syncContext = new SingleThreadedSynchronizationContext();
var tcs = new TaskCompletionSource<int>();
ValueTask task = new ValueTask(tcs.Task);

syncContext.Post(t => { Assert.Catch<InvalidOperationException>(() => task.EnsureCompleted()); }, null);
}

[Test]
public void TaskExtensions_ValueTaskOfTEnsureCompleted_NotCompletedInSyncContext()
{
using SingleThreadedSynchronizationContext syncContext = new SingleThreadedSynchronizationContext();
var tcs = new TaskCompletionSource<int>();
var task = new ValueTask<int>(tcs.Task);

syncContext.Post(t => { Assert.Catch<InvalidOperationException>(() => task.EnsureCompleted()); }, null);
}

[Test]
public void TaskExtensions_TaskWithCancellationDefault()
{
Expand Down Expand Up @@ -323,55 +192,5 @@ public void TaskExtensions_ValueTaskWithCancellationFailedAfterContinuationSched
Assert.AreEqual(true, awaiter.IsCompleted);
Assert.Catch<OperationCanceledException>(() => awaiter.GetResult(), "Error");
}

private sealed class SingleThreadedSynchronizationContext : SynchronizationContext, IDisposable
{
private readonly Task _task;
private readonly BlockingCollection<Action> _queue;
private readonly ConcurrentQueue<Exception> _exceptions;

public SingleThreadedSynchronizationContext()
{
_queue = new BlockingCollection<Action>();
_exceptions = new ConcurrentQueue<Exception>();
_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);
}
}
}
3 changes: 3 additions & 0 deletions sdk/storage/Azure.Storage.Blobs.Batch/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# Release History

## 12.3.1 (2020-08-18)
- Bug in TaskExtensions.EnsureCompleted method that causes it to unconditionally throw an exception in the environments with synchronization context

## 12.3.0 (2020-08-13)
- This release contains bug fixes to improve quality.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
</PropertyGroup>
<PropertyGroup>
<AssemblyTitle>Microsoft Azure.Storage.Blobs.Batch client library</AssemblyTitle>
<Version>12.3.0</Version>
<Version>12.3.1</Version>
<ApiCompatVersion>12.2.1</ApiCompatVersion>
<DefineConstants>BlobSDK;$(DefineConstants)</DefineConstants>
<PackageTags>Microsoft Azure Storage Blobs Batching;Batch blob;Batch operation;BlobBatchClient;BlobBatch;Microsoft;Azure;Blobs;Blob;Storage;StorageScalable;$(PackageCommonTags)</PackageTags>
Expand Down
3 changes: 3 additions & 0 deletions sdk/storage/Azure.Storage.Blobs.ChangeFeed/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# Release History

## 12.0.0-preview.4 (2020-08-18)
- Bug in TaskExtensions.EnsureCompleted method that causes it to unconditionally throw an exception in the environments with synchronization context

## 12.0.0-preview.3 (2020-08-13)
- This release contains bug fixes to improve quality.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
</PropertyGroup>
<PropertyGroup>
<AssemblyTitle>Microsoft Azure.Storage.Blobs.ChangeFeed client library</AssemblyTitle>
<Version>12.0.0-preview.3</Version>
<Version>12.0.0-preview.4</Version>
<DefineConstants>ChangeFeedSDK;$(DefineConstants)</DefineConstants>
<PackageTags>Microsoft Azure Change Feed;Microsoft;Azure;Storage;StorageScalable;$(PackageCommonTags)</PackageTags>
<Description>
Expand Down
3 changes: 3 additions & 0 deletions sdk/storage/Azure.Storage.Blobs/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# Release History

## 12.5.1 (2020-08-18)
- Bug in TaskExtensions.EnsureCompleted method that causes it to unconditionally throw an exception in the environments with synchronization context

## 12.5.0 (2020-08-13)
- Added support for custom local emulator hostname for blob storage endpoints.
- Fixed bug where BlobContainerClient.SetAccessPolicy() sends DateTimeOffset.MinValue when StartsOn and ExpiresOn when not set in BlobAccessPolicy
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
</PropertyGroup>
<PropertyGroup>
<AssemblyTitle>Microsoft Azure.Storage.Blobs client library</AssemblyTitle>
<Version>12.5.0</Version>
<Version>12.5.1</Version>
<ApiCompatVersion>12.4.4</ApiCompatVersion>
<DefineConstants>BlobSDK;$(DefineConstants)</DefineConstants>
<PackageTags>Microsoft Azure Storage Blobs;Microsoft;Azure;Blobs;Blob;Storage;StorageScalable;$(PackageCommonTags)</PackageTags>
Expand Down
3 changes: 3 additions & 0 deletions sdk/storage/Azure.Storage.Common/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# Release History

## 12.5.1 (2020-08-18)
- Bug in TaskExtensions.EnsureCompleted method that causes it to unconditionally throw an exception in the environments with synchronization context

## 12.5.0 (2020-08-13)
- This release contains bug fixes to improve quality.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
</PropertyGroup>
<PropertyGroup>
<AssemblyTitle>Microsoft Azure.Storage.Common client library</AssemblyTitle>
<Version>12.5.0</Version>
<Version>12.5.1</Version>
<ApiCompatVersion>12.4.3</ApiCompatVersion>
<DefineConstants>CommonSDK;$(DefineConstants)</DefineConstants>
<PackageTags>Microsoft Azure Storage Common, Microsoft, Azure, StorageScalable, azureofficial</PackageTags>
Expand Down
3 changes: 3 additions & 0 deletions sdk/storage/Azure.Storage.Files.DataLake/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# Release History

## 12.3.1 (2020-08-18)
- Bug in TaskExtensions.EnsureCompleted method that causes it to unconditionally throw an exception in the environments with synchronization context

## 12.3.0 (2020-08-13)
- Fixed bug where DataLakeFileSystemClient.SetAccessPolicy() sends DateTimeOffset.MinValue when StartsOn and ExpiresOn when not set in DataLakeAccessPolicy
- Added nullable properties, PolicyStartsOn and PolicyExpiresOn to DataLakeAccessPolicy
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
</PropertyGroup>
<PropertyGroup>
<AssemblyTitle>Microsoft Azure.Storage.Files.DataLake client library</AssemblyTitle>
<Version>12.3.0</Version>
<Version>12.3.1</Version>
<ApiCompatVersion>12.2.2</ApiCompatVersion>
<DefineConstants>DataLakeSDK;$(DefineConstants)</DefineConstants>
<PackageTags>Microsoft Azure Storage Files;Microsoft;Azure;File;Files;Data Lake;Storage;StorageScalable;$(PackageCommonTags)</PackageTags>
Expand Down
3 changes: 3 additions & 0 deletions sdk/storage/Azure.Storage.Files.Shares/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# Release History

## 12.3.1 (2020-08-18)
- Bug in TaskExtensions.EnsureCompleted method that causes it to unconditionally throw an exception in the environments with synchronization context

## 12.3.0 (2020-08-13)
- Fixed bug where ShareClient.SetAccessPolicy() sends DateTimeOffset.MinValue when StartsOn and ExpiresOn when not set in ShareAccessPolicy
- Added nullable properties, PolicyStartsOn and PolicyExpiresOn to ShareAccessPolicy
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
</PropertyGroup>
<PropertyGroup>
<AssemblyTitle>Microsoft Azure.Storage.Files.Shares client library</AssemblyTitle>
<Version>12.3.0</Version>
<Version>12.3.1</Version>
<ApiCompatVersion>12.2.3</ApiCompatVersion>
<DefineConstants>FileSDK;$(DefineConstants)</DefineConstants>
<PackageTags>Microsoft Azure Storage Files;Microsoft;Azure;File;Files;Storage;StorageScalable;$(PackageCommonTags)</PackageTags>
Expand Down
Loading