From 0918664bc9a218f860a2acd9e9a436e31c661796 Mon Sep 17 00:00:00 2001 From: Chris Sienkiewicz Date: Thu, 13 Jan 2022 11:17:31 -0800 Subject: [PATCH 1/9] Filter cancellation exceptions in generator driver --- .../SyntaxAwareGeneratorTests.cs | 31 +++++++++++++++++++ .../InternalUtilities/ExceptionUtilities.cs | 22 +++++++++++++ .../Portable/InternalUtilities/FatalError.cs | 10 +++--- .../SourceGeneration/GeneratorDriver.cs | 5 +-- .../Nodes/SyntaxReceiverInputNode.cs | 2 +- .../Portable/SourceGeneration/UserFunction.cs | 14 +++------ 6 files changed, 63 insertions(+), 21 deletions(-) diff --git a/src/Compilers/CSharp/Test/Semantic/SourceGeneration/SyntaxAwareGeneratorTests.cs b/src/Compilers/CSharp/Test/Semantic/SourceGeneration/SyntaxAwareGeneratorTests.cs index ce8de0cfc2d46..9663e7d30622f 100644 --- a/src/Compilers/CSharp/Test/Semantic/SourceGeneration/SyntaxAwareGeneratorTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/SourceGeneration/SyntaxAwareGeneratorTests.cs @@ -1960,6 +1960,37 @@ class C { } Assert.True(generatorCancelled); } + [Fact] + public void Syntax_Receiver_Cancellation_During_Visit() + { + var source = @" +class C +{ + int Property { get; set; } + + void Function() + { + var x = 5; + x += 4; + } +} +"; + var parseOptions = TestOptions.Regular; + Compilation compilation = CreateCompilation(source, options: TestOptions.DebugDll, parseOptions: parseOptions); + compilation.VerifyDiagnostics(); + + Assert.Single(compilation.SyntaxTrees); + + var cts = new CancellationTokenSource(); + var testGenerator = new CallbackGenerator( + onInit: (i) => i.RegisterForSyntaxNotifications(() => new TestSyntaxReceiver(tag: 0, callback: (a) => { if (a is AssignmentExpressionSyntax){ cts.Cancel(); cts.Token.ThrowIfCancellationRequested(); } })), + onExecute: (e) => { e.AddSource("test", SourceText.From("public class D{}", Encoding.UTF8)); } + ); + + GeneratorDriver driver = CSharpGeneratorDriver.Create(new[] { testGenerator }, parseOptions: parseOptions); + Assert.Throws(() => driver.RunGeneratorsAndUpdateCompilation(compilation, out var outputCompilation, out var outputDiagnostics, cts.Token)); + } + private class TestReceiverBase { private readonly Action? _callback; diff --git a/src/Compilers/Core/Portable/InternalUtilities/ExceptionUtilities.cs b/src/Compilers/Core/Portable/InternalUtilities/ExceptionUtilities.cs index 8e3dc303ca8f3..1f0db0af93366 100644 --- a/src/Compilers/Core/Portable/InternalUtilities/ExceptionUtilities.cs +++ b/src/Compilers/Core/Portable/InternalUtilities/ExceptionUtilities.cs @@ -4,6 +4,7 @@ using System; using System.Diagnostics; +using System.Threading; namespace Roslyn.Utilities { @@ -27,5 +28,26 @@ internal static Exception Unreachable { get { return new InvalidOperationException("This program location is thought to be unreachable."); } } + + /// + /// Determine if an exception was not an , and optionally check that the provided token was cancelled. + /// + /// + /// Used in exception filters to determine if the exception should be handled or not. + /// + /// The exception to test. + /// Optional. Checked to see if the cancellation was caused by the provided token and considered not cancelled if not. + /// if the exception should be handled by the caller, or if the exception was caused by cancellation. + internal static bool IsNotCancelled(Exception exception, CancellationToken? cancellationToken = null) + => !IsCurrentOperationBeingCancelled(exception, cancellationToken ?? new CancellationToken(true)); + + /// + /// Determine if an exception was an , and that the token was cancelled. + /// + /// The exception to test. + /// Checked to see if the provided token was cancelled. + /// if the exception was an and the token was canceled. + internal static bool IsCurrentOperationBeingCancelled(Exception exception, CancellationToken cancellationToken) + => exception is OperationCanceledException && cancellationToken.IsCancellationRequested; } } diff --git a/src/Compilers/Core/Portable/InternalUtilities/FatalError.cs b/src/Compilers/Core/Portable/InternalUtilities/FatalError.cs index 6007023687490..5b5e814e7b3f2 100644 --- a/src/Compilers/Core/Portable/InternalUtilities/FatalError.cs +++ b/src/Compilers/Core/Portable/InternalUtilities/FatalError.cs @@ -7,6 +7,7 @@ using System.Diagnostics.CodeAnalysis; using System.Reflection; using System.Threading; +using Roslyn.Utilities; #if NET20 // Some APIs referenced by documentation comments are not available on .NET Framework 2.0. @@ -91,9 +92,6 @@ public static void CopyHandlerTo(Assembly assembly) #endif - private static bool IsCurrentOperationBeingCancelled(Exception exception, CancellationToken cancellationToken) - => exception is OperationCanceledException && cancellationToken.IsCancellationRequested; - /// /// Use in an exception filter to report an error without catching the exception. /// The error is reported by calling . @@ -143,7 +141,7 @@ public static bool ReportAndPropagateUnlessCanceled(Exception exception, ErrorSe [DebuggerHidden] public static bool ReportAndPropagateUnlessCanceled(Exception exception, CancellationToken contextCancellationToken, ErrorSeverity severity = ErrorSeverity.Uncategorized) { - if (IsCurrentOperationBeingCancelled(exception, contextCancellationToken)) + if (ExceptionUtilities.IsCurrentOperationBeingCancelled(exception, contextCancellationToken)) { return false; } @@ -258,7 +256,7 @@ public static bool ReportIfNonFatalAndCatchUnlessCanceled(Exception exception, E #endif static bool ReportAndCatchUnlessCanceled(Exception exception, CancellationToken contextCancellationToken, ErrorSeverity severity = ErrorSeverity.Uncategorized) { - if (IsCurrentOperationBeingCancelled(exception, contextCancellationToken)) + if (ExceptionUtilities.IsCurrentOperationBeingCancelled(exception, contextCancellationToken)) { return false; } @@ -290,7 +288,7 @@ static bool ReportAndCatchUnlessCanceled(Exception exception, CancellationToken [Obsolete("This is only to support places the compiler is catching and swallowing exceptions on the command line; do not use in new code.")] public static bool ReportIfNonFatalAndCatchUnlessCanceled(Exception exception, CancellationToken contextCancellationToken, ErrorSeverity severity = ErrorSeverity.Uncategorized) { - if (IsCurrentOperationBeingCancelled(exception, contextCancellationToken)) + if (ExceptionUtilities.IsCurrentOperationBeingCancelled(exception, contextCancellationToken)) { return false; } diff --git a/src/Compilers/Core/Portable/SourceGeneration/GeneratorDriver.cs b/src/Compilers/Core/Portable/SourceGeneration/GeneratorDriver.cs index d4d845a7c20eb..9c4dc305cdb9a 100644 --- a/src/Compilers/Core/Portable/SourceGeneration/GeneratorDriver.cs +++ b/src/Compilers/Core/Portable/SourceGeneration/GeneratorDriver.cs @@ -10,7 +10,6 @@ using System.Linq; using System.Threading; using Microsoft.CodeAnalysis.Diagnostics; -using Microsoft.CodeAnalysis.ErrorReporting; using Microsoft.CodeAnalysis.PooledObjects; using Roslyn.Utilities; @@ -193,9 +192,7 @@ internal GeneratorDriverState RunGeneratorsCore(Compilation compilation, Diagnos { generator.Initialize(pipelineContext); } -#pragma warning disable CS0618 // ReportIfNonFatalAndCatchUnlessCanceled is obsolete; tracked by https://github.com/dotnet/roslyn/issues/58375 - catch (Exception e) when (FatalError.ReportIfNonFatalAndCatchUnlessCanceled(e, cancellationToken)) -#pragma warning restore CS0618 // ReportIfNonFatalAndCatchUnlessCanceled is obsolete + catch (Exception e) when (ExceptionUtilities.IsNotCancelled(e, cancellationToken)) { ex = e; } diff --git a/src/Compilers/Core/Portable/SourceGeneration/Nodes/SyntaxReceiverInputNode.cs b/src/Compilers/Core/Portable/SourceGeneration/Nodes/SyntaxReceiverInputNode.cs index 865edba0763b1..69e09fc213128 100644 --- a/src/Compilers/Core/Portable/SourceGeneration/Nodes/SyntaxReceiverInputNode.cs +++ b/src/Compilers/Core/Portable/SourceGeneration/Nodes/SyntaxReceiverInputNode.cs @@ -96,7 +96,7 @@ public void VisitTree(Lazy root, EntryState state, SemanticModel? mo lastElapsedTime = stopwatch.Elapsed; } } - catch (Exception e) + catch (Exception e) when (ExceptionUtilities.IsNotCancelled(e, cancellationToken)) { throw new UserFunctionException(e); } diff --git a/src/Compilers/Core/Portable/SourceGeneration/UserFunction.cs b/src/Compilers/Core/Portable/SourceGeneration/UserFunction.cs index f38b4cc2864b2..38b611c030cdc 100644 --- a/src/Compilers/Core/Portable/SourceGeneration/UserFunction.cs +++ b/src/Compilers/Core/Portable/SourceGeneration/UserFunction.cs @@ -6,7 +6,7 @@ using System.Collections.Generic; using System.Collections.Immutable; using System.Threading; -using Microsoft.CodeAnalysis.ErrorReporting; +using Roslyn.Utilities; namespace Microsoft.CodeAnalysis { @@ -30,9 +30,7 @@ internal static Func WrapUserFunction WrapUserAction(this Action userAc { userAction(input); } -#pragma warning disable CS0618 // ReportIfNonFatalAndCatchUnlessCanceled is obsolete; tracked by https://github.com/dotnet/roslyn/issues/58375 - catch (Exception e) when (FatalError.ReportIfNonFatalAndCatchUnlessCanceled(e)) -#pragma warning restore CS0618 // ReportIfNonFatalAndCatchUnlessCanceled is obsolete + catch (Exception e) when (ExceptionUtilities.IsNotCancelled(e)) { throw new UserFunctionException(e); } @@ -69,9 +65,7 @@ internal static Action WrapUserAction(this A { userAction(input1, input2); } -#pragma warning disable CS0618 // ReportIfNonFatalAndCatchUnlessCanceled is obsolete; tracked by https://github.com/dotnet/roslyn/issues/58375 - catch (Exception e) when (FatalError.ReportIfNonFatalAndCatchUnlessCanceled(e)) -#pragma warning restore CS0618 // ReportIfNonFatalAndCatchUnlessCanceled is obsolete + catch (Exception e) when (ExceptionUtilities.IsNotCancelled(e)) { throw new UserFunctionException(e); } From d7a7c30e3663a211230d8c9af112bdd696cae497 Mon Sep 17 00:00:00 2001 From: Chris Sienkiewicz Date: Tue, 18 Jan 2022 15:03:18 -0800 Subject: [PATCH 2/9] Simplify exception filter. Make all uses take a token Pass the token through from parent wrapper operations. --- .../SourceGeneration/GeneratorDriverTests.cs | 12 ++++++++---- .../InternalUtilities/ExceptionUtilities.cs | 16 ++-------------- .../Portable/SourceGeneration/GeneratorDriver.cs | 2 +- .../SourceGeneration/Nodes/PostInitOutputNode.cs | 6 +++--- .../SourceGeneration/Nodes/SourceOutputNode.cs | 6 +++--- .../Nodes/SyntaxReceiverInputNode.cs | 2 +- .../Portable/SourceGeneration/UserFunction.cs | 14 +++++++------- 7 files changed, 25 insertions(+), 33 deletions(-) diff --git a/src/Compilers/CSharp/Test/Semantic/SourceGeneration/GeneratorDriverTests.cs b/src/Compilers/CSharp/Test/Semantic/SourceGeneration/GeneratorDriverTests.cs index 083731a461eb5..bb31078de657d 100644 --- a/src/Compilers/CSharp/Test/Semantic/SourceGeneration/GeneratorDriverTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/SourceGeneration/GeneratorDriverTests.cs @@ -1527,13 +1527,17 @@ public void User_WrappedFunc_Throw_Exceptions() Assert.IsType(e.InnerException); } + CancellationTokenSource cts = new CancellationTokenSource(); + cts.Cancel(); + // cancellation is not wrapped, and is bubbled up - Assert.Throws(() => timeoutFunc(30, new CancellationToken(true))); - Assert.Throws(() => userTimeoutFunc(30, new CancellationToken(true))); + Assert.Throws(() => timeoutFunc(30, cts.Token)); + Assert.Throws(() => userTimeoutFunc(30, cts.Token)); // unless it wasn't *our* cancellation token, in which case it still gets wrapped - Assert.Throws(() => otherTimeoutFunc(30, CancellationToken.None)); - Assert.Throws(() => userOtherTimeoutFunc(30, CancellationToken.None)); + Assert.True(cts.TryReset()); + Assert.Throws(() => otherTimeoutFunc(30, cts.Token)); + Assert.Throws(() => userOtherTimeoutFunc(30, cts.Token)); } [Fact] diff --git a/src/Compilers/Core/Portable/InternalUtilities/ExceptionUtilities.cs b/src/Compilers/Core/Portable/InternalUtilities/ExceptionUtilities.cs index 1f0db0af93366..4d8c37a74ebb3 100644 --- a/src/Compilers/Core/Portable/InternalUtilities/ExceptionUtilities.cs +++ b/src/Compilers/Core/Portable/InternalUtilities/ExceptionUtilities.cs @@ -29,25 +29,13 @@ internal static Exception Unreachable get { return new InvalidOperationException("This program location is thought to be unreachable."); } } - /// - /// Determine if an exception was not an , and optionally check that the provided token was cancelled. - /// - /// - /// Used in exception filters to determine if the exception should be handled or not. - /// - /// The exception to test. - /// Optional. Checked to see if the cancellation was caused by the provided token and considered not cancelled if not. - /// if the exception should be handled by the caller, or if the exception was caused by cancellation. - internal static bool IsNotCancelled(Exception exception, CancellationToken? cancellationToken = null) - => !IsCurrentOperationBeingCancelled(exception, cancellationToken ?? new CancellationToken(true)); - /// - /// Determine if an exception was an , and that the token was cancelled. + /// Determine if an exception was an , and that the provided token caused the cancellation. /// /// The exception to test. /// Checked to see if the provided token was cancelled. /// if the exception was an and the token was canceled. internal static bool IsCurrentOperationBeingCancelled(Exception exception, CancellationToken cancellationToken) - => exception is OperationCanceledException && cancellationToken.IsCancellationRequested; + => exception is OperationCanceledException oce && oce.CancellationToken == cancellationToken; } } diff --git a/src/Compilers/Core/Portable/SourceGeneration/GeneratorDriver.cs b/src/Compilers/Core/Portable/SourceGeneration/GeneratorDriver.cs index 9c4dc305cdb9a..dcc1541554a6a 100644 --- a/src/Compilers/Core/Portable/SourceGeneration/GeneratorDriver.cs +++ b/src/Compilers/Core/Portable/SourceGeneration/GeneratorDriver.cs @@ -192,7 +192,7 @@ internal GeneratorDriverState RunGeneratorsCore(Compilation compilation, Diagnos { generator.Initialize(pipelineContext); } - catch (Exception e) when (ExceptionUtilities.IsNotCancelled(e, cancellationToken)) + catch (Exception e) when (!ExceptionUtilities.IsCurrentOperationBeingCancelled(e, cancellationToken)) { ex = e; } diff --git a/src/Compilers/Core/Portable/SourceGeneration/Nodes/PostInitOutputNode.cs b/src/Compilers/Core/Portable/SourceGeneration/Nodes/PostInitOutputNode.cs index cd55a2de97631..550b79682bb0f 100644 --- a/src/Compilers/Core/Portable/SourceGeneration/Nodes/PostInitOutputNode.cs +++ b/src/Compilers/Core/Portable/SourceGeneration/Nodes/PostInitOutputNode.cs @@ -9,9 +9,9 @@ namespace Microsoft.CodeAnalysis { internal sealed class PostInitOutputNode : IIncrementalGeneratorOutputNode { - private readonly Action _callback; + private readonly Action _callback; - public PostInitOutputNode(Action callback) + public PostInitOutputNode(Action callback) { _callback = callback; } @@ -20,7 +20,7 @@ public PostInitOutputNode(Action public void AppendOutputs(IncrementalExecutionContext context, CancellationToken cancellationToken) { - _callback(new IncrementalGeneratorPostInitializationContext(context.Sources, cancellationToken)); + _callback(new IncrementalGeneratorPostInitializationContext(context.Sources, cancellationToken), cancellationToken); } } } diff --git a/src/Compilers/Core/Portable/SourceGeneration/Nodes/SourceOutputNode.cs b/src/Compilers/Core/Portable/SourceGeneration/Nodes/SourceOutputNode.cs index 3e8dc48ef4d4b..12ed23b020569 100644 --- a/src/Compilers/Core/Portable/SourceGeneration/Nodes/SourceOutputNode.cs +++ b/src/Compilers/Core/Portable/SourceGeneration/Nodes/SourceOutputNode.cs @@ -17,13 +17,13 @@ internal sealed class SourceOutputNode : IIncrementalGeneratorOutputNode { private readonly IIncrementalGeneratorNode _source; - private readonly Action _action; + private readonly Action _action; private readonly IncrementalGeneratorOutputKind _outputKind; private readonly string _sourceExtension; - public SourceOutputNode(IIncrementalGeneratorNode source, Action action, IncrementalGeneratorOutputKind outputKind, string sourceExtension) + public SourceOutputNode(IIncrementalGeneratorNode source, Actionaction, IncrementalGeneratorOutputKind outputKind, string sourceExtension) { _source = source; _action = action; @@ -70,7 +70,7 @@ public NodeStateTable UpdateStateTable(DriverStateTable.Builder graphSt try { var stopwatch = SharedStopwatch.StartNew(); - _action(context, entry.Item); + _action(context, entry.Item, cancellationToken); var sourcesAndDiagnostics = (sourcesBuilder.ToImmutable(), diagnostics.ToReadOnly()); nodeTable.AddEntry(sourcesAndDiagnostics, EntryState.Added, stopwatch.Elapsed, inputs, EntryState.Added); } diff --git a/src/Compilers/Core/Portable/SourceGeneration/Nodes/SyntaxReceiverInputNode.cs b/src/Compilers/Core/Portable/SourceGeneration/Nodes/SyntaxReceiverInputNode.cs index 69e09fc213128..e96d9746965d5 100644 --- a/src/Compilers/Core/Portable/SourceGeneration/Nodes/SyntaxReceiverInputNode.cs +++ b/src/Compilers/Core/Portable/SourceGeneration/Nodes/SyntaxReceiverInputNode.cs @@ -96,7 +96,7 @@ public void VisitTree(Lazy root, EntryState state, SemanticModel? mo lastElapsedTime = stopwatch.Elapsed; } } - catch (Exception e) when (ExceptionUtilities.IsNotCancelled(e, cancellationToken)) + catch (Exception e) when (!ExceptionUtilities.IsCurrentOperationBeingCancelled(e, cancellationToken)) { throw new UserFunctionException(e); } diff --git a/src/Compilers/Core/Portable/SourceGeneration/UserFunction.cs b/src/Compilers/Core/Portable/SourceGeneration/UserFunction.cs index 38b611c030cdc..72ee87619702f 100644 --- a/src/Compilers/Core/Portable/SourceGeneration/UserFunction.cs +++ b/src/Compilers/Core/Portable/SourceGeneration/UserFunction.cs @@ -30,7 +30,7 @@ internal static Func WrapUserFunction> WrapUse return (input, token) => userFunction.WrapUserFunction()(input, token).ToImmutableArray(); } - internal static Action WrapUserAction(this Action userAction) + internal static Action WrapUserAction(this Action userAction) { - return input => + return (input, token) => { try { userAction(input); } - catch (Exception e) when (ExceptionUtilities.IsNotCancelled(e)) + catch (Exception e) when (!ExceptionUtilities.IsCurrentOperationBeingCancelled(e, token)) { throw new UserFunctionException(e); } }; } - internal static Action WrapUserAction(this Action userAction) + internal static Action WrapUserAction(this Action userAction) { - return (input1, input2) => + return (input1, input2, token) => { try { userAction(input1, input2); } - catch (Exception e) when (ExceptionUtilities.IsNotCancelled(e)) + catch (Exception e) when (!ExceptionUtilities.IsCurrentOperationBeingCancelled(e, token)) { throw new UserFunctionException(e); } From 79e0a5faaa92f2a67416ad737582af3efcfadff9 Mon Sep 17 00:00:00 2001 From: Chris Sienkiewicz Date: Tue, 18 Jan 2022 15:55:41 -0800 Subject: [PATCH 3/9] Remove cancellation filter for init --- src/Compilers/Core/Portable/SourceGeneration/GeneratorDriver.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Compilers/Core/Portable/SourceGeneration/GeneratorDriver.cs b/src/Compilers/Core/Portable/SourceGeneration/GeneratorDriver.cs index dcc1541554a6a..b5df676c89d9b 100644 --- a/src/Compilers/Core/Portable/SourceGeneration/GeneratorDriver.cs +++ b/src/Compilers/Core/Portable/SourceGeneration/GeneratorDriver.cs @@ -192,7 +192,7 @@ internal GeneratorDriverState RunGeneratorsCore(Compilation compilation, Diagnos { generator.Initialize(pipelineContext); } - catch (Exception e) when (!ExceptionUtilities.IsCurrentOperationBeingCancelled(e, cancellationToken)) + catch (Exception e) { ex = e; } From f1613ec36b9344927ad76b2d4e3257dbb5c7271c Mon Sep 17 00:00:00 2001 From: Chris Sienkiewicz Date: Thu, 20 Jan 2022 11:57:50 -0800 Subject: [PATCH 4/9] Formatting and windows test fix --- .../Test/Semantic/SourceGeneration/GeneratorDriverTests.cs | 2 +- .../Core/Portable/SourceGeneration/Nodes/SourceOutputNode.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Compilers/CSharp/Test/Semantic/SourceGeneration/GeneratorDriverTests.cs b/src/Compilers/CSharp/Test/Semantic/SourceGeneration/GeneratorDriverTests.cs index bb31078de657d..c2c8019e94cec 100644 --- a/src/Compilers/CSharp/Test/Semantic/SourceGeneration/GeneratorDriverTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/SourceGeneration/GeneratorDriverTests.cs @@ -1535,7 +1535,7 @@ public void User_WrappedFunc_Throw_Exceptions() Assert.Throws(() => userTimeoutFunc(30, cts.Token)); // unless it wasn't *our* cancellation token, in which case it still gets wrapped - Assert.True(cts.TryReset()); + cts = new CancellationTokenSource(); Assert.Throws(() => otherTimeoutFunc(30, cts.Token)); Assert.Throws(() => userOtherTimeoutFunc(30, cts.Token)); } diff --git a/src/Compilers/Core/Portable/SourceGeneration/Nodes/SourceOutputNode.cs b/src/Compilers/Core/Portable/SourceGeneration/Nodes/SourceOutputNode.cs index 12ed23b020569..b6b715f807af5 100644 --- a/src/Compilers/Core/Portable/SourceGeneration/Nodes/SourceOutputNode.cs +++ b/src/Compilers/Core/Portable/SourceGeneration/Nodes/SourceOutputNode.cs @@ -23,7 +23,7 @@ internal sealed class SourceOutputNode : IIncrementalGeneratorOutputNode private readonly string _sourceExtension; - public SourceOutputNode(IIncrementalGeneratorNode source, Actionaction, IncrementalGeneratorOutputKind outputKind, string sourceExtension) + public SourceOutputNode(IIncrementalGeneratorNode source, Action action, IncrementalGeneratorOutputKind outputKind, string sourceExtension) { _source = source; _action = action; From 7b053c2bb4aa331d81a1289de5e8319f8c4ffff4 Mon Sep 17 00:00:00 2001 From: Chris Sienkiewicz Date: Thu, 20 Jan 2022 13:40:36 -0800 Subject: [PATCH 5/9] Formatting --- .../Core/Portable/InternalUtilities/ExceptionUtilities.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Compilers/Core/Portable/InternalUtilities/ExceptionUtilities.cs b/src/Compilers/Core/Portable/InternalUtilities/ExceptionUtilities.cs index 4d8c37a74ebb3..98088b680dedc 100644 --- a/src/Compilers/Core/Portable/InternalUtilities/ExceptionUtilities.cs +++ b/src/Compilers/Core/Portable/InternalUtilities/ExceptionUtilities.cs @@ -29,7 +29,7 @@ internal static Exception Unreachable get { return new InvalidOperationException("This program location is thought to be unreachable."); } } - /// + /// /// Determine if an exception was an , and that the provided token caused the cancellation. /// /// The exception to test. From 4b4cfe6e9bf0fe0ac8ba44f0607a180c781f3d28 Mon Sep 17 00:00:00 2001 From: Chris Sienkiewicz Date: Wed, 26 Jan 2022 15:44:17 -0800 Subject: [PATCH 6/9] Restore cancellation check behavior --- .../SourceGeneration/GeneratorDriverTests.cs | 12 ++++-------- .../Portable/InternalUtilities/ExceptionUtilities.cs | 2 +- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/Compilers/CSharp/Test/Semantic/SourceGeneration/GeneratorDriverTests.cs b/src/Compilers/CSharp/Test/Semantic/SourceGeneration/GeneratorDriverTests.cs index c2c8019e94cec..083731a461eb5 100644 --- a/src/Compilers/CSharp/Test/Semantic/SourceGeneration/GeneratorDriverTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/SourceGeneration/GeneratorDriverTests.cs @@ -1527,17 +1527,13 @@ public void User_WrappedFunc_Throw_Exceptions() Assert.IsType(e.InnerException); } - CancellationTokenSource cts = new CancellationTokenSource(); - cts.Cancel(); - // cancellation is not wrapped, and is bubbled up - Assert.Throws(() => timeoutFunc(30, cts.Token)); - Assert.Throws(() => userTimeoutFunc(30, cts.Token)); + Assert.Throws(() => timeoutFunc(30, new CancellationToken(true))); + Assert.Throws(() => userTimeoutFunc(30, new CancellationToken(true))); // unless it wasn't *our* cancellation token, in which case it still gets wrapped - cts = new CancellationTokenSource(); - Assert.Throws(() => otherTimeoutFunc(30, cts.Token)); - Assert.Throws(() => userOtherTimeoutFunc(30, cts.Token)); + Assert.Throws(() => otherTimeoutFunc(30, CancellationToken.None)); + Assert.Throws(() => userOtherTimeoutFunc(30, CancellationToken.None)); } [Fact] diff --git a/src/Compilers/Core/Portable/InternalUtilities/ExceptionUtilities.cs b/src/Compilers/Core/Portable/InternalUtilities/ExceptionUtilities.cs index 98088b680dedc..f8ce4adb256ce 100644 --- a/src/Compilers/Core/Portable/InternalUtilities/ExceptionUtilities.cs +++ b/src/Compilers/Core/Portable/InternalUtilities/ExceptionUtilities.cs @@ -36,6 +36,6 @@ internal static Exception Unreachable /// Checked to see if the provided token was cancelled. /// if the exception was an and the token was canceled. internal static bool IsCurrentOperationBeingCancelled(Exception exception, CancellationToken cancellationToken) - => exception is OperationCanceledException oce && oce.CancellationToken == cancellationToken; + => exception is OperationCanceledException oce && cancellationToken.IsCancellationRequested; } } From ab0988186a9fe18810e85f292316f6f11bebcc39 Mon Sep 17 00:00:00 2001 From: Chris Sienkiewicz Date: Thu, 27 Jan 2022 12:31:59 -0800 Subject: [PATCH 7/9] Feedback --- .../SourceGeneration/SyntaxAwareGeneratorTests.cs | 11 ++++++++--- .../Portable/InternalUtilities/ExceptionUtilities.cs | 2 +- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/Compilers/CSharp/Test/Semantic/SourceGeneration/SyntaxAwareGeneratorTests.cs b/src/Compilers/CSharp/Test/Semantic/SourceGeneration/SyntaxAwareGeneratorTests.cs index 9663e7d30622f..874f52ea92568 100644 --- a/src/Compilers/CSharp/Test/Semantic/SourceGeneration/SyntaxAwareGeneratorTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/SourceGeneration/SyntaxAwareGeneratorTests.cs @@ -1981,14 +1981,19 @@ void Function() Assert.Single(compilation.SyntaxTrees); - var cts = new CancellationTokenSource(); var testGenerator = new CallbackGenerator( - onInit: (i) => i.RegisterForSyntaxNotifications(() => new TestSyntaxReceiver(tag: 0, callback: (a) => { if (a is AssignmentExpressionSyntax){ cts.Cancel(); cts.Token.ThrowIfCancellationRequested(); } })), + onInit: (i) => i.RegisterForSyntaxNotifications(() => new TestSyntaxReceiver(tag: 0, callback: (a) => { if (a is AssignmentExpressionSyntax) { throw new OperationCanceledException("Simulated cancellation from external source"); } })), onExecute: (e) => { e.AddSource("test", SourceText.From("public class D{}", Encoding.UTF8)); } ); GeneratorDriver driver = CSharpGeneratorDriver.Create(new[] { testGenerator }, parseOptions: parseOptions); - Assert.Throws(() => driver.RunGeneratorsAndUpdateCompilation(compilation, out var outputCompilation, out var outputDiagnostics, cts.Token)); + driver = driver.RunGenerators(compilation, CancellationToken.None); + var results = driver.GetRunResult(); + + + Assert.Single(results.Results); + Assert.IsType(results.Results[0].Exception); + Assert.Equal("Simulated cancellation from external source", results.Results[0].Exception.Message); } private class TestReceiverBase diff --git a/src/Compilers/Core/Portable/InternalUtilities/ExceptionUtilities.cs b/src/Compilers/Core/Portable/InternalUtilities/ExceptionUtilities.cs index f8ce4adb256ce..ba9629189ff06 100644 --- a/src/Compilers/Core/Portable/InternalUtilities/ExceptionUtilities.cs +++ b/src/Compilers/Core/Portable/InternalUtilities/ExceptionUtilities.cs @@ -36,6 +36,6 @@ internal static Exception Unreachable /// Checked to see if the provided token was cancelled. /// if the exception was an and the token was canceled. internal static bool IsCurrentOperationBeingCancelled(Exception exception, CancellationToken cancellationToken) - => exception is OperationCanceledException oce && cancellationToken.IsCancellationRequested; + => exception is OperationCanceledException && cancellationToken.IsCancellationRequested; } } From 7ba4bbf26836ca494521e9aea8415373453ea0fd Mon Sep 17 00:00:00 2001 From: Chris Sienkiewicz Date: Thu, 27 Jan 2022 12:50:56 -0800 Subject: [PATCH 8/9] Update src/Compilers/CSharp/Test/Semantic/SourceGeneration/SyntaxAwareGeneratorTests.cs Co-authored-by: Jared Parsons --- .../Test/Semantic/SourceGeneration/SyntaxAwareGeneratorTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Compilers/CSharp/Test/Semantic/SourceGeneration/SyntaxAwareGeneratorTests.cs b/src/Compilers/CSharp/Test/Semantic/SourceGeneration/SyntaxAwareGeneratorTests.cs index 874f52ea92568..43696fe0f04f4 100644 --- a/src/Compilers/CSharp/Test/Semantic/SourceGeneration/SyntaxAwareGeneratorTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/SourceGeneration/SyntaxAwareGeneratorTests.cs @@ -1990,7 +1990,6 @@ void Function() driver = driver.RunGenerators(compilation, CancellationToken.None); var results = driver.GetRunResult(); - Assert.Single(results.Results); Assert.IsType(results.Results[0].Exception); Assert.Equal("Simulated cancellation from external source", results.Results[0].Exception.Message); From 1c33b7e90c231777f99616b59756d3633c0de093 Mon Sep 17 00:00:00 2001 From: Chris Sienkiewicz Date: Thu, 27 Jan 2022 13:39:56 -0800 Subject: [PATCH 9/9] Fix null warning --- .../Test/Semantic/SourceGeneration/SyntaxAwareGeneratorTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Test/Semantic/SourceGeneration/SyntaxAwareGeneratorTests.cs b/src/Compilers/CSharp/Test/Semantic/SourceGeneration/SyntaxAwareGeneratorTests.cs index 43696fe0f04f4..e677403ca1a5a 100644 --- a/src/Compilers/CSharp/Test/Semantic/SourceGeneration/SyntaxAwareGeneratorTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/SourceGeneration/SyntaxAwareGeneratorTests.cs @@ -1992,7 +1992,7 @@ void Function() Assert.Single(results.Results); Assert.IsType(results.Results[0].Exception); - Assert.Equal("Simulated cancellation from external source", results.Results[0].Exception.Message); + Assert.Equal("Simulated cancellation from external source", results.Results[0].Exception!.Message); } private class TestReceiverBase