From dad4f5012e6ec9fb45537d607b9e4a0ed8f17203 Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Thu, 6 Feb 2020 05:32:48 -0800 Subject: [PATCH] Enable VSTHRD103 (Call async methods when in an async method) --- .editorconfig | 3 --- .../Utilities/ResourceManagerUtils.cs | 4 ++-- .../Utilities/TaskExtensions.cs | 20 +++++++++++++++++++ .../Microsoft.ML.Sweeper.Tests/TestSweeper.cs | 14 ++++++------- 4 files changed, 29 insertions(+), 12 deletions(-) create mode 100644 src/Microsoft.ML.Core/Utilities/TaskExtensions.cs diff --git a/.editorconfig b/.editorconfig index 6b5fc14e6e..0ce9d216c3 100644 --- a/.editorconfig +++ b/.editorconfig @@ -7,9 +7,6 @@ dotnet_sort_system_directives_first = true # VSTHRD002: Avoid problematic synchronous waits dotnet_diagnostic.VSTHRD002.severity = none -# VSTHRD103: Call async methods when in an async method -dotnet_diagnostic.VSTHRD103.severity = none - # VSTHRD200: Use "Async" suffix for async methods dotnet_diagnostic.VSTHRD200.severity = none diff --git a/src/Microsoft.ML.Core/Utilities/ResourceManagerUtils.cs b/src/Microsoft.ML.Core/Utilities/ResourceManagerUtils.cs index 046880b922..f731b12e91 100644 --- a/src/Microsoft.ML.Core/Utilities/ResourceManagerUtils.cs +++ b/src/Microsoft.ML.Core/Utilities/ResourceManagerUtils.cs @@ -154,11 +154,11 @@ private async Task DownloadFromUrl(IHostEnvironment env, IChannel ch, st var timeoutTask = Task.Delay(timeout).ContinueWith(task => default(Exception), TaskScheduler.Default); ch.Info($"Downloading {fileName} from {url} to {filePath}"); var completedTask = await Task.WhenAny(t, timeoutTask); - if (completedTask != t || completedTask.Result != null) + if (completedTask != t || completedTask.CompletedResult() != null) { downloadCancel.Cancel(); deleteNeeded = true; - return t.Result.Message; + return (await t).Message; } return CheckValidDownload(ch, filePath, url, ref deleteNeeded); diff --git a/src/Microsoft.ML.Core/Utilities/TaskExtensions.cs b/src/Microsoft.ML.Core/Utilities/TaskExtensions.cs new file mode 100644 index 0000000000..eba5cdb0d7 --- /dev/null +++ b/src/Microsoft.ML.Core/Utilities/TaskExtensions.cs @@ -0,0 +1,20 @@ +// 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.Diagnostics.CodeAnalysis; +using System.Threading.Tasks; +using Microsoft.ML.Runtime; + +namespace Microsoft.ML.Internal.Utilities +{ + internal static class TaskExtensions + { + [SuppressMessage("Usage", "VSTHRD002:Avoid problematic synchronous waits", Justification = "The task is completed.")] + public static TResult CompletedResult(this Task task) + { + Contracts.Check(task.IsCompleted); + return task.Result; + } + } +} diff --git a/test/Microsoft.ML.Sweeper.Tests/TestSweeper.cs b/test/Microsoft.ML.Sweeper.Tests/TestSweeper.cs index c1b20cd6bb..d0c116a80b 100644 --- a/test/Microsoft.ML.Sweeper.Tests/TestSweeper.cs +++ b/test/Microsoft.ML.Sweeper.Tests/TestSweeper.cs @@ -206,7 +206,7 @@ public async Task TestDeterministicSweeperAsyncCancellation() if (i < args.BatchSize - args.Relaxation) { Assert.True(task.IsCompleted); - sweeper.Update(task.Result.Id, new RunResult(task.Result.ParameterSet, random.NextDouble(), true)); + sweeper.Update(task.CompletedResult().Id, new RunResult(task.CompletedResult().ParameterSet, random.NextDouble(), true)); numCompleted++; } else @@ -218,7 +218,7 @@ public async Task TestDeterministicSweeperAsyncCancellation() await Task.WhenAll(tasks); foreach (var task in tasks) { - if (task.Result != null) + if (task.CompletedResult() != null) numCompleted++; } Assert.Equal(args.BatchSize + args.BatchSize, numCompleted); @@ -254,9 +254,9 @@ public async Task TestDeterministicSweeperAsync() { var task = sweeper.Propose(); Assert.True(task.IsCompleted); - paramSets.Add(task.Result.ParameterSet); - var result = new RunResult(task.Result.ParameterSet, random.NextDouble(), true); - sweeper.Update(task.Result.Id, result); + paramSets.Add(task.CompletedResult().ParameterSet); + var result = new RunResult(task.CompletedResult().ParameterSet, random.NextDouble(), true); + sweeper.Update(task.CompletedResult().Id, result); } Assert.Equal(sweeps, paramSets.Count); CheckAsyncSweeperResult(paramSets); @@ -273,9 +273,9 @@ public async Task TestDeterministicSweeperAsync() var task = sweeper.Propose(); Assert.True(task.IsCompleted); tasks[i] = task; - if (task.Result == null) + if (task.CompletedResult() == null) continue; - results.Add(new KeyValuePair(task.Result.Id, new RunResult(task.Result.ParameterSet, 0.42, true))); + results.Add(new KeyValuePair(task.CompletedResult().Id, new RunResult(task.CompletedResult().ParameterSet, 0.42, true))); } // Register consumers for the 2nd batch. Those consumers will await until at least one run // in the previous batch has been posted to the sweeper.