From b51fbae159e6351bc6c67c7579ea91fe6fde3c30 Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Wed, 10 Feb 2021 22:17:20 +0000 Subject: [PATCH 01/44] move Quit command to Microsoft.DotNet.Interactive and ensure tests for serialization roundtrip and contract validation --- ...act_has_not_been_broken.approved.Quit.json | 1 + .../Server/SerializationTests.cs | 2 ++ .../Commands/Quit.cs | 16 ++++++++--- .../Server/KernelCommandEnvelope.cs | 3 +- .../CommandLine/CommandLineParserTests.cs | 28 ++++++------------- src/dotnet-interactive/KernelExtensions.cs | 7 ++--- 6 files changed, 29 insertions(+), 28 deletions(-) create mode 100644 src/Microsoft.DotNet.Interactive.Tests/Server/SerializationTests.Command_contract_has_not_been_broken.approved.Quit.json rename src/{dotnet-interactive => Microsoft.DotNet.Interactive}/Commands/Quit.cs (57%) diff --git a/src/Microsoft.DotNet.Interactive.Tests/Server/SerializationTests.Command_contract_has_not_been_broken.approved.Quit.json b/src/Microsoft.DotNet.Interactive.Tests/Server/SerializationTests.Command_contract_has_not_been_broken.approved.Quit.json new file mode 100644 index 0000000000..075c03f505 --- /dev/null +++ b/src/Microsoft.DotNet.Interactive.Tests/Server/SerializationTests.Command_contract_has_not_been_broken.approved.Quit.json @@ -0,0 +1 @@ +{"token":"the-token","commandType":"Quit","command":{"targetKernelName":null}} \ No newline at end of file diff --git a/src/Microsoft.DotNet.Interactive.Tests/Server/SerializationTests.cs b/src/Microsoft.DotNet.Interactive.Tests/Server/SerializationTests.cs index d2bd77327e..0788d7df0e 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/Server/SerializationTests.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/Server/SerializationTests.cs @@ -196,6 +196,8 @@ IEnumerable commands() yield return new UpdateDisplayedValue( new FormattedValue("text/html", "hi!"), "the-value-id"); + + yield return new Quit(); } } diff --git a/src/dotnet-interactive/Commands/Quit.cs b/src/Microsoft.DotNet.Interactive/Commands/Quit.cs similarity index 57% rename from src/dotnet-interactive/Commands/Quit.cs rename to src/Microsoft.DotNet.Interactive/Commands/Quit.cs index 7862881aba..f8894cb6ea 100644 --- a/src/dotnet-interactive/Commands/Quit.cs +++ b/src/Microsoft.DotNet.Interactive/Commands/Quit.cs @@ -3,17 +3,25 @@ using System; +using System.Reactive.Disposables; using System.Threading.Tasks; -using Microsoft.DotNet.Interactive.Commands; -namespace Microsoft.DotNet.Interactive.App.Commands +namespace Microsoft.DotNet.Interactive.Commands { public class Quit : KernelCommand { - internal static IDisposable DisposeOnQuit { get; set; } + private static readonly CompositeDisposable DisposeOnQuit = new CompositeDisposable(); + public static void RegisterForDisposalOnQuit(IDisposable disposable) + { + if (disposable is not null) + { + DisposeOnQuit.Add(disposable); + } + } + public Quit(string targetKernelName = null): base(targetKernelName) { - Handler = (command, context) => + Handler = (_, context) => { context.Complete(context.Command); DisposeOnQuit?.Dispose(); diff --git a/src/Microsoft.DotNet.Interactive/Server/KernelCommandEnvelope.cs b/src/Microsoft.DotNet.Interactive/Server/KernelCommandEnvelope.cs index c94ba2abd0..fa337b30a1 100644 --- a/src/Microsoft.DotNet.Interactive/Server/KernelCommandEnvelope.cs +++ b/src/Microsoft.DotNet.Interactive/Server/KernelCommandEnvelope.cs @@ -74,7 +74,8 @@ public static void ResetToDefaults() [nameof(RequestSignatureHelp)] = typeof(KernelCommandEnvelope), [nameof(SerializeNotebook)] = typeof(KernelCommandEnvelope), [nameof(SubmitCode)] = typeof(KernelCommandEnvelope), - [nameof(UpdateDisplayedValue)] = typeof(KernelCommandEnvelope) + [nameof(UpdateDisplayedValue)] = typeof(KernelCommandEnvelope), + [nameof(Quit)] = typeof(KernelCommandEnvelope) }; _commandTypesByCommandTypeName = new ConcurrentDictionary(_envelopeTypesByCommandTypeName diff --git a/src/dotnet-interactive.Tests/CommandLine/CommandLineParserTests.cs b/src/dotnet-interactive.Tests/CommandLine/CommandLineParserTests.cs index af5e91aa2d..3b189b7274 100644 --- a/src/dotnet-interactive.Tests/CommandLine/CommandLineParserTests.cs +++ b/src/dotnet-interactive.Tests/CommandLine/CommandLineParserTests.cs @@ -8,16 +8,17 @@ using System.IO; using System.Linq; using System.Threading.Tasks; + using FluentAssertions; using FluentAssertions.Execution; + using Microsoft.DotNet.Interactive.App.CommandLine; -using Microsoft.DotNet.Interactive.App.Commands; using Microsoft.DotNet.Interactive.Http; using Microsoft.DotNet.Interactive.Server; using Microsoft.DotNet.Interactive.Telemetry; using Microsoft.DotNet.Interactive.Tests.Utility; -using Microsoft.DotNet.Interactive.Utility; using Microsoft.Extensions.DependencyInjection; + using Xunit; using Xunit.Abstractions; @@ -86,7 +87,7 @@ public async Task It_parses_log_output_directory() var logPath = new DirectoryInfo(Path.GetTempPath()); await _parser.InvokeAsync($"jupyter --log-path {logPath} {_connectionFile}", _console); - + _startOptions .LogPath .FullName @@ -132,7 +133,7 @@ public void jupyter_command_help_shows_default_port_range() public void jupyter_install_command_parses_path_option() { Directory.CreateDirectory(_kernelSpecInstallPath.FullName); - + _parser.InvokeAsync($"jupyter install --path {_kernelSpecInstallPath}"); var installedKernels = _kernelSpecInstallPath.GetDirectories(); @@ -204,7 +205,7 @@ public void http_command__does_not_parse_http_port_range_option() public async Task jupyter_command_returns_error_if_connection_file_path_is_not_passed() { var testConsole = new TestConsole(); - + await _parser.InvokeAsync("jupyter", testConsole); testConsole.Error.ToString().Should().Contain("Required argument missing for command: jupyter"); @@ -224,7 +225,7 @@ public void jupyter_command_does_not_parse_http_port_option() [Fact] public async Task jupyter_command_enables_http_api_when_http_port_range_is_specified() { - await _parser.InvokeAsync($"jupyter --http-port-range 3000-5000 {_connectionFile}"); + await _parser.InvokeAsync($"jupyter --http-port-range 3000-5000 {_connectionFile}"); _startOptions.EnableHttpApi.Should().BeTrue(); } @@ -248,7 +249,7 @@ public void jupyter_command_parses_connection_file_path() [Fact] public async Task jupyter_command_enables_http_api_by_default() { - await _parser.InvokeAsync($"jupyter {_connectionFile}"); + await _parser.InvokeAsync($"jupyter {_connectionFile}"); _startOptions.EnableHttpApi.Should().BeTrue(); } @@ -369,7 +370,7 @@ public async Task stdio_command_parses_http_port_range_options() public async Task stdio_command_requires_api_bootstrapping_when_http_is_enabled() { await _parser.InvokeAsync("stdio --http-port-range 3000-4000"); - + var kernel = GetKernel(); kernel.FrontendEnvironment.As() @@ -418,16 +419,5 @@ public void stdio_command_honors_default_kernel_option() options.DefaultKernel.Should().Be("bsharp"); } - - [Fact] - public async Task stdio_command_extends_the_protocol_with_quit_command() - { - await _parser.InvokeAsync("stdio"); - - var envelope = KernelCommandEnvelope.Deserialize(@"{ ""commandType"": ""Quit"", ""command"" : { } }"); - - envelope.Command.Should() - .BeOfType(); - } } } diff --git a/src/dotnet-interactive/KernelExtensions.cs b/src/dotnet-interactive/KernelExtensions.cs index c497ffb50c..be54291966 100644 --- a/src/dotnet-interactive/KernelExtensions.cs +++ b/src/dotnet-interactive/KernelExtensions.cs @@ -17,8 +17,7 @@ public static class KernelExtensions { public static T UseQuitCommand(this T kernel, IDisposable disposeOnQuit, CancellationToken cancellationToken) where T : Kernel { - Quit.DisposeOnQuit = disposeOnQuit; - KernelCommandEnvelope.RegisterCommandType(nameof(Quit)); + Quit.RegisterForDisposalOnQuit(disposeOnQuit); cancellationToken.Register(async () => { await kernel.SendAsync(new Quit()); @@ -42,8 +41,8 @@ public static T UseAboutMagicCommand(this T kernel) var url = "https://github.com/dotnet/interactive"; var encodedImage = string.Empty; - var assembly = typeof(Program).Assembly; - using (var resourceStream = assembly.GetManifestResourceStream($"{typeof(Program).Namespace}.resources.logo-456x456.png")) + var assembly = typeof(KernelExtensions).Assembly; + using (var resourceStream = assembly.GetManifestResourceStream($"{typeof(KernelExtensions).Namespace}.resources.logo-456x456.png")) { if (resourceStream != null) { From 91b3a7086a2475090100ccfd7488b06b48ca641b Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Wed, 10 Feb 2021 22:23:46 +0000 Subject: [PATCH 02/44] move kernel extension to enable quit command --- .../Commands/Quit.cs | 1 + .../KernelExtensions.cs | 10 ++++++++++ src/dotnet-interactive/KernelExtensions.cs | 20 +++++-------------- 3 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/Microsoft.DotNet.Interactive/Commands/Quit.cs b/src/Microsoft.DotNet.Interactive/Commands/Quit.cs index f8894cb6ea..6a74f69bd4 100644 --- a/src/Microsoft.DotNet.Interactive/Commands/Quit.cs +++ b/src/Microsoft.DotNet.Interactive/Commands/Quit.cs @@ -11,6 +11,7 @@ namespace Microsoft.DotNet.Interactive.Commands public class Quit : KernelCommand { private static readonly CompositeDisposable DisposeOnQuit = new CompositeDisposable(); + public static void RegisterForDisposalOnQuit(IDisposable disposable) { if (disposable is not null) diff --git a/src/Microsoft.DotNet.Interactive/KernelExtensions.cs b/src/Microsoft.DotNet.Interactive/KernelExtensions.cs index f3bd306218..28e2c9010b 100644 --- a/src/Microsoft.DotNet.Interactive/KernelExtensions.cs +++ b/src/Microsoft.DotNet.Interactive/KernelExtensions.cs @@ -22,6 +22,16 @@ namespace Microsoft.DotNet.Interactive { public static class KernelExtensions { + public static T UseQuitCommand(this T kernel, IDisposable disposeOnQuit, CancellationToken cancellationToken) where T : Kernel + { + Quit.RegisterForDisposalOnQuit(disposeOnQuit); + cancellationToken.Register(async () => + { + await kernel.SendAsync(new Quit()); + }); + return kernel; + } + public static Kernel FindKernel(this Kernel kernel, string name) { var root = kernel diff --git a/src/dotnet-interactive/KernelExtensions.cs b/src/dotnet-interactive/KernelExtensions.cs index be54291966..e5e763a8c1 100644 --- a/src/dotnet-interactive/KernelExtensions.cs +++ b/src/dotnet-interactive/KernelExtensions.cs @@ -4,28 +4,18 @@ using System; using System.CommandLine; using System.CommandLine.Invocation; -using System.Threading; -using Microsoft.DotNet.Interactive.App.Commands; + using Microsoft.DotNet.Interactive.Formatting; -using Microsoft.DotNet.Interactive.Server; + using Recipes; + using static Microsoft.DotNet.Interactive.Formatting.PocketViewTags; namespace Microsoft.DotNet.Interactive.App { public static class KernelExtensions { - public static T UseQuitCommand(this T kernel, IDisposable disposeOnQuit, CancellationToken cancellationToken) where T : Kernel - { - Quit.RegisterForDisposalOnQuit(disposeOnQuit); - cancellationToken.Register(async () => - { - await kernel.SendAsync(new Quit()); - }); - return kernel; - } - - public static T UseAboutMagicCommand(this T kernel) + public static T UseAbout(this T kernel) where T : Kernel { var about = new Command("#!about", "Show version and build information") @@ -56,7 +46,7 @@ public static T UseAboutMagicCommand(this T kernel) PocketView html = table( tbody( tr( - td(img[src: encodedImage, width:"125em"]), + td(img[src: encodedImage, width: "125em"]), td[style: "line-height:.8em"]( p[style: "font-size:1.5em"](b(".NET Interactive")), p("© 2020 Microsoft Corporation"), From 262e87fb51d70f3e543450f993b97af1b1c367da Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Wed, 10 Feb 2021 23:52:36 +0000 Subject: [PATCH 03/44] add tests for quit command behaviour --- .../CompositeKernelTests.cs | 91 +++++++++++++++++++ .../Commands/Quit.cs | 19 +++- .../CompositeKernel.cs | 8 ++ src/Microsoft.DotNet.Interactive/Kernel.cs | 19 +++- .../KernelInvocationContext.cs | 2 + 5 files changed, 133 insertions(+), 6 deletions(-) diff --git a/src/Microsoft.DotNet.Interactive.Tests/CompositeKernelTests.cs b/src/Microsoft.DotNet.Interactive.Tests/CompositeKernelTests.cs index 6f6a40ea70..d417d1d3bc 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/CompositeKernelTests.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/CompositeKernelTests.cs @@ -8,6 +8,7 @@ using System.Linq; using System.Threading.Tasks; using FluentAssertions; +using FluentAssertions.Execution; using Microsoft.DotNet.Interactive.Commands; using Microsoft.DotNet.Interactive.CSharp; using Microsoft.DotNet.Interactive.Events; @@ -425,6 +426,96 @@ public async Task Deferred_commands_on_composite_kernel_are_execute_on_first_sub typeof(CommandSucceeded)); } + [Fact] + public async Task Deferred_commands_on_composite_kernel_are_not_executed_when_quit_command_is_submitted() + { + var deferredCommandExecuted = false; + var quitCommandHandled = false; + + var subKernel = new CSharpKernel(); + + using var compositeKernel = new CompositeKernel + { + subKernel + }; + + compositeKernel.DefaultKernelName = subKernel.Name; + + var deferred = new SubmitCode("placeholder") + { + Handler = (command, context) => + { + deferredCommandExecuted = true; + return Task.CompletedTask; + } + }; + + compositeKernel.DeferCommand(deferred); + + var events = compositeKernel.KernelEvents.ToSubscribedList(); + + await compositeKernel.SendAsync(new Quit(() => + { + quitCommandHandled = true; + })); + + using var _ = new AssertionScope(); + + deferredCommandExecuted.Should().BeFalse(); + quitCommandHandled.Should().BeTrue(); + + events + .Select(e => e.GetType()) + .Should() + .ContainInOrder( + typeof(CommandSucceeded)); + } + + [Fact] + public async Task Deferred_commands_on_subkernels_are_not_executed_when_quit_command_is_submitted() + { + var deferredCommandExecuted = false; + var quitCommandHandled = false; + + var subKernel = new CSharpKernel(); + + using var compositeKernel = new CompositeKernel + { + subKernel + }; + + compositeKernel.DefaultKernelName = subKernel.Name; + + var deferred = new SubmitCode("placeholder") + { + Handler = (command, context) => + { + deferredCommandExecuted = true; + return Task.CompletedTask; + } + }; + + subKernel.DeferCommand(deferred); + + var events = compositeKernel.KernelEvents.ToSubscribedList(); + + await compositeKernel.SendAsync(new Quit(() => + { + quitCommandHandled = true; + })); + + using var _ = new AssertionScope(); + + deferredCommandExecuted.Should().BeFalse(); + quitCommandHandled.Should().BeTrue(); + + events + .Select(e => e.GetType()) + .Should() + .ContainInOrder( + typeof(CommandSucceeded)); + } + [Fact] public async Task Deferred_commands_on_composite_kernel_can_use_directives() { diff --git a/src/Microsoft.DotNet.Interactive/Commands/Quit.cs b/src/Microsoft.DotNet.Interactive/Commands/Quit.cs index 6a74f69bd4..88adbf5da3 100644 --- a/src/Microsoft.DotNet.Interactive/Commands/Quit.cs +++ b/src/Microsoft.DotNet.Interactive/Commands/Quit.cs @@ -4,6 +4,7 @@ using System; using System.Reactive.Disposables; +using System.Text.Json.Serialization; using System.Threading.Tasks; namespace Microsoft.DotNet.Interactive.Commands @@ -20,13 +21,25 @@ public static void RegisterForDisposalOnQuit(IDisposable disposable) } } - public Quit(string targetKernelName = null): base(targetKernelName) + [JsonConstructor] + public Quit(string targetKernelName = null): this(() => { + Environment.Exit(0); + }, targetKernelName) + { + } + + public Quit(Action onQuit, string targetKernelName = null) : base(targetKernelName) + { + if (onQuit == null) + { + throw new ArgumentNullException(nameof(onQuit)); + } Handler = (_, context) => { - context.Complete(context.Command); + context?.Complete(context.Command); DisposeOnQuit?.Dispose(); - Environment.Exit(0); + onQuit(); return Task.CompletedTask; }; } diff --git a/src/Microsoft.DotNet.Interactive/CompositeKernel.cs b/src/Microsoft.DotNet.Interactive/CompositeKernel.cs index f90d3115eb..c0e94d2803 100644 --- a/src/Microsoft.DotNet.Interactive/CompositeKernel.cs +++ b/src/Microsoft.DotNet.Interactive/CompositeKernel.cs @@ -213,6 +213,14 @@ internal override async Task HandleAsync( throw new NoSuitableKernelException(command); } + switch (command) + { + case Quit _: + ClearPendingCommands(); + kernel.ClearPendingCommands(); + break; + } + await kernel.RunDeferredCommandsAsync(); if (kernel != this) diff --git a/src/Microsoft.DotNet.Interactive/Kernel.cs b/src/Microsoft.DotNet.Interactive/Kernel.cs index cc9ab76838..c38ab508e4 100644 --- a/src/Microsoft.DotNet.Interactive/Kernel.cs +++ b/src/Microsoft.DotNet.Interactive/Kernel.cs @@ -312,11 +312,18 @@ internal Task SendAsync( throw new ArgumentNullException(nameof(command)); } - UndeferCommands(); - var tcs = new TaskCompletionSource(); - + var operation = new KernelOperation(command, tcs); + + switch (command) + { + case Quit _: + ClearPendingCommands(); + break; + } + + UndeferCommands(); _commandQueue.Enqueue(operation); @@ -347,6 +354,12 @@ private void ProcessCommandQueue( } } + internal void ClearPendingCommands() + { + _deferredCommands.Clear(); + _commandQueue.Clear(); + } + internal Task RunDeferredCommandsAsync() { var tcs = new TaskCompletionSource(); diff --git a/src/Microsoft.DotNet.Interactive/KernelInvocationContext.cs b/src/Microsoft.DotNet.Interactive/KernelInvocationContext.cs index f160e6b6d9..56144df568 100644 --- a/src/Microsoft.DotNet.Interactive/KernelInvocationContext.cs +++ b/src/Microsoft.DotNet.Interactive/KernelInvocationContext.cs @@ -35,6 +35,7 @@ private KernelInvocationContext(KernelCommand command) CommandToSignalCompletion = command; Result = new KernelCommandResult(_events); + _disposables.Add(_cancellationTokenSource); _disposables.Add(ConsoleOutput.Subscribe(c => { return new CompositeDisposable @@ -77,6 +78,7 @@ public void Fail( Publish(new CommandFailed(exception, Command, message)); _events.OnCompleted(); + _cancellationTokenSource.Cancel(false); IsComplete = true; } From fc79eb533c302f67efab7ccedceafbdd92a5f649 Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Thu, 11 Feb 2021 01:39:41 +0000 Subject: [PATCH 04/44] add changes to contract --- .../src/dotnet-interactive/contracts.ts | 5 +++++ .../src/interfaces/src/contracts.ts | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/src/Microsoft.DotNet.Interactive.Js/src/dotnet-interactive/contracts.ts b/src/Microsoft.DotNet.Interactive.Js/src/dotnet-interactive/contracts.ts index c2e74ba698..b1aa8d0b46 100644 --- a/src/Microsoft.DotNet.Interactive.Js/src/dotnet-interactive/contracts.ts +++ b/src/Microsoft.DotNet.Interactive.Js/src/dotnet-interactive/contracts.ts @@ -10,6 +10,7 @@ export const ChangeWorkingDirectoryType = "ChangeWorkingDirectory"; export const DisplayErrorType = "DisplayError"; export const DisplayValueType = "DisplayValue"; export const ParseNotebookType = "ParseNotebook"; +export const QuitType = "Quit"; export const RequestCompletionsType = "RequestCompletions"; export const RequestDiagnosticsType = "RequestDiagnostics"; export const RequestHoverTextType = "RequestHoverText"; @@ -24,6 +25,7 @@ export type KernelCommandType = | typeof DisplayErrorType | typeof DisplayValueType | typeof ParseNotebookType + | typeof QuitType | typeof RequestCompletionsType | typeof RequestDiagnosticsType | typeof RequestHoverTextType @@ -58,6 +60,9 @@ export interface ParseNotebook extends KernelCommand { rawData: Uint8Array; } +export interface Quit extends KernelCommand { +} + export interface RequestCompletions extends LanguageServiceCommand { } diff --git a/src/dotnet-interactive-vscode/src/interfaces/src/contracts.ts b/src/dotnet-interactive-vscode/src/interfaces/src/contracts.ts index c2e74ba698..b1aa8d0b46 100644 --- a/src/dotnet-interactive-vscode/src/interfaces/src/contracts.ts +++ b/src/dotnet-interactive-vscode/src/interfaces/src/contracts.ts @@ -10,6 +10,7 @@ export const ChangeWorkingDirectoryType = "ChangeWorkingDirectory"; export const DisplayErrorType = "DisplayError"; export const DisplayValueType = "DisplayValue"; export const ParseNotebookType = "ParseNotebook"; +export const QuitType = "Quit"; export const RequestCompletionsType = "RequestCompletions"; export const RequestDiagnosticsType = "RequestDiagnostics"; export const RequestHoverTextType = "RequestHoverText"; @@ -24,6 +25,7 @@ export type KernelCommandType = | typeof DisplayErrorType | typeof DisplayValueType | typeof ParseNotebookType + | typeof QuitType | typeof RequestCompletionsType | typeof RequestDiagnosticsType | typeof RequestHoverTextType @@ -58,6 +60,9 @@ export interface ParseNotebook extends KernelCommand { rawData: Uint8Array; } +export interface Quit extends KernelCommand { +} + export interface RequestCompletions extends LanguageServiceCommand { } From 0e9ee79ee711a915ac180c6060608772e9bd3cbd Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Thu, 11 Feb 2021 12:25:09 +0000 Subject: [PATCH 05/44] ensure inflight commands are failed on quit --- .../LanguageKernelTests.cs | 47 +++++++++++++++++++ src/Microsoft.DotNet.Interactive/Kernel.cs | 34 +++++++++++--- 2 files changed, 74 insertions(+), 7 deletions(-) diff --git a/src/Microsoft.DotNet.Interactive.Tests/LanguageKernelTests.cs b/src/Microsoft.DotNet.Interactive.Tests/LanguageKernelTests.cs index 1d46d0d6dd..97aff88510 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/LanguageKernelTests.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/LanguageKernelTests.cs @@ -1264,5 +1264,52 @@ public async Task SetVariableAsync_can_redeclare_an_existing_variable_and_change succeeded.Should().BeTrue(); x.Should().Be("hello"); } + + [Theory] + [InlineData(Language.CSharp,"System.Threading.Thread.Sleep(3000);")] + [InlineData(Language.FSharp, "System.Threading.Thread.Sleep(3000)")] + [InlineData(Language.PowerShell, "Start-Sleep -Milliseconds 3000")] + public void Quit_command_cause_current_command_to_fail(Language language, string code) + { + var kernel = CreateKernel(language); + + var languageKernel = kernel.ChildKernels.OfType().Single(); + var quitCommandExecuted = false; + + var quitCommand = new Quit(() => { + quitCommandExecuted = true; + }); + + var submitCodeCommand = new SubmitCode(code); + + Task.WhenAll( + Task.Run( () => languageKernel.SendAsync(submitCodeCommand)), + Task.Run(async () => + { + await Task.Delay(2000); + await languageKernel.SendAsync(quitCommand); + })) + .Wait(TimeSpan.FromSeconds(20)); + + using var _ = new AssertionScope(); + + quitCommandExecuted.Should().BeTrue(); + + KernelEvents + .Should() + .ContainSingle() + .Which + .Command + .Should() + .Be(quitCommand); + + KernelEvents + .Should() + .ContainSingle() + .Which + .Command + .Should() + .Be(submitCodeCommand); + } } } diff --git a/src/Microsoft.DotNet.Interactive/Kernel.cs b/src/Microsoft.DotNet.Interactive/Kernel.cs index c38ab508e4..c0a19a3b2e 100644 --- a/src/Microsoft.DotNet.Interactive/Kernel.cs +++ b/src/Microsoft.DotNet.Interactive/Kernel.cs @@ -235,10 +235,12 @@ private class KernelOperation { public KernelOperation( KernelCommand command, - TaskCompletionSource taskCompletionSource) + TaskCompletionSource taskCompletionSource, + bool IsDeferredOperation) { Command = command; TaskCompletionSource = taskCompletionSource; + this.IsDeferredOperation = IsDeferredOperation; AsyncContext.TryEstablish(out var id); AsyncContextId = id; @@ -247,6 +249,7 @@ public KernelOperation( public KernelCommand Command { get; } public TaskCompletionSource TaskCompletionSource { get; } + public bool IsDeferredOperation { get; } public int AsyncContextId { get; } } @@ -314,19 +317,20 @@ internal Task SendAsync( var tcs = new TaskCompletionSource(); - var operation = new KernelOperation(command, tcs); + var operation = new KernelOperation(command, tcs, false); switch (command) { case Quit _: ClearPendingCommands(); + _commandQueue.Enqueue(operation); + break; + default: + UndeferCommands(); + _commandQueue.Enqueue(operation); break; } - UndeferCommands(); - - _commandQueue.Enqueue(operation); - ProcessCommandQueue(_commandQueue, cancellationToken, onDone); return tcs.Task; @@ -356,8 +360,20 @@ private void ProcessCommandQueue( internal void ClearPendingCommands() { + KernelInvocationContext currentContext = null; + var inFlightOperation = _commandQueue.FirstOrDefault(c => !c.IsDeferredOperation); + if (inFlightOperation is not null + ) + { + currentContext = KernelInvocationContext.Current?? KernelInvocationContext.Establish(inFlightOperation.Command); + inFlightOperation.TaskCompletionSource.SetResult(currentContext.Result); + } + _deferredCommands.Clear(); _commandQueue.Clear(); + + currentContext?.Fail(new OperationCanceledException("Command has been cancelled")); + } internal Task RunDeferredCommandsAsync() @@ -375,7 +391,11 @@ private void UndeferCommands() { while (_deferredCommands.TryDequeue(out var initCommand)) { - _commandQueue.Enqueue(new KernelOperation(initCommand, new TaskCompletionSource())); + _commandQueue.Enqueue( + new KernelOperation( + initCommand, + new TaskCompletionSource(), + true)); } } From e41a0d7ce4c32095ca24f51347ab0c7631664261 Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Thu, 11 Feb 2021 13:43:50 +0000 Subject: [PATCH 06/44] wiring up parent kernel event pump --- .../LanguageKernelTests.cs | 12 ++++++++---- src/Microsoft.DotNet.Interactive/Kernel.cs | 2 ++ 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.DotNet.Interactive.Tests/LanguageKernelTests.cs b/src/Microsoft.DotNet.Interactive.Tests/LanguageKernelTests.cs index 97aff88510..9d2c59c5c1 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/LanguageKernelTests.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/LanguageKernelTests.cs @@ -1283,10 +1283,14 @@ public void Quit_command_cause_current_command_to_fail(Language language, string var submitCodeCommand = new SubmitCode(code); Task.WhenAll( - Task.Run( () => languageKernel.SendAsync(submitCodeCommand)), Task.Run(async () => { - await Task.Delay(2000); + await Task.Delay(20); + await languageKernel.SendAsync(submitCodeCommand); + }), + Task.Run(async () => + { + await Task.Delay(100); await languageKernel.SendAsync(quitCommand); })) .Wait(TimeSpan.FromSeconds(20)); @@ -1307,9 +1311,9 @@ public void Quit_command_cause_current_command_to_fail(Language language, string .Should() .ContainSingle() .Which - .Command + .Exception .Should() - .Be(submitCodeCommand); + .BeOfType(); } } } diff --git a/src/Microsoft.DotNet.Interactive/Kernel.cs b/src/Microsoft.DotNet.Interactive/Kernel.cs index c0a19a3b2e..23664f83aa 100644 --- a/src/Microsoft.DotNet.Interactive/Kernel.cs +++ b/src/Microsoft.DotNet.Interactive/Kernel.cs @@ -360,12 +360,14 @@ private void ProcessCommandQueue( internal void ClearPendingCommands() { + using var disposables = new CompositeDisposable(); KernelInvocationContext currentContext = null; var inFlightOperation = _commandQueue.FirstOrDefault(c => !c.IsDeferredOperation); if (inFlightOperation is not null ) { currentContext = KernelInvocationContext.Current?? KernelInvocationContext.Establish(inFlightOperation.Command); + disposables.Add( currentContext.KernelEvents.Subscribe(PublishEvent)); inFlightOperation.TaskCompletionSource.SetResult(currentContext.Result); } From 471e312b6d2b4314841aabc8a268f187149ddd5b Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Fri, 12 Feb 2021 01:16:15 +0000 Subject: [PATCH 07/44] add cancel command --- .../CompositeKernelTests.cs | 127 +++++++++++++++--- .../LanguageKernelTests.cs | 20 ++- ...t_has_not_been_broken.approved.Cancel.json | 1 + ...act_has_not_been_broken.approved.Quit.json | 2 +- .../Server/SerializationTests.cs | 4 +- .../Commands/Cancel.cs | 15 +++ .../Commands/Quit.cs | 2 +- .../CompositeKernel.cs | 3 + src/Microsoft.DotNet.Interactive/Kernel.cs | 48 +++---- .../Server/KernelCommandEnvelope.cs | 3 +- 10 files changed, 177 insertions(+), 48 deletions(-) create mode 100644 src/Microsoft.DotNet.Interactive.Tests/Server/SerializationTests.Command_contract_has_not_been_broken.approved.Cancel.json create mode 100644 src/Microsoft.DotNet.Interactive/Commands/Cancel.cs diff --git a/src/Microsoft.DotNet.Interactive.Tests/CompositeKernelTests.cs b/src/Microsoft.DotNet.Interactive.Tests/CompositeKernelTests.cs index d417d1d3bc..85b4c957d3 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/CompositeKernelTests.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/CompositeKernelTests.cs @@ -427,7 +427,7 @@ public async Task Deferred_commands_on_composite_kernel_are_execute_on_first_sub } [Fact] - public async Task Deferred_commands_on_composite_kernel_are_not_executed_when_quit_command_is_submitted() + public async Task quit_command_cancels_all_deferred_commands_on_composite_kernel() { var deferredCommandExecuted = false; var quitCommandHandled = false; @@ -450,14 +450,17 @@ public async Task Deferred_commands_on_composite_kernel_are_not_executed_when_qu } }; + + var quitCommand = new Quit(() => + { + quitCommandHandled = true; + }); + compositeKernel.DeferCommand(deferred); var events = compositeKernel.KernelEvents.ToSubscribedList(); - await compositeKernel.SendAsync(new Quit(() => - { - quitCommandHandled = true; - })); + await compositeKernel.SendAsync(quitCommand); using var _ = new AssertionScope(); @@ -465,14 +468,15 @@ await compositeKernel.SendAsync(new Quit(() => quitCommandHandled.Should().BeTrue(); events - .Select(e => e.GetType()) + .Should().ContainSingle() + .Which + .Command .Should() - .ContainInOrder( - typeof(CommandSucceeded)); + .Be(quitCommand); } [Fact] - public async Task Deferred_commands_on_subkernels_are_not_executed_when_quit_command_is_submitted() + public async Task quit_command_cancels_all_deferred_commands_on_subkernels() { var deferredCommandExecuted = false; var quitCommandHandled = false; @@ -495,14 +499,16 @@ public async Task Deferred_commands_on_subkernels_are_not_executed_when_quit_com } }; + var quitCommand = new Quit(() => + { + quitCommandHandled = true; + }); + subKernel.DeferCommand(deferred); - + var events = compositeKernel.KernelEvents.ToSubscribedList(); - await compositeKernel.SendAsync(new Quit(() => - { - quitCommandHandled = true; - })); + await compositeKernel.SendAsync(quitCommand); using var _ = new AssertionScope(); @@ -510,10 +516,97 @@ await compositeKernel.SendAsync(new Quit(() => quitCommandHandled.Should().BeTrue(); events - .Select(e => e.GetType()) + .Should().ContainSingle() + .Which + .Command .Should() - .ContainInOrder( - typeof(CommandSucceeded)); + .Be(quitCommand); + } + + [Fact] + public async Task cancel_command_cancels_all_deferred_commands_on_composite_kernel() + { + var deferredCommandExecuted = false; + + var subKernel = new CSharpKernel(); + + using var compositeKernel = new CompositeKernel + { + subKernel + }; + + compositeKernel.DefaultKernelName = subKernel.Name; + + var deferred = new SubmitCode("placeholder") + { + Handler = (command, context) => + { + deferredCommandExecuted = true; + return Task.CompletedTask; + } + }; + + var cancelCommand = new Cancel(); + + compositeKernel.DeferCommand(deferred); + + var events = compositeKernel.KernelEvents.ToSubscribedList(); + + await compositeKernel.SendAsync(cancelCommand); + + using var _ = new AssertionScope(); + + deferredCommandExecuted.Should().BeFalse(); + + events + .Should().ContainSingle() + .Which + .Command + .Should() + .Be(cancelCommand); + } + + [Fact] + public async Task cancel_command_cancels_all_deferred_commands_on_subkernels() + { + var deferredCommandExecuted = false; + + var subKernel = new CSharpKernel(); + + using var compositeKernel = new CompositeKernel + { + subKernel + }; + + compositeKernel.DefaultKernelName = subKernel.Name; + + var deferred = new SubmitCode("placeholder") + { + Handler = (command, context) => + { + deferredCommandExecuted = true; + return Task.CompletedTask; + } + }; + + var cancelCommand = new Cancel(); + + subKernel.DeferCommand(deferred); + + var events = compositeKernel.KernelEvents.ToSubscribedList(); + + await compositeKernel.SendAsync(cancelCommand); + + using var _ = new AssertionScope(); + + deferredCommandExecuted.Should().BeFalse(); + + events + .Should().ContainSingle() + .Which + .Command + .Should() + .Be(cancelCommand); } [Fact] diff --git a/src/Microsoft.DotNet.Interactive.Tests/LanguageKernelTests.cs b/src/Microsoft.DotNet.Interactive.Tests/LanguageKernelTests.cs index 9d2c59c5c1..331517bfd1 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/LanguageKernelTests.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/LanguageKernelTests.cs @@ -1268,7 +1268,7 @@ public async Task SetVariableAsync_can_redeclare_an_existing_variable_and_change [Theory] [InlineData(Language.CSharp,"System.Threading.Thread.Sleep(3000);")] [InlineData(Language.FSharp, "System.Threading.Thread.Sleep(3000)")] - [InlineData(Language.PowerShell, "Start-Sleep -Milliseconds 3000")] + [InlineData(Language.PowerShell, "Start-Sleep -Milliseconds 3000", Skip = "to address later")] public void Quit_command_cause_current_command_to_fail(Language language, string code) { var kernel = CreateKernel(language); @@ -1315,5 +1315,23 @@ public void Quit_command_cause_current_command_to_fail(Language language, string .Should() .BeOfType(); } + + [Fact] + public void user_code_can_react_to_cancel_command_using_cancellation_token() + { + throw new NotImplementedException(); + } + + [Fact] + public void cancel_command_cancels_the_running_command() + { + throw new NotImplementedException(); + } + + [Fact] + public void cancel_command_cancels_all_deferred_commands() + { + throw new NotImplementedException(); + } } } diff --git a/src/Microsoft.DotNet.Interactive.Tests/Server/SerializationTests.Command_contract_has_not_been_broken.approved.Cancel.json b/src/Microsoft.DotNet.Interactive.Tests/Server/SerializationTests.Command_contract_has_not_been_broken.approved.Cancel.json new file mode 100644 index 0000000000..2dc8e40766 --- /dev/null +++ b/src/Microsoft.DotNet.Interactive.Tests/Server/SerializationTests.Command_contract_has_not_been_broken.approved.Cancel.json @@ -0,0 +1 @@ +{"token":"the-token","commandType":"Cancel","command":{"targetKernelName":"csharp"}} \ No newline at end of file diff --git a/src/Microsoft.DotNet.Interactive.Tests/Server/SerializationTests.Command_contract_has_not_been_broken.approved.Quit.json b/src/Microsoft.DotNet.Interactive.Tests/Server/SerializationTests.Command_contract_has_not_been_broken.approved.Quit.json index 075c03f505..72391cb97a 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/Server/SerializationTests.Command_contract_has_not_been_broken.approved.Quit.json +++ b/src/Microsoft.DotNet.Interactive.Tests/Server/SerializationTests.Command_contract_has_not_been_broken.approved.Quit.json @@ -1 +1 @@ -{"token":"the-token","commandType":"Quit","command":{"targetKernelName":null}} \ No newline at end of file +{"token":"the-token","commandType":"Quit","command":{"targetKernelName":"csharp"}} \ No newline at end of file diff --git a/src/Microsoft.DotNet.Interactive.Tests/Server/SerializationTests.cs b/src/Microsoft.DotNet.Interactive.Tests/Server/SerializationTests.cs index 0788d7df0e..afb0a17b27 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/Server/SerializationTests.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/Server/SerializationTests.cs @@ -197,7 +197,9 @@ IEnumerable commands() new FormattedValue("text/html", "hi!"), "the-value-id"); - yield return new Quit(); + yield return new Quit("csharp"); + + yield return new Cancel("csharp"); } } diff --git a/src/Microsoft.DotNet.Interactive/Commands/Cancel.cs b/src/Microsoft.DotNet.Interactive/Commands/Cancel.cs new file mode 100644 index 0000000000..56ed79765d --- /dev/null +++ b/src/Microsoft.DotNet.Interactive/Commands/Cancel.cs @@ -0,0 +1,15 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System.Threading.Tasks; + +namespace Microsoft.DotNet.Interactive.Commands +{ + public class Cancel : KernelCommand + { + public Cancel(string targetKernelName = null): base(targetKernelName) + { + Handler = (command, context) => Task.CompletedTask; + } + } +} \ No newline at end of file diff --git a/src/Microsoft.DotNet.Interactive/Commands/Quit.cs b/src/Microsoft.DotNet.Interactive/Commands/Quit.cs index 88adbf5da3..66de0765b3 100644 --- a/src/Microsoft.DotNet.Interactive/Commands/Quit.cs +++ b/src/Microsoft.DotNet.Interactive/Commands/Quit.cs @@ -11,7 +11,7 @@ namespace Microsoft.DotNet.Interactive.Commands { public class Quit : KernelCommand { - private static readonly CompositeDisposable DisposeOnQuit = new CompositeDisposable(); + private static readonly CompositeDisposable DisposeOnQuit = new(); public static void RegisterForDisposalOnQuit(IDisposable disposable) { diff --git a/src/Microsoft.DotNet.Interactive/CompositeKernel.cs b/src/Microsoft.DotNet.Interactive/CompositeKernel.cs index c0e94d2803..fe117be2cf 100644 --- a/src/Microsoft.DotNet.Interactive/CompositeKernel.cs +++ b/src/Microsoft.DotNet.Interactive/CompositeKernel.cs @@ -215,8 +215,11 @@ internal override async Task HandleAsync( switch (command) { + case Cancel _: case Quit _: + CancelInflightCommand(); ClearPendingCommands(); + kernel.CancelInflightCommand(); kernel.ClearPendingCommands(); break; } diff --git a/src/Microsoft.DotNet.Interactive/Kernel.cs b/src/Microsoft.DotNet.Interactive/Kernel.cs index 23664f83aa..054fac39bf 100644 --- a/src/Microsoft.DotNet.Interactive/Kernel.cs +++ b/src/Microsoft.DotNet.Interactive/Kernel.cs @@ -24,14 +24,12 @@ namespace Microsoft.DotNet.Interactive { public abstract partial class Kernel : IDisposable { - private readonly Subject _kernelEvents = new Subject(); + private readonly Subject _kernelEvents = new(); private readonly CompositeDisposable _disposables; - private readonly ConcurrentQueue _deferredCommands = new ConcurrentQueue(); + private readonly ConcurrentQueue _deferredCommands = new(); - private readonly ConcurrentQueue _commandQueue = - new ConcurrentQueue(); - private readonly Dictionary _dynamicHandlers = - new Dictionary(); + private readonly ConcurrentQueue _commandQueue = new(); + private readonly Dictionary _dynamicHandlers = new(); private FrontendEnvironment _frontendEnvironment; private ChooseKernelDirective _chooseKernelDirective; @@ -141,15 +139,9 @@ private IReadOnlyList PreprocessCommands(KernelCommand command) { return command switch { - SubmitCode submitCode - when submitCode.LanguageNode is null => SubmissionParser.SplitSubmission(submitCode), - - RequestDiagnostics requestDiagnostics - when requestDiagnostics.LanguageNode is null => SubmissionParser.SplitSubmission(requestDiagnostics), - - LanguageServiceCommand languageServiceCommand - when languageServiceCommand.LanguageNode is null => PreprocessLanguageServiceCommand(languageServiceCommand), - + SubmitCode {LanguageNode: null} submitCode => SubmissionParser.SplitSubmission(submitCode), + RequestDiagnostics {LanguageNode: null} requestDiagnostics => SubmissionParser.SplitSubmission(requestDiagnostics), + LanguageServiceCommand {LanguageNode: null} languageServiceCommand => PreprocessLanguageServiceCommand(languageServiceCommand), _ => new[] { command } }; } @@ -236,11 +228,11 @@ private class KernelOperation public KernelOperation( KernelCommand command, TaskCompletionSource taskCompletionSource, - bool IsDeferredOperation) + bool isDeferred) { Command = command; TaskCompletionSource = taskCompletionSource; - this.IsDeferredOperation = IsDeferredOperation; + IsDeferred = isDeferred; AsyncContext.TryEstablish(out var id); AsyncContextId = id; @@ -249,7 +241,7 @@ public KernelOperation( public KernelCommand Command { get; } public TaskCompletionSource TaskCompletionSource { get; } - public bool IsDeferredOperation { get; } + public bool IsDeferred { get; } public int AsyncContextId { get; } } @@ -322,6 +314,8 @@ internal Task SendAsync( switch (command) { case Quit _: + case Cancel _: + CancelInflightCommand(); ClearPendingCommands(); _commandQueue.Enqueue(operation); break; @@ -358,25 +352,27 @@ private void ProcessCommandQueue( } } - internal void ClearPendingCommands() + internal void CancelInflightCommand() { using var disposables = new CompositeDisposable(); KernelInvocationContext currentContext = null; - var inFlightOperation = _commandQueue.FirstOrDefault(c => !c.IsDeferredOperation); + var inFlightOperation = _commandQueue.FirstOrDefault(operation => !operation.IsDeferred); if (inFlightOperation is not null - ) + ) { - currentContext = KernelInvocationContext.Current?? KernelInvocationContext.Establish(inFlightOperation.Command); - disposables.Add( currentContext.KernelEvents.Subscribe(PublishEvent)); + currentContext = KernelInvocationContext.Current ?? KernelInvocationContext.Establish(inFlightOperation.Command); + disposables.Add(currentContext.KernelEvents.Subscribe(PublishEvent)); inFlightOperation.TaskCompletionSource.SetResult(currentContext.Result); } - _deferredCommands.Clear(); - _commandQueue.Clear(); - currentContext?.Fail(new OperationCanceledException("Command has been cancelled")); } + internal void ClearPendingCommands() + { + _deferredCommands.Clear(); + _commandQueue.Clear(); + } internal Task RunDeferredCommandsAsync() { diff --git a/src/Microsoft.DotNet.Interactive/Server/KernelCommandEnvelope.cs b/src/Microsoft.DotNet.Interactive/Server/KernelCommandEnvelope.cs index fa337b30a1..74969fa4e2 100644 --- a/src/Microsoft.DotNet.Interactive/Server/KernelCommandEnvelope.cs +++ b/src/Microsoft.DotNet.Interactive/Server/KernelCommandEnvelope.cs @@ -75,7 +75,8 @@ public static void ResetToDefaults() [nameof(SerializeNotebook)] = typeof(KernelCommandEnvelope), [nameof(SubmitCode)] = typeof(KernelCommandEnvelope), [nameof(UpdateDisplayedValue)] = typeof(KernelCommandEnvelope), - [nameof(Quit)] = typeof(KernelCommandEnvelope) + [nameof(Quit)] = typeof(KernelCommandEnvelope), + [nameof(Cancel)] = typeof(KernelCommandEnvelope) }; _commandTypesByCommandTypeName = new ConcurrentDictionary(_envelopeTypesByCommandTypeName From 8a873c574565dcc682899ec31b063a39aa7f9f31 Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Fri, 12 Feb 2021 01:22:52 +0000 Subject: [PATCH 08/44] updated typescript contract files --- .../src/dotnet-interactive/contracts.ts | 5 +++++ .../src/interfaces/src/contracts.ts | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/src/Microsoft.DotNet.Interactive.Js/src/dotnet-interactive/contracts.ts b/src/Microsoft.DotNet.Interactive.Js/src/dotnet-interactive/contracts.ts index b1aa8d0b46..0594cce8cb 100644 --- a/src/Microsoft.DotNet.Interactive.Js/src/dotnet-interactive/contracts.ts +++ b/src/Microsoft.DotNet.Interactive.Js/src/dotnet-interactive/contracts.ts @@ -6,6 +6,7 @@ // --------------------------------------------- Kernel Commands export const AddPackageType = "AddPackage"; +export const CancelType = "Cancel"; export const ChangeWorkingDirectoryType = "ChangeWorkingDirectory"; export const DisplayErrorType = "DisplayError"; export const DisplayValueType = "DisplayValue"; @@ -21,6 +22,7 @@ export const UpdateDisplayedValueType = "UpdateDisplayedValue"; export type KernelCommandType = typeof AddPackageType + | typeof CancelType | typeof ChangeWorkingDirectoryType | typeof DisplayErrorType | typeof DisplayValueType @@ -42,6 +44,9 @@ export interface KernelCommand { targetKernelName?: string; } +export interface Cancel extends KernelCommand { +} + export interface ChangeWorkingDirectory extends KernelCommand { workingDirectory: string; } diff --git a/src/dotnet-interactive-vscode/src/interfaces/src/contracts.ts b/src/dotnet-interactive-vscode/src/interfaces/src/contracts.ts index b1aa8d0b46..0594cce8cb 100644 --- a/src/dotnet-interactive-vscode/src/interfaces/src/contracts.ts +++ b/src/dotnet-interactive-vscode/src/interfaces/src/contracts.ts @@ -6,6 +6,7 @@ // --------------------------------------------- Kernel Commands export const AddPackageType = "AddPackage"; +export const CancelType = "Cancel"; export const ChangeWorkingDirectoryType = "ChangeWorkingDirectory"; export const DisplayErrorType = "DisplayError"; export const DisplayValueType = "DisplayValue"; @@ -21,6 +22,7 @@ export const UpdateDisplayedValueType = "UpdateDisplayedValue"; export type KernelCommandType = typeof AddPackageType + | typeof CancelType | typeof ChangeWorkingDirectoryType | typeof DisplayErrorType | typeof DisplayValueType @@ -42,6 +44,9 @@ export interface KernelCommand { targetKernelName?: string; } +export interface Cancel extends KernelCommand { +} + export interface ChangeWorkingDirectory extends KernelCommand { workingDirectory: string; } From 73e72b2fff626e6a1436fcfd4d981c1536e29fcb Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Fri, 12 Feb 2021 14:16:57 +0000 Subject: [PATCH 09/44] implementing cancel command cancel all active context that are not executing quit or cancel make sure all commands in the queue are cancelled --- .../LanguageKernelTests.cs | 201 +++++++++++++++--- .../CompositeKernel.cs | 9 +- src/Microsoft.DotNet.Interactive/Kernel.cs | 53 +++-- .../KernelInvocationContext.cs | 37 +++- 4 files changed, 242 insertions(+), 58 deletions(-) diff --git a/src/Microsoft.DotNet.Interactive.Tests/LanguageKernelTests.cs b/src/Microsoft.DotNet.Interactive.Tests/LanguageKernelTests.cs index 331517bfd1..fcd291eb76 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/LanguageKernelTests.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/LanguageKernelTests.cs @@ -7,13 +7,16 @@ using System.Linq; using System.Reactive.Linq; using System.Threading.Tasks; + using FluentAssertions; using FluentAssertions.Execution; using FluentAssertions.Extensions; + using Microsoft.DotNet.Interactive.Commands; using Microsoft.DotNet.Interactive.Events; using Microsoft.DotNet.Interactive.Formatting; using Microsoft.DotNet.Interactive.Tests.Utility; + using Xunit; using Xunit.Abstractions; @@ -294,9 +297,9 @@ public async Task when_code_contains_compile_time_error_diagnostics_are_produced .Which .Diagnostics .Should() - .ContainSingle(diag => - diag.LinePositionSpan == diagnosticRange && - diag.Code == code && + .ContainSingle(diag => + diag.LinePositionSpan == diagnosticRange && + diag.Code == code && diag.Message == diagnosticMessage); // The FormattedValues are populated of DiagnosticsProduced event are populated @@ -374,7 +377,7 @@ public async Task powershell_produces_diagnostics_from_parse_errors() .Should() .ContainSingle(d => d.LinePositionSpan == diagnosticRange && - d.Code == "ExpectedExpression" && + d.Code == "ExpectedExpression" && d.Message == "An expression was expected after '('."); KernelEvents @@ -1038,7 +1041,7 @@ open System .Select(e => e.Value as StandardOutputValueProduced) .SelectMany(e => e.FormattedValues.Select(v => v.Value)) .Should() - .BeEquivalentTo(new [] {"1", "2"}); + .BeEquivalentTo(new[] { "1", "2" }); } @@ -1222,7 +1225,7 @@ public async Task SetVariableAsync_declares_the_specified_variable(Language lang succeeded.Should().BeTrue(); x.Should().Be(123); } - + [Theory] [InlineData(Language.CSharp)] [InlineData(Language.FSharp)] @@ -1265,38 +1268,39 @@ public async Task SetVariableAsync_can_redeclare_an_existing_variable_and_change x.Should().Be("hello"); } + [Theory] - [InlineData(Language.CSharp,"System.Threading.Thread.Sleep(3000);")] + [InlineData(Language.CSharp, "System.Threading.Thread.Sleep(3000);")] [InlineData(Language.FSharp, "System.Threading.Thread.Sleep(3000)")] [InlineData(Language.PowerShell, "Start-Sleep -Milliseconds 3000", Skip = "to address later")] - public void Quit_command_cause_current_command_to_fail(Language language, string code) + public void Quit_command_is_handled(Language language, string code) { var kernel = CreateKernel(language); - var languageKernel = kernel.ChildKernels.OfType().Single(); var quitCommandExecuted = false; - var quitCommand = new Quit(() => { + var quitCommand = new Quit(() => + { quitCommandExecuted = true; }); var submitCodeCommand = new SubmitCode(code); - Task.WhenAll( - Task.Run(async () => - { - await Task.Delay(20); - await languageKernel.SendAsync(submitCodeCommand); - }), - Task.Run(async () => - { - await Task.Delay(100); - await languageKernel.SendAsync(quitCommand); - })) + Task.WhenAll( + Task.Run(async () => + { + await Task.Delay(20); + await kernel.SendAsync(submitCodeCommand); + }), + Task.Run(async () => + { + await Task.Delay(100); + await kernel.SendAsync(quitCommand); + })) .Wait(TimeSpan.FromSeconds(20)); using var _ = new AssertionScope(); - + quitCommandExecuted.Should().BeTrue(); KernelEvents @@ -1306,32 +1310,165 @@ public void Quit_command_cause_current_command_to_fail(Language language, string .Command .Should() .Be(quitCommand); + } + + [Theory] + [InlineData(Language.CSharp, "System.Threading.Thread.Sleep(3000);")] + [InlineData(Language.FSharp, "System.Threading.Thread.Sleep(3000)")] + [InlineData(Language.PowerShell, "Start-Sleep -Milliseconds 3000", Skip = "to address later")] + public void Quit_command_cause_the_running_command_to_fail(Language language, string code) + { + var kernel = CreateKernel(language); + + var quitCommand = new Quit(() => { }); + + var submitCodeCommand = new SubmitCode(code); + + Task.WhenAll( + Task.Run(async () => + { + await Task.Delay(20); + await kernel.SendAsync(submitCodeCommand); + }), + Task.Run(async () => + { + await Task.Delay(100); + await kernel.SendAsync(quitCommand); + })) + .Wait(TimeSpan.FromSeconds(20)); + + using var _ = new AssertionScope(); KernelEvents .Should() - .ContainSingle() + .ContainSingle(c => c.Command == submitCodeCommand) .Which .Exception .Should() .BeOfType(); } - [Fact] - public void user_code_can_react_to_cancel_command_using_cancellation_token() + + [Theory] + [InlineData(Language.CSharp, "await Task.Delay(Microsoft.DotNet.Interactive.KernelInvocationContext.Current.Token); Console.WriteLine(\"done c#\")", "done c#")] + [InlineData(Language.FSharp, @" +System.Threading.Thread.Sleep(3000) +Console.WriteLine(""done c#"")", "done f#")] + public async Task user_code_can_react_to_cancel_command_using_cancellation_token(Language language, string code, string expectedValue) { - throw new NotImplementedException(); + var kernel = CreateKernel(language); + var cancelCommand = new Cancel(); + var submitCodeCommand = new SubmitCode(code); + + Task.Run(async () => await kernel.SendAsync(submitCodeCommand)); + await Task.Delay(100); + await kernel.SendAsync(cancelCommand); + + + KernelEvents + .Should() + .ContainSingle() + .Which + .Value + .Should() + .Be(expectedValue); + } - [Fact] - public void cancel_command_cancels_the_running_command() + [Theory] + [InlineData(Language.CSharp)] + [InlineData(Language.FSharp)] + [InlineData(Language.PowerShell, Skip = "to address later")] + public void cancel_command_cancels_the_running_command(Language language) { - throw new NotImplementedException(); + var submitCodeCommandCancelled = false; + var submitCodeCommandStarted = false; + var kernel = CreateKernel(language); + + var cancelCommand = new Cancel(); + + var submitCodeCommand = new SubmitCode("placeholder") + { + Handler = (command, context) => + { + submitCodeCommandStarted = true; + + while (!context.CancellationToken.IsCancellationRequested) + { + + } + + submitCodeCommandCancelled = true; + + return Task.CompletedTask; + } + }; + + Task.WhenAll( + Task.Run(async () => + { + await kernel.SendAsync(submitCodeCommand); + }), + Task.Run(async () => + { + await Task.Delay(100); + await kernel.SendAsync(cancelCommand); + })).Wait(); + // .Wait(TimeSpan.FromSeconds(20)); + + + using var _ = new AssertionScope(); + + submitCodeCommandStarted.Should().BeTrue(); + submitCodeCommandCancelled.Should().BeTrue(); + + KernelEvents + .Should() + .ContainSingle(c => c.Command == submitCodeCommand) + .Which + .Exception + .Should() + .BeOfType(); } - [Fact] - public void cancel_command_cancels_all_deferred_commands() + [Theory] + [InlineData(Language.CSharp)] + [InlineData(Language.FSharp)] + public async Task cancel_command_cancels_all_deferred_commands(Language language) { - throw new NotImplementedException(); + var deferredCommandExecuted = false; + + var kernel = CreateKernel(language); + var languageKernel = kernel.ChildKernels.OfType().Single(); + + + var deferred = new SubmitCode("placeholder") + { + Handler = (command, context) => + { + deferredCommandExecuted = true; + return Task.CompletedTask; + } + }; + + var cancelCommand = new Cancel(); + + languageKernel.DeferCommand(deferred); + + var events = kernel.KernelEvents.ToSubscribedList(); + + await kernel.SendAsync(cancelCommand); + + using var _ = new AssertionScope(); + + deferredCommandExecuted.Should().BeFalse(); + + events + .Should().ContainSingle() + .Which + .Command + .Should() + .Be(cancelCommand); } } } diff --git a/src/Microsoft.DotNet.Interactive/CompositeKernel.cs b/src/Microsoft.DotNet.Interactive/CompositeKernel.cs index fe117be2cf..3d1fb4907f 100644 --- a/src/Microsoft.DotNet.Interactive/CompositeKernel.cs +++ b/src/Microsoft.DotNet.Interactive/CompositeKernel.cs @@ -216,10 +216,15 @@ internal override async Task HandleAsync( switch (command) { case Cancel _: + + CancelInflightCommands(); + ClearPendingCommands(); + kernel.ClearPendingCommands(); + break; case Quit _: - CancelInflightCommand(); + CancelInflightCommands(); ClearPendingCommands(); - kernel.CancelInflightCommand(); + kernel.CancelInflightCommands(); kernel.ClearPendingCommands(); break; } diff --git a/src/Microsoft.DotNet.Interactive/Kernel.cs b/src/Microsoft.DotNet.Interactive/Kernel.cs index 054fac39bf..683188fc80 100644 --- a/src/Microsoft.DotNet.Interactive/Kernel.cs +++ b/src/Microsoft.DotNet.Interactive/Kernel.cs @@ -313,18 +313,19 @@ internal Task SendAsync( switch (command) { - case Quit _: case Cancel _: - CancelInflightCommand(); + CancelInflightCommands(); + ClearPendingCommands(); + break; + case Quit _: + CancelInflightCommands(); ClearPendingCommands(); - _commandQueue.Enqueue(operation); break; default: UndeferCommands(); - _commandQueue.Enqueue(operation); break; } - + _commandQueue.Enqueue(operation); ProcessCommandQueue(_commandQueue, cancellationToken, onDone); return tcs.Task; @@ -352,22 +353,44 @@ private void ProcessCommandQueue( } } - internal void CancelInflightCommand() + private static bool CanCancel(KernelCommand command) { - using var disposables = new CompositeDisposable(); - KernelInvocationContext currentContext = null; - var inFlightOperation = _commandQueue.FirstOrDefault(operation => !operation.IsDeferred); - if (inFlightOperation is not null - ) + return command switch + { + Quit _ => false, + Cancel _ => false, + _ => true + }; + } + + + internal void CancelInflightCommands() + { + foreach (var kernelInvocationContext in KernelInvocationContext.ActiveContexts.Where(c => !c.IsComplete && CanCancel(c.Command))) { - currentContext = KernelInvocationContext.Current ?? KernelInvocationContext.Establish(inFlightOperation.Command); - disposables.Add(currentContext.KernelEvents.Subscribe(PublishEvent)); - inFlightOperation.TaskCompletionSource.SetResult(currentContext.Result); + kernelInvocationContext.Cancel(); + } - currentContext?.Fail(new OperationCanceledException("Command has been cancelled")); + using var disposables = new CompositeDisposable(); + var inFlightOperations = _commandQueue.Where(operation => !operation.IsDeferred && CanCancel(operation.Command)).ToList(); + foreach (var inFlightOperation in inFlightOperations) + { + KernelInvocationContext currentContext = null; + + if (inFlightOperation is not null + ) + { + currentContext = KernelInvocationContext.Establish(inFlightOperation.Command); + disposables.Add(currentContext.KernelEvents.Subscribe(PublishEvent)); + inFlightOperation.TaskCompletionSource.SetResult(currentContext.Result); + } + + currentContext?.Cancel(); + } } + internal void ClearPendingCommands() { _deferredCommands.Clear(); diff --git a/src/Microsoft.DotNet.Interactive/KernelInvocationContext.cs b/src/Microsoft.DotNet.Interactive/KernelInvocationContext.cs index 56144df568..206827f3cb 100644 --- a/src/Microsoft.DotNet.Interactive/KernelInvocationContext.cs +++ b/src/Microsoft.DotNet.Interactive/KernelInvocationContext.cs @@ -2,7 +2,9 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; +using System.Collections.Concurrent; using System.Collections.Generic; +using System.Linq; using System.Reactive.Disposables; using System.Reactive.Subjects; using System.Threading; @@ -16,15 +18,16 @@ namespace Microsoft.DotNet.Interactive { public class KernelInvocationContext : IAsyncDisposable { - private static readonly AsyncLocal _current = new AsyncLocal(); + internal static ConcurrentBag ActiveContexts { get; private set; } = new(); + private static readonly AsyncLocal _current = new(); - private readonly ReplaySubject _events = new ReplaySubject(); + private readonly ReplaySubject _events = new(); - private readonly HashSet _childCommands = new HashSet(); + private readonly HashSet _childCommands = new(); - private readonly CompositeDisposable _disposables = new CompositeDisposable(); + private readonly CompositeDisposable _disposables = new(); - private readonly List> _onCompleteActions = new List>(); + private readonly List> _onCompleteActions = new(); private readonly CancellationTokenSource _cancellationTokenSource; @@ -71,15 +74,27 @@ public void Complete(KernelCommand command) } } + public void Cancel() + { + if (!IsComplete) + { + _cancellationTokenSource.Cancel(false); + Fail(new OperationCanceledException($"Command :{Command} cancelled.")); + } + } + public void Fail( Exception exception = null, string message = null) { - Publish(new CommandFailed(exception, Command, message)); + if (!IsComplete) + { + Publish(new CommandFailed(exception, Command, message)); - _events.OnCompleted(); - _cancellationTokenSource.Cancel(false); - IsComplete = true; + _events.OnCompleted(); + _cancellationTokenSource.Cancel(false); + IsComplete = true; + } } public void OnComplete(Func onComplete) @@ -113,6 +128,8 @@ public static KernelInvocationContext Establish(KernelCommand command) if (_current.Value == null || _current.Value.IsComplete) { var context = new KernelInvocationContext(command); + + ActiveContexts.Add(context); _current.Value = context; } @@ -140,6 +157,8 @@ public ValueTask DisposeAsync() { if (_current.Value is { } active) { + ActiveContexts = + new ConcurrentBag(ActiveContexts.Except(new[] {_current.Value})); _current.Value = null; if (_onCompleteActions.Count > 0) From 34b41d0dcacc118d98416dd59721fe2141802a1e Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Fri, 12 Feb 2021 15:48:19 +0000 Subject: [PATCH 10/44] fix file --- .../RecordTranscriptExtension.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.DotNet.Interactive.ExtensionLab/RecordTranscriptExtension.cs b/src/Microsoft.DotNet.Interactive.ExtensionLab/RecordTranscriptExtension.cs index 77a2122c9d..fc9008bc35 100644 --- a/src/Microsoft.DotNet.Interactive.ExtensionLab/RecordTranscriptExtension.cs +++ b/src/Microsoft.DotNet.Interactive.ExtensionLab/RecordTranscriptExtension.cs @@ -1,9 +1,11 @@ -using System; +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + using System.CommandLine; using System.CommandLine.Invocation; using System.IO; -using System.Linq; using System.Threading.Tasks; + using Microsoft.DotNet.Interactive.Server; namespace Microsoft.DotNet.Interactive.ExtensionLab From 04bea52082c1941a917700251af11325dc712d3b Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Sun, 14 Feb 2021 19:01:36 +0000 Subject: [PATCH 11/44] fix code --- src/Microsoft.DotNet.Interactive/KernelInvocationContext.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Microsoft.DotNet.Interactive/KernelInvocationContext.cs b/src/Microsoft.DotNet.Interactive/KernelInvocationContext.cs index 206827f3cb..7f26e3e779 100644 --- a/src/Microsoft.DotNet.Interactive/KernelInvocationContext.cs +++ b/src/Microsoft.DotNet.Interactive/KernelInvocationContext.cs @@ -19,6 +19,7 @@ namespace Microsoft.DotNet.Interactive public class KernelInvocationContext : IAsyncDisposable { internal static ConcurrentBag ActiveContexts { get; private set; } = new(); + private static readonly AsyncLocal _current = new(); private readonly ReplaySubject _events = new(); From bc0a1fa56c6c3104e4ce601bdbe39ed309002f40 Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Tue, 16 Feb 2021 19:00:38 +0000 Subject: [PATCH 12/44] Update src/Microsoft.DotNet.Interactive.Tests/LanguageKernelTests.cs Co-authored-by: Jon Sequeira --- src/Microsoft.DotNet.Interactive.Tests/LanguageKernelTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.DotNet.Interactive.Tests/LanguageKernelTests.cs b/src/Microsoft.DotNet.Interactive.Tests/LanguageKernelTests.cs index fcd291eb76..c850a63355 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/LanguageKernelTests.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/LanguageKernelTests.cs @@ -1316,7 +1316,7 @@ public void Quit_command_is_handled(Language language, string code) [InlineData(Language.CSharp, "System.Threading.Thread.Sleep(3000);")] [InlineData(Language.FSharp, "System.Threading.Thread.Sleep(3000)")] [InlineData(Language.PowerShell, "Start-Sleep -Milliseconds 3000", Skip = "to address later")] - public void Quit_command_cause_the_running_command_to_fail(Language language, string code) + public void Quit_command_causes_the_running_command_to_fail(Language language, string code) { var kernel = CreateKernel(language); From 24d26695385f3d73bab37e83b7ae1bb77e65c3c4 Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Tue, 16 Feb 2021 20:59:51 +0000 Subject: [PATCH 13/44] quit default behaviour throws if not configured --- .../CompositeKernelTests.cs | 26 +++---- .../LanguageKernelTests.cs | 11 +-- .../QuitCommandTests.cs | 70 +++++++++++++++++++ .../Commands/Quit.cs | 39 +++++------ .../KernelExtensions.cs | 7 +- 5 files changed, 111 insertions(+), 42 deletions(-) create mode 100644 src/Microsoft.DotNet.Interactive.Tests/QuitCommandTests.cs diff --git a/src/Microsoft.DotNet.Interactive.Tests/CompositeKernelTests.cs b/src/Microsoft.DotNet.Interactive.Tests/CompositeKernelTests.cs index 85b4c957d3..450354312b 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/CompositeKernelTests.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/CompositeKernelTests.cs @@ -22,7 +22,7 @@ namespace Microsoft.DotNet.Interactive.Tests { public class CompositeKernelTests : IDisposable { - private readonly CompositeDisposable _disposables = new CompositeDisposable(); + private readonly CompositeDisposable _disposables = new(); public CompositeKernelTests(ITestOutputHelper output) { @@ -426,11 +426,13 @@ public async Task Deferred_commands_on_composite_kernel_are_execute_on_first_sub typeof(CommandSucceeded)); } + + [Fact] public async Task quit_command_cancels_all_deferred_commands_on_composite_kernel() { var deferredCommandExecuted = false; - var quitCommandHandled = false; + var quitCommandExecuted = false; var subKernel = new CSharpKernel(); @@ -451,10 +453,9 @@ public async Task quit_command_cancels_all_deferred_commands_on_composite_kernel }; - var quitCommand = new Quit(() => - { - quitCommandHandled = true; - }); + Quit.OnQuit(() => { quitCommandExecuted = true; }); + + var quitCommand = new Quit(); compositeKernel.DeferCommand(deferred); @@ -465,7 +466,7 @@ public async Task quit_command_cancels_all_deferred_commands_on_composite_kernel using var _ = new AssertionScope(); deferredCommandExecuted.Should().BeFalse(); - quitCommandHandled.Should().BeTrue(); + quitCommandExecuted.Should().BeTrue(); events .Should().ContainSingle() @@ -479,7 +480,7 @@ public async Task quit_command_cancels_all_deferred_commands_on_composite_kernel public async Task quit_command_cancels_all_deferred_commands_on_subkernels() { var deferredCommandExecuted = false; - var quitCommandHandled = false; + var quitCommandExecuted = false; var subKernel = new CSharpKernel(); @@ -499,10 +500,9 @@ public async Task quit_command_cancels_all_deferred_commands_on_subkernels() } }; - var quitCommand = new Quit(() => - { - quitCommandHandled = true; - }); + Quit.OnQuit(() => { quitCommandExecuted = true; }); + + var quitCommand = new Quit(); subKernel.DeferCommand(deferred); @@ -513,7 +513,7 @@ public async Task quit_command_cancels_all_deferred_commands_on_subkernels() using var _ = new AssertionScope(); deferredCommandExecuted.Should().BeFalse(); - quitCommandHandled.Should().BeTrue(); + quitCommandExecuted.Should().BeTrue(); events .Should().ContainSingle() diff --git a/src/Microsoft.DotNet.Interactive.Tests/LanguageKernelTests.cs b/src/Microsoft.DotNet.Interactive.Tests/LanguageKernelTests.cs index c850a63355..74349f125a 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/LanguageKernelTests.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/LanguageKernelTests.cs @@ -1278,11 +1278,10 @@ public void Quit_command_is_handled(Language language, string code) var kernel = CreateKernel(language); var quitCommandExecuted = false; + + Quit.OnQuit(() => { quitCommandExecuted = true; }); - var quitCommand = new Quit(() => - { - quitCommandExecuted = true; - }); + var quitCommand = new Quit(); var submitCodeCommand = new SubmitCode(code); @@ -1320,7 +1319,9 @@ public void Quit_command_causes_the_running_command_to_fail(Language language, s { var kernel = CreateKernel(language); - var quitCommand = new Quit(() => { }); + Quit.OnQuit(() => { }); + + var quitCommand = new Quit(); var submitCodeCommand = new SubmitCode(code); diff --git a/src/Microsoft.DotNet.Interactive.Tests/QuitCommandTests.cs b/src/Microsoft.DotNet.Interactive.Tests/QuitCommandTests.cs new file mode 100644 index 0000000000..38e4d35d0f --- /dev/null +++ b/src/Microsoft.DotNet.Interactive.Tests/QuitCommandTests.cs @@ -0,0 +1,70 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; +using System.Threading.Tasks; +using FluentAssertions; +using FluentAssertions.Execution; +using Microsoft.DotNet.Interactive.Commands; +using Microsoft.DotNet.Interactive.CSharp; +using Microsoft.DotNet.Interactive.Events; +using Microsoft.DotNet.Interactive.Tests.Utility; +using Pocket; +using Xunit; +using Xunit.Abstractions; + +namespace Microsoft.DotNet.Interactive.Tests +{ + public class QuitCommandTests : IDisposable + { + private readonly CompositeDisposable _disposables = new(); + + public QuitCommandTests(ITestOutputHelper output) + { + _disposables.Add(output.SubscribeToPocketLogger()); + } + + public void Dispose() + { + _disposables.Dispose(); + } + + [Fact] + public async Task quit_command_fails_when_not_configured() + { + + var subKernel = new CSharpKernel(); + + using var compositeKernel = new CompositeKernel + { + subKernel + }; + + compositeKernel.DefaultKernelName = subKernel.Name; + + + + var quitCommand = new Quit(); + + var events = compositeKernel.KernelEvents.ToSubscribedList(); + + await compositeKernel.SendAsync(quitCommand); + + using var _ = new AssertionScope(); + + events + .Should().ContainSingle() + .Which + .Command + .Should() + .Be(quitCommand); + + events + .Should().ContainSingle() + .Which + .Exception + .Should() + .BeOfType(); + } + } +} \ No newline at end of file diff --git a/src/Microsoft.DotNet.Interactive/Commands/Quit.cs b/src/Microsoft.DotNet.Interactive/Commands/Quit.cs index 66de0765b3..3aaeb3a8fa 100644 --- a/src/Microsoft.DotNet.Interactive/Commands/Quit.cs +++ b/src/Microsoft.DotNet.Interactive/Commands/Quit.cs @@ -3,7 +3,6 @@ using System; -using System.Reactive.Disposables; using System.Text.Json.Serialization; using System.Threading.Tasks; @@ -11,37 +10,31 @@ namespace Microsoft.DotNet.Interactive.Commands { public class Quit : KernelCommand { - private static readonly CompositeDisposable DisposeOnQuit = new(); + private static Action _onQuit; + private static readonly Action DefaultOnQuit = () => throw new InvalidOperationException("Quit command is not configured"); - public static void RegisterForDisposalOnQuit(IDisposable disposable) + static Quit() { - if (disposable is not null) - { - DisposeOnQuit.Add(disposable); - } + _onQuit = DefaultOnQuit; + } + + public static void OnQuit(Action onQuit) + { + _onQuit = onQuit ?? DefaultOnQuit; } [JsonConstructor] - public Quit(string targetKernelName = null): this(() => - { - Environment.Exit(0); - }, targetKernelName) + public Quit(string targetKernelName = null): base(targetKernelName) { } - public Quit(Action onQuit, string targetKernelName = null) : base(targetKernelName) + public override Task InvokeAsync(KernelInvocationContext context) { - if (onQuit == null) - { - throw new ArgumentNullException(nameof(onQuit)); - } - Handler = (_, context) => - { - context?.Complete(context.Command); - DisposeOnQuit?.Dispose(); - onQuit(); - return Task.CompletedTask; - }; + _onQuit(); + context?.Complete(context.Command); + return Task.CompletedTask; } + + } } \ No newline at end of file diff --git a/src/Microsoft.DotNet.Interactive/KernelExtensions.cs b/src/Microsoft.DotNet.Interactive/KernelExtensions.cs index 28e2c9010b..d5d9859ad9 100644 --- a/src/Microsoft.DotNet.Interactive/KernelExtensions.cs +++ b/src/Microsoft.DotNet.Interactive/KernelExtensions.cs @@ -24,7 +24,12 @@ public static class KernelExtensions { public static T UseQuitCommand(this T kernel, IDisposable disposeOnQuit, CancellationToken cancellationToken) where T : Kernel { - Quit.RegisterForDisposalOnQuit(disposeOnQuit); + Quit.OnQuit(() => + { + disposeOnQuit?.Dispose(); + Environment.Exit(0); + }); + cancellationToken.Register(async () => { await kernel.SendAsync(new Quit()); From 7ca9c3e5cc55042ebfd2a5c9e0c26660dd17a80a Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Tue, 16 Feb 2021 21:08:13 +0000 Subject: [PATCH 14/44] update ncrunch settings --- .../dotnet-interactive.Tests.v3.ncrunchproject | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dotnet-interactive.Tests/dotnet-interactive.Tests.v3.ncrunchproject b/src/dotnet-interactive.Tests/dotnet-interactive.Tests.v3.ncrunchproject index 5ef144da36..aa6b1f8b1a 100644 --- a/src/dotnet-interactive.Tests/dotnet-interactive.Tests.v3.ncrunchproject +++ b/src/dotnet-interactive.Tests/dotnet-interactive.Tests.v3.ncrunchproject @@ -1,8 +1,8 @@  - ..\dotnet-interactive-vscode\src\contracts.ts ..\Microsoft.DotNet.Interactive.Js\src\dotnet-interactive\contracts.ts + ..\dotnet-interactive-vscode\src\interfaces\src\contracts.ts From 23e1ff71e83e4b4f0224cf30b5052a4ff5ef842e Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Tue, 16 Feb 2021 21:19:45 +0000 Subject: [PATCH 15/44] regroup tests for quit command --- .../CompositeKernelTests.cs | 95 ---------------- .../QuitCommandTests.cs | 103 ++++++++++++++---- 2 files changed, 84 insertions(+), 114 deletions(-) diff --git a/src/Microsoft.DotNet.Interactive.Tests/CompositeKernelTests.cs b/src/Microsoft.DotNet.Interactive.Tests/CompositeKernelTests.cs index 450354312b..88f3e44660 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/CompositeKernelTests.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/CompositeKernelTests.cs @@ -425,103 +425,8 @@ public async Task Deferred_commands_on_composite_kernel_are_execute_on_first_sub typeof(CompleteCodeSubmissionReceived), typeof(CommandSucceeded)); } - - [Fact] - public async Task quit_command_cancels_all_deferred_commands_on_composite_kernel() - { - var deferredCommandExecuted = false; - var quitCommandExecuted = false; - - var subKernel = new CSharpKernel(); - - using var compositeKernel = new CompositeKernel - { - subKernel - }; - - compositeKernel.DefaultKernelName = subKernel.Name; - - var deferred = new SubmitCode("placeholder") - { - Handler = (command, context) => - { - deferredCommandExecuted = true; - return Task.CompletedTask; - } - }; - - - Quit.OnQuit(() => { quitCommandExecuted = true; }); - - var quitCommand = new Quit(); - - compositeKernel.DeferCommand(deferred); - - var events = compositeKernel.KernelEvents.ToSubscribedList(); - - await compositeKernel.SendAsync(quitCommand); - - using var _ = new AssertionScope(); - - deferredCommandExecuted.Should().BeFalse(); - quitCommandExecuted.Should().BeTrue(); - - events - .Should().ContainSingle() - .Which - .Command - .Should() - .Be(quitCommand); - } - - [Fact] - public async Task quit_command_cancels_all_deferred_commands_on_subkernels() - { - var deferredCommandExecuted = false; - var quitCommandExecuted = false; - - var subKernel = new CSharpKernel(); - - using var compositeKernel = new CompositeKernel - { - subKernel - }; - - compositeKernel.DefaultKernelName = subKernel.Name; - - var deferred = new SubmitCode("placeholder") - { - Handler = (command, context) => - { - deferredCommandExecuted = true; - return Task.CompletedTask; - } - }; - - Quit.OnQuit(() => { quitCommandExecuted = true; }); - - var quitCommand = new Quit(); - - subKernel.DeferCommand(deferred); - - var events = compositeKernel.KernelEvents.ToSubscribedList(); - - await compositeKernel.SendAsync(quitCommand); - - using var _ = new AssertionScope(); - - deferredCommandExecuted.Should().BeFalse(); - quitCommandExecuted.Should().BeTrue(); - - events - .Should().ContainSingle() - .Which - .Command - .Should() - .Be(quitCommand); - } [Fact] public async Task cancel_command_cancels_all_deferred_commands_on_composite_kernel() diff --git a/src/Microsoft.DotNet.Interactive.Tests/QuitCommandTests.cs b/src/Microsoft.DotNet.Interactive.Tests/QuitCommandTests.cs index 38e4d35d0f..09150633f2 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/QuitCommandTests.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/QuitCommandTests.cs @@ -15,56 +15,121 @@ namespace Microsoft.DotNet.Interactive.Tests { - public class QuitCommandTests : IDisposable + public class QuitCommandTests : LanguageKernelTestBase { - private readonly CompositeDisposable _disposables = new(); - - public QuitCommandTests(ITestOutputHelper output) + public QuitCommandTests(ITestOutputHelper output) : base(output) { - _disposables.Add(output.SubscribeToPocketLogger()); } - public void Dispose() + [Fact] + public async Task quit_command_fails_when_not_configured() { - _disposables.Dispose(); + var kernel = CreateKernel(); + + var quitCommand = new Quit(); + + var events = kernel.KernelEvents.ToSubscribedList(); + + await kernel.SendAsync(quitCommand); + + using var _ = new AssertionScope(); + + events + .Should().ContainSingle() + .Which + .Command + .Should() + .Be(quitCommand); + + events + .Should().ContainSingle() + .Which + .Exception + .Should() + .BeOfType(); } [Fact] - public async Task quit_command_fails_when_not_configured() + public async Task quit_command_cancels_all_deferred_commands_on_composite_kernel() { + var deferredCommandExecuted = false; + + var quitCommandExecuted = false; - var subKernel = new CSharpKernel(); + var kernel = CreateKernel(); - using var compositeKernel = new CompositeKernel + var deferred = new SubmitCode("placeholder") { - subKernel + Handler = (command, context) => + { + deferredCommandExecuted = true; + return Task.CompletedTask; + } }; - compositeKernel.DefaultKernelName = subKernel.Name; - + Quit.OnQuit(() => { quitCommandExecuted = true; }); var quitCommand = new Quit(); - var events = compositeKernel.KernelEvents.ToSubscribedList(); + kernel.DeferCommand(deferred); - await compositeKernel.SendAsync(quitCommand); + var events = kernel.KernelEvents.ToSubscribedList(); + + await kernel.SendAsync(quitCommand); using var _ = new AssertionScope(); + deferredCommandExecuted.Should().BeFalse(); + quitCommandExecuted.Should().BeTrue(); + events - .Should().ContainSingle() + .Should().ContainSingle() .Which .Command .Should() .Be(quitCommand); + } + + [Fact] + public async Task quit_command_cancels_all_deferred_commands_on_subkernels() + { + var deferredCommandExecuted = false; + + var quitCommandExecuted = false; + + var kernel = CreateKernel(); + + var deferred = new SubmitCode("placeholder") + { + Handler = (command, context) => + { + deferredCommandExecuted = true; + return Task.CompletedTask; + } + }; + + Quit.OnQuit(() => { quitCommandExecuted = true; }); + + var quitCommand = new Quit(); + + kernel.ChildKernels[0].DeferCommand(deferred); + + var events = kernel.KernelEvents.ToSubscribedList(); + + await kernel.SendAsync(quitCommand); + + using var _ = new AssertionScope(); + + deferredCommandExecuted.Should().BeFalse(); + quitCommandExecuted.Should().BeTrue(); events - .Should().ContainSingle() + .Should().ContainSingle() .Which - .Exception + .Command .Should() - .BeOfType(); + .Be(quitCommand); } } } \ No newline at end of file From 925d26a262e0cf901f215ddc3910d077fa83cb92 Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Tue, 16 Feb 2021 21:23:04 +0000 Subject: [PATCH 16/44] split tests --- .../CancelCommandTests.cs | 92 +++++++++++++++++++ .../CompositeKernelTests.cs | 87 ------------------ .../QuitCommandTests.cs | 10 +- 3 files changed, 94 insertions(+), 95 deletions(-) create mode 100644 src/Microsoft.DotNet.Interactive.Tests/CancelCommandTests.cs diff --git a/src/Microsoft.DotNet.Interactive.Tests/CancelCommandTests.cs b/src/Microsoft.DotNet.Interactive.Tests/CancelCommandTests.cs new file mode 100644 index 0000000000..6f2d34c264 --- /dev/null +++ b/src/Microsoft.DotNet.Interactive.Tests/CancelCommandTests.cs @@ -0,0 +1,92 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System.Threading.Tasks; +using FluentAssertions; +using FluentAssertions.Execution; +using Microsoft.DotNet.Interactive.Commands; +using Microsoft.DotNet.Interactive.Events; +using Microsoft.DotNet.Interactive.Tests.Utility; +using Xunit; +using Xunit.Abstractions; + +namespace Microsoft.DotNet.Interactive.Tests +{ + public class CancelCommandTests : LanguageKernelTestBase + { + public CancelCommandTests(ITestOutputHelper output) : base(output) + { + } + + [Fact] + public async Task cancel_command_cancels_all_deferred_commands_on_composite_kernel() + { + var deferredCommandExecuted = false; + + var kernel = CreateKernel(); + + var deferred = new SubmitCode("placeholder") + { + Handler = (command, context) => + { + deferredCommandExecuted = true; + return Task.CompletedTask; + } + }; + + var cancelCommand = new Cancel(); + + kernel.DeferCommand(deferred); + + var events = kernel.KernelEvents.ToSubscribedList(); + + await kernel.SendAsync(cancelCommand); + + using var _ = new AssertionScope(); + + deferredCommandExecuted.Should().BeFalse(); + + events + .Should().ContainSingle() + .Which + .Command + .Should() + .Be(cancelCommand); + } + + [Fact] + public async Task cancel_command_cancels_all_deferred_commands_on_subkernels() + { + var deferredCommandExecuted = false; + + var kernel = CreateKernel(); + + var deferred = new SubmitCode("placeholder") + { + Handler = (command, context) => + { + deferredCommandExecuted = true; + return Task.CompletedTask; + } + }; + + var cancelCommand = new Cancel(); + + kernel.ChildKernels[0].DeferCommand(deferred); + + await kernel.SendAsync(cancelCommand); + + using var _ = new AssertionScope(); + + deferredCommandExecuted.Should().BeFalse(); + + KernelEvents + .Should().ContainSingle() + .Which + .Command + .Should() + .Be(cancelCommand); + } + + } +} \ No newline at end of file diff --git a/src/Microsoft.DotNet.Interactive.Tests/CompositeKernelTests.cs b/src/Microsoft.DotNet.Interactive.Tests/CompositeKernelTests.cs index 88f3e44660..7d31537183 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/CompositeKernelTests.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/CompositeKernelTests.cs @@ -427,93 +427,6 @@ public async Task Deferred_commands_on_composite_kernel_are_execute_on_first_sub } - - [Fact] - public async Task cancel_command_cancels_all_deferred_commands_on_composite_kernel() - { - var deferredCommandExecuted = false; - - var subKernel = new CSharpKernel(); - - using var compositeKernel = new CompositeKernel - { - subKernel - }; - - compositeKernel.DefaultKernelName = subKernel.Name; - - var deferred = new SubmitCode("placeholder") - { - Handler = (command, context) => - { - deferredCommandExecuted = true; - return Task.CompletedTask; - } - }; - - var cancelCommand = new Cancel(); - - compositeKernel.DeferCommand(deferred); - - var events = compositeKernel.KernelEvents.ToSubscribedList(); - - await compositeKernel.SendAsync(cancelCommand); - - using var _ = new AssertionScope(); - - deferredCommandExecuted.Should().BeFalse(); - - events - .Should().ContainSingle() - .Which - .Command - .Should() - .Be(cancelCommand); - } - - [Fact] - public async Task cancel_command_cancels_all_deferred_commands_on_subkernels() - { - var deferredCommandExecuted = false; - - var subKernel = new CSharpKernel(); - - using var compositeKernel = new CompositeKernel - { - subKernel - }; - - compositeKernel.DefaultKernelName = subKernel.Name; - - var deferred = new SubmitCode("placeholder") - { - Handler = (command, context) => - { - deferredCommandExecuted = true; - return Task.CompletedTask; - } - }; - - var cancelCommand = new Cancel(); - - subKernel.DeferCommand(deferred); - - var events = compositeKernel.KernelEvents.ToSubscribedList(); - - await compositeKernel.SendAsync(cancelCommand); - - using var _ = new AssertionScope(); - - deferredCommandExecuted.Should().BeFalse(); - - events - .Should().ContainSingle() - .Which - .Command - .Should() - .Be(cancelCommand); - } - [Fact] public async Task Deferred_commands_on_composite_kernel_can_use_directives() { diff --git a/src/Microsoft.DotNet.Interactive.Tests/QuitCommandTests.cs b/src/Microsoft.DotNet.Interactive.Tests/QuitCommandTests.cs index 09150633f2..b83d0b372d 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/QuitCommandTests.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/QuitCommandTests.cs @@ -6,10 +6,8 @@ using FluentAssertions; using FluentAssertions.Execution; using Microsoft.DotNet.Interactive.Commands; -using Microsoft.DotNet.Interactive.CSharp; using Microsoft.DotNet.Interactive.Events; using Microsoft.DotNet.Interactive.Tests.Utility; -using Pocket; using Xunit; using Xunit.Abstractions; @@ -74,8 +72,6 @@ public async Task quit_command_cancels_all_deferred_commands_on_composite_kernel kernel.DeferCommand(deferred); - var events = kernel.KernelEvents.ToSubscribedList(); - await kernel.SendAsync(quitCommand); using var _ = new AssertionScope(); @@ -83,7 +79,7 @@ public async Task quit_command_cancels_all_deferred_commands_on_composite_kernel deferredCommandExecuted.Should().BeFalse(); quitCommandExecuted.Should().BeTrue(); - events + KernelEvents .Should().ContainSingle() .Which .Command @@ -115,8 +111,6 @@ public async Task quit_command_cancels_all_deferred_commands_on_subkernels() kernel.ChildKernels[0].DeferCommand(deferred); - var events = kernel.KernelEvents.ToSubscribedList(); - await kernel.SendAsync(quitCommand); using var _ = new AssertionScope(); @@ -124,7 +118,7 @@ public async Task quit_command_cancels_all_deferred_commands_on_subkernels() deferredCommandExecuted.Should().BeFalse(); quitCommandExecuted.Should().BeTrue(); - events + KernelEvents .Should().ContainSingle() .Which .Command From 5fd1b21caf76b89ee10c01ae4439ec4b5375dcce Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Tue, 16 Feb 2021 21:30:51 +0000 Subject: [PATCH 17/44] refactor cancel tests out to specific test class --- .../CancelCommandTests.cs | 98 ++++++++- .../LanguageKernelTests.cs | 204 ------------------ .../QuitCommandTests.cs | 95 +++++++- .../Commands/Cancel.cs | 7 +- 4 files changed, 191 insertions(+), 213 deletions(-) diff --git a/src/Microsoft.DotNet.Interactive.Tests/CancelCommandTests.cs b/src/Microsoft.DotNet.Interactive.Tests/CancelCommandTests.cs index 6f2d34c264..d7c94ada1a 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/CancelCommandTests.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/CancelCommandTests.cs @@ -1,6 +1,8 @@ // Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using System; +using System.Linq; using System.Threading.Tasks; using FluentAssertions; using FluentAssertions.Execution; @@ -54,12 +56,15 @@ public async Task cancel_command_cancels_all_deferred_commands_on_composite_kern .Be(cancelCommand); } - [Fact] - public async Task cancel_command_cancels_all_deferred_commands_on_subkernels() + [Theory] + [InlineData(Language.CSharp)] + [InlineData(Language.FSharp)] + [InlineData(Language.PowerShell)] + public async Task cancel_command_cancels_all_deferred_commands_on_subkernels(Language language) { var deferredCommandExecuted = false; - var kernel = CreateKernel(); + var kernel = CreateKernel(language); var deferred = new SubmitCode("placeholder") { @@ -72,7 +77,10 @@ public async Task cancel_command_cancels_all_deferred_commands_on_subkernels() var cancelCommand = new Cancel(); - kernel.ChildKernels[0].DeferCommand(deferred); + foreach (var subkernel in kernel.ChildKernels) + { + subkernel.DeferCommand(deferred); + } await kernel.SendAsync(cancelCommand); @@ -88,5 +96,87 @@ public async Task cancel_command_cancels_all_deferred_commands_on_subkernels() .Be(cancelCommand); } + [Theory] + [InlineData(Language.CSharp)] + [InlineData(Language.FSharp)] + [InlineData(Language.PowerShell, Skip = "to address later")] + public void cancel_command_cancels_the_running_command(Language language) + { + var submitCodeCommandCancelled = false; + var submitCodeCommandStarted = false; + var kernel = CreateKernel(language); + + var cancelCommand = new Cancel(); + + var submitCodeCommand = new SubmitCode("placeholder") + { + Handler = (command, context) => + { + submitCodeCommandStarted = true; + + while (!context.CancellationToken.IsCancellationRequested) + { + + } + + submitCodeCommandCancelled = true; + + return Task.CompletedTask; + } + }; + + Task.WhenAll( + Task.Run(async () => + { + await kernel.SendAsync(submitCodeCommand); + }), + Task.Run(async () => + { + await Task.Delay(100); + await kernel.SendAsync(cancelCommand); + })).Wait(); + // .Wait(TimeSpan.FromSeconds(20)); + + + using var _ = new AssertionScope(); + + submitCodeCommandStarted.Should().BeTrue(); + submitCodeCommandCancelled.Should().BeTrue(); + + KernelEvents + .Should() + .ContainSingle(c => c.Command == submitCodeCommand) + .Which + .Exception + .Should() + .BeOfType(); + } + + + [Theory] + [InlineData(Language.CSharp, "await Task.Delay(Microsoft.DotNet.Interactive.KernelInvocationContext.Current.Token); Console.WriteLine(\"done c#\")", "done c#")] + [InlineData(Language.FSharp, @" +System.Threading.Thread.Sleep(3000) +Console.WriteLine(""done c#"")", "done f#")] + public async Task user_code_can_react_to_cancel_command_using_cancellation_token(Language language, string code, string expectedValue) + { + var kernel = CreateKernel(language); + var cancelCommand = new Cancel(); + var submitCodeCommand = new SubmitCode(code); + + Task.Run(async () => await kernel.SendAsync(submitCodeCommand)); + await Task.Delay(100); + await kernel.SendAsync(cancelCommand); + + + KernelEvents + .Should() + .ContainSingle() + .Which + .Value + .Should() + .Be(expectedValue); + + } } } \ No newline at end of file diff --git a/src/Microsoft.DotNet.Interactive.Tests/LanguageKernelTests.cs b/src/Microsoft.DotNet.Interactive.Tests/LanguageKernelTests.cs index 74349f125a..4c5408644c 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/LanguageKernelTests.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/LanguageKernelTests.cs @@ -1267,209 +1267,5 @@ public async Task SetVariableAsync_can_redeclare_an_existing_variable_and_change succeeded.Should().BeTrue(); x.Should().Be("hello"); } - - - [Theory] - [InlineData(Language.CSharp, "System.Threading.Thread.Sleep(3000);")] - [InlineData(Language.FSharp, "System.Threading.Thread.Sleep(3000)")] - [InlineData(Language.PowerShell, "Start-Sleep -Milliseconds 3000", Skip = "to address later")] - public void Quit_command_is_handled(Language language, string code) - { - var kernel = CreateKernel(language); - - var quitCommandExecuted = false; - - Quit.OnQuit(() => { quitCommandExecuted = true; }); - - var quitCommand = new Quit(); - - var submitCodeCommand = new SubmitCode(code); - - Task.WhenAll( - Task.Run(async () => - { - await Task.Delay(20); - await kernel.SendAsync(submitCodeCommand); - }), - Task.Run(async () => - { - await Task.Delay(100); - await kernel.SendAsync(quitCommand); - })) - .Wait(TimeSpan.FromSeconds(20)); - - using var _ = new AssertionScope(); - - quitCommandExecuted.Should().BeTrue(); - - KernelEvents - .Should() - .ContainSingle() - .Which - .Command - .Should() - .Be(quitCommand); - } - - [Theory] - [InlineData(Language.CSharp, "System.Threading.Thread.Sleep(3000);")] - [InlineData(Language.FSharp, "System.Threading.Thread.Sleep(3000)")] - [InlineData(Language.PowerShell, "Start-Sleep -Milliseconds 3000", Skip = "to address later")] - public void Quit_command_causes_the_running_command_to_fail(Language language, string code) - { - var kernel = CreateKernel(language); - - Quit.OnQuit(() => { }); - - var quitCommand = new Quit(); - - var submitCodeCommand = new SubmitCode(code); - - Task.WhenAll( - Task.Run(async () => - { - await Task.Delay(20); - await kernel.SendAsync(submitCodeCommand); - }), - Task.Run(async () => - { - await Task.Delay(100); - await kernel.SendAsync(quitCommand); - })) - .Wait(TimeSpan.FromSeconds(20)); - - using var _ = new AssertionScope(); - - KernelEvents - .Should() - .ContainSingle(c => c.Command == submitCodeCommand) - .Which - .Exception - .Should() - .BeOfType(); - } - - - [Theory] - [InlineData(Language.CSharp, "await Task.Delay(Microsoft.DotNet.Interactive.KernelInvocationContext.Current.Token); Console.WriteLine(\"done c#\")", "done c#")] - [InlineData(Language.FSharp, @" -System.Threading.Thread.Sleep(3000) -Console.WriteLine(""done c#"")", "done f#")] - public async Task user_code_can_react_to_cancel_command_using_cancellation_token(Language language, string code, string expectedValue) - { - var kernel = CreateKernel(language); - var cancelCommand = new Cancel(); - var submitCodeCommand = new SubmitCode(code); - - Task.Run(async () => await kernel.SendAsync(submitCodeCommand)); - await Task.Delay(100); - await kernel.SendAsync(cancelCommand); - - - KernelEvents - .Should() - .ContainSingle() - .Which - .Value - .Should() - .Be(expectedValue); - - } - - [Theory] - [InlineData(Language.CSharp)] - [InlineData(Language.FSharp)] - [InlineData(Language.PowerShell, Skip = "to address later")] - public void cancel_command_cancels_the_running_command(Language language) - { - var submitCodeCommandCancelled = false; - var submitCodeCommandStarted = false; - var kernel = CreateKernel(language); - - var cancelCommand = new Cancel(); - - var submitCodeCommand = new SubmitCode("placeholder") - { - Handler = (command, context) => - { - submitCodeCommandStarted = true; - - while (!context.CancellationToken.IsCancellationRequested) - { - - } - - submitCodeCommandCancelled = true; - - return Task.CompletedTask; - } - }; - - Task.WhenAll( - Task.Run(async () => - { - await kernel.SendAsync(submitCodeCommand); - }), - Task.Run(async () => - { - await Task.Delay(100); - await kernel.SendAsync(cancelCommand); - })).Wait(); - // .Wait(TimeSpan.FromSeconds(20)); - - - using var _ = new AssertionScope(); - - submitCodeCommandStarted.Should().BeTrue(); - submitCodeCommandCancelled.Should().BeTrue(); - - KernelEvents - .Should() - .ContainSingle(c => c.Command == submitCodeCommand) - .Which - .Exception - .Should() - .BeOfType(); - } - - [Theory] - [InlineData(Language.CSharp)] - [InlineData(Language.FSharp)] - public async Task cancel_command_cancels_all_deferred_commands(Language language) - { - var deferredCommandExecuted = false; - - var kernel = CreateKernel(language); - var languageKernel = kernel.ChildKernels.OfType().Single(); - - - var deferred = new SubmitCode("placeholder") - { - Handler = (command, context) => - { - deferredCommandExecuted = true; - return Task.CompletedTask; - } - }; - - var cancelCommand = new Cancel(); - - languageKernel.DeferCommand(deferred); - - var events = kernel.KernelEvents.ToSubscribedList(); - - await kernel.SendAsync(cancelCommand); - - using var _ = new AssertionScope(); - - deferredCommandExecuted.Should().BeFalse(); - - events - .Should().ContainSingle() - .Which - .Command - .Should() - .Be(cancelCommand); - } } } diff --git a/src/Microsoft.DotNet.Interactive.Tests/QuitCommandTests.cs b/src/Microsoft.DotNet.Interactive.Tests/QuitCommandTests.cs index b83d0b372d..9a416cc9d4 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/QuitCommandTests.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/QuitCommandTests.cs @@ -87,14 +87,17 @@ public async Task quit_command_cancels_all_deferred_commands_on_composite_kernel .Be(quitCommand); } - [Fact] - public async Task quit_command_cancels_all_deferred_commands_on_subkernels() + [Theory] + [InlineData(Language.CSharp)] + [InlineData(Language.FSharp)] + [InlineData(Language.PowerShell)] + public async Task quit_command_cancels_all_deferred_commands_on_subkernels(Language language) { var deferredCommandExecuted = false; var quitCommandExecuted = false; - var kernel = CreateKernel(); + var kernel = CreateKernel(language); var deferred = new SubmitCode("placeholder") { @@ -109,7 +112,10 @@ public async Task quit_command_cancels_all_deferred_commands_on_subkernels() var quitCommand = new Quit(); - kernel.ChildKernels[0].DeferCommand(deferred); + foreach (var subkernel in kernel.ChildKernels) + { + subkernel.DeferCommand(deferred); + } await kernel.SendAsync(quitCommand); @@ -125,5 +131,86 @@ public async Task quit_command_cancels_all_deferred_commands_on_subkernels() .Should() .Be(quitCommand); } + + [Theory] + [InlineData(Language.CSharp, "System.Threading.Thread.Sleep(3000);")] + [InlineData(Language.FSharp, "System.Threading.Thread.Sleep(3000)")] + [InlineData(Language.PowerShell, "Start-Sleep -Milliseconds 3000", Skip = "to address later")] + public void Quit_command_is_handled(Language language, string code) + { + var kernel = CreateKernel(language); + + var quitCommandExecuted = false; + + Quit.OnQuit(() => { quitCommandExecuted = true; }); + + var quitCommand = new Quit(); + + var submitCodeCommand = new SubmitCode(code); + + Task.WhenAll( + Task.Run(async () => + { + await Task.Delay(20); + await kernel.SendAsync(submitCodeCommand); + }), + Task.Run(async () => + { + await Task.Delay(100); + await kernel.SendAsync(quitCommand); + })) + .Wait(TimeSpan.FromSeconds(20)); + + using var _ = new AssertionScope(); + + quitCommandExecuted.Should().BeTrue(); + + KernelEvents + .Should() + .ContainSingle() + .Which + .Command + .Should() + .Be(quitCommand); + } + + + [Theory] + [InlineData(Language.CSharp, "System.Threading.Thread.Sleep(3000);")] + [InlineData(Language.FSharp, "System.Threading.Thread.Sleep(3000)")] + [InlineData(Language.PowerShell, "Start-Sleep -Milliseconds 3000", Skip = "to address later")] + public void Quit_command_causes_the_running_command_to_fail(Language language, string code) + { + var kernel = CreateKernel(language); + + Quit.OnQuit(() => { }); + + var quitCommand = new Quit(); + + var submitCodeCommand = new SubmitCode(code); + + Task.WhenAll( + Task.Run(async () => + { + await Task.Delay(20); + await kernel.SendAsync(submitCodeCommand); + }), + Task.Run(async () => + { + await Task.Delay(100); + await kernel.SendAsync(quitCommand); + })) + .Wait(TimeSpan.FromSeconds(20)); + + using var _ = new AssertionScope(); + + KernelEvents + .Should() + .ContainSingle(c => c.Command == submitCodeCommand) + .Which + .Exception + .Should() + .BeOfType(); + } } } \ No newline at end of file diff --git a/src/Microsoft.DotNet.Interactive/Commands/Cancel.cs b/src/Microsoft.DotNet.Interactive/Commands/Cancel.cs index 56ed79765d..7f582010d1 100644 --- a/src/Microsoft.DotNet.Interactive/Commands/Cancel.cs +++ b/src/Microsoft.DotNet.Interactive/Commands/Cancel.cs @@ -9,7 +9,12 @@ public class Cancel : KernelCommand { public Cancel(string targetKernelName = null): base(targetKernelName) { - Handler = (command, context) => Task.CompletedTask; + + } + + public override Task InvokeAsync(KernelInvocationContext context) + { + return Task.CompletedTask; } } } \ No newline at end of file From b7701080fee6cad8798cf92aee0c7d31caa844b6 Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Tue, 16 Feb 2021 21:32:14 +0000 Subject: [PATCH 18/44] timeout on wait --- src/Microsoft.DotNet.Interactive.Tests/CancelCommandTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.DotNet.Interactive.Tests/CancelCommandTests.cs b/src/Microsoft.DotNet.Interactive.Tests/CancelCommandTests.cs index d7c94ada1a..3ae4f152fe 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/CancelCommandTests.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/CancelCommandTests.cs @@ -134,8 +134,8 @@ public void cancel_command_cancels_the_running_command(Language language) { await Task.Delay(100); await kernel.SendAsync(cancelCommand); - })).Wait(); - // .Wait(TimeSpan.FromSeconds(20)); + })) + .Wait(TimeSpan.FromSeconds(20)); using var _ = new AssertionScope(); From 67b552496f38dd8f6e0d30b902d600c85c77dc17 Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Tue, 16 Feb 2021 21:35:16 +0000 Subject: [PATCH 19/44] remove unused namespace --- .../CancelCommandTests.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.DotNet.Interactive.Tests/CancelCommandTests.cs b/src/Microsoft.DotNet.Interactive.Tests/CancelCommandTests.cs index 3ae4f152fe..4e1918f8c8 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/CancelCommandTests.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/CancelCommandTests.cs @@ -2,13 +2,15 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; -using System.Linq; using System.Threading.Tasks; + using FluentAssertions; using FluentAssertions.Execution; + using Microsoft.DotNet.Interactive.Commands; using Microsoft.DotNet.Interactive.Events; using Microsoft.DotNet.Interactive.Tests.Utility; + using Xunit; using Xunit.Abstractions; @@ -26,7 +28,7 @@ public async Task cancel_command_cancels_all_deferred_commands_on_composite_kern var deferredCommandExecuted = false; var kernel = CreateKernel(); - + var deferred = new SubmitCode("placeholder") { Handler = (command, context) => @@ -134,7 +136,7 @@ public void cancel_command_cancels_the_running_command(Language language) { await Task.Delay(100); await kernel.SendAsync(cancelCommand); - })) + })) .Wait(TimeSpan.FromSeconds(20)); From fa9b8a7f519c031d8ef393b4b591e3bacd37d2c1 Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Tue, 16 Feb 2021 23:15:35 +0000 Subject: [PATCH 20/44] rebase quit on cancel command --- .../LanguageKernelTestBase.cs | 4 +++- .../QuitCommandTests.cs | 9 ++++----- src/Microsoft.DotNet.Interactive/Commands/Quit.cs | 4 +--- src/Microsoft.DotNet.Interactive/CompositeKernel.cs | 6 ------ src/Microsoft.DotNet.Interactive/Kernel.cs | 4 ---- 5 files changed, 8 insertions(+), 19 deletions(-) diff --git a/src/Microsoft.DotNet.Interactive.Tests/LanguageKernelTestBase.cs b/src/Microsoft.DotNet.Interactive.Tests/LanguageKernelTestBase.cs index 5292ab1fc1..95c54ba600 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/LanguageKernelTestBase.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/LanguageKernelTestBase.cs @@ -15,11 +15,13 @@ using Microsoft.DotNet.Interactive.Tests.Utility; using Pocket; - +using Xunit; using static Pocket.Logger; using Xunit.Abstractions; +[assembly: CollectionBehavior(DisableTestParallelization = true)] + namespace Microsoft.DotNet.Interactive.Tests { [LogTestNamesToPocketLogger] diff --git a/src/Microsoft.DotNet.Interactive.Tests/QuitCommandTests.cs b/src/Microsoft.DotNet.Interactive.Tests/QuitCommandTests.cs index 9a416cc9d4..4916969b67 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/QuitCommandTests.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/QuitCommandTests.cs @@ -13,6 +13,7 @@ namespace Microsoft.DotNet.Interactive.Tests { + [Collection("Do not parallelize")] public class QuitCommandTests : LanguageKernelTestBase { public QuitCommandTests(ITestOutputHelper output) : base(output) @@ -25,21 +26,19 @@ public async Task quit_command_fails_when_not_configured() var kernel = CreateKernel(); var quitCommand = new Quit(); - - var events = kernel.KernelEvents.ToSubscribedList(); - + await kernel.SendAsync(quitCommand); using var _ = new AssertionScope(); - events + KernelEvents .Should().ContainSingle() .Which .Command .Should() .Be(quitCommand); - events + KernelEvents .Should().ContainSingle() .Which .Exception diff --git a/src/Microsoft.DotNet.Interactive/Commands/Quit.cs b/src/Microsoft.DotNet.Interactive/Commands/Quit.cs index 3aaeb3a8fa..4ac1b73a9c 100644 --- a/src/Microsoft.DotNet.Interactive/Commands/Quit.cs +++ b/src/Microsoft.DotNet.Interactive/Commands/Quit.cs @@ -8,7 +8,7 @@ namespace Microsoft.DotNet.Interactive.Commands { - public class Quit : KernelCommand + public class Quit : Cancel { private static Action _onQuit; private static readonly Action DefaultOnQuit = () => throw new InvalidOperationException("Quit command is not configured"); @@ -34,7 +34,5 @@ public override Task InvokeAsync(KernelInvocationContext context) context?.Complete(context.Command); return Task.CompletedTask; } - - } } \ No newline at end of file diff --git a/src/Microsoft.DotNet.Interactive/CompositeKernel.cs b/src/Microsoft.DotNet.Interactive/CompositeKernel.cs index 3d1fb4907f..d86d4adfec 100644 --- a/src/Microsoft.DotNet.Interactive/CompositeKernel.cs +++ b/src/Microsoft.DotNet.Interactive/CompositeKernel.cs @@ -216,12 +216,6 @@ internal override async Task HandleAsync( switch (command) { case Cancel _: - - CancelInflightCommands(); - ClearPendingCommands(); - kernel.ClearPendingCommands(); - break; - case Quit _: CancelInflightCommands(); ClearPendingCommands(); kernel.CancelInflightCommands(); diff --git a/src/Microsoft.DotNet.Interactive/Kernel.cs b/src/Microsoft.DotNet.Interactive/Kernel.cs index 683188fc80..60bed033d7 100644 --- a/src/Microsoft.DotNet.Interactive/Kernel.cs +++ b/src/Microsoft.DotNet.Interactive/Kernel.cs @@ -317,10 +317,6 @@ internal Task SendAsync( CancelInflightCommands(); ClearPendingCommands(); break; - case Quit _: - CancelInflightCommands(); - ClearPendingCommands(); - break; default: UndeferCommands(); break; From 60a3853dea029e2f8ed59567f65b63f0a695921c Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Tue, 16 Feb 2021 23:24:50 +0000 Subject: [PATCH 21/44] reset handler before test --- src/Microsoft.DotNet.Interactive.Tests/QuitCommandTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.DotNet.Interactive.Tests/QuitCommandTests.cs b/src/Microsoft.DotNet.Interactive.Tests/QuitCommandTests.cs index 4916969b67..6d2f409237 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/QuitCommandTests.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/QuitCommandTests.cs @@ -24,7 +24,7 @@ public QuitCommandTests(ITestOutputHelper output) : base(output) public async Task quit_command_fails_when_not_configured() { var kernel = CreateKernel(); - + Quit.OnQuit(null); var quitCommand = new Quit(); await kernel.SendAsync(quitCommand); From a282d76a5238538892fa8ad9aa0d9108d4823908 Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Tue, 16 Feb 2021 23:34:52 +0000 Subject: [PATCH 22/44] reflect changes in contract.ts --- .../src/dotnet-interactive/contracts.ts | 2 +- src/dotnet-interactive-vscode/src/interfaces/src/contracts.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.DotNet.Interactive.Js/src/dotnet-interactive/contracts.ts b/src/Microsoft.DotNet.Interactive.Js/src/dotnet-interactive/contracts.ts index 0594cce8cb..edc7175c76 100644 --- a/src/Microsoft.DotNet.Interactive.Js/src/dotnet-interactive/contracts.ts +++ b/src/Microsoft.DotNet.Interactive.Js/src/dotnet-interactive/contracts.ts @@ -65,7 +65,7 @@ export interface ParseNotebook extends KernelCommand { rawData: Uint8Array; } -export interface Quit extends KernelCommand { +export interface Quit extends Cancel { } export interface RequestCompletions extends LanguageServiceCommand { diff --git a/src/dotnet-interactive-vscode/src/interfaces/src/contracts.ts b/src/dotnet-interactive-vscode/src/interfaces/src/contracts.ts index 0594cce8cb..edc7175c76 100644 --- a/src/dotnet-interactive-vscode/src/interfaces/src/contracts.ts +++ b/src/dotnet-interactive-vscode/src/interfaces/src/contracts.ts @@ -65,7 +65,7 @@ export interface ParseNotebook extends KernelCommand { rawData: Uint8Array; } -export interface Quit extends KernelCommand { +export interface Quit extends Cancel { } export interface RequestCompletions extends LanguageServiceCommand { From 2bcc44e417799893ea8e5f2e17467ea28df3de84 Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Tue, 16 Feb 2021 23:41:32 +0000 Subject: [PATCH 23/44] refactor cancelCommands remove unused methods --- .../CompositeKernel.cs | 6 ++--- src/Microsoft.DotNet.Interactive/Kernel.cs | 24 ++++++++----------- 2 files changed, 12 insertions(+), 18 deletions(-) diff --git a/src/Microsoft.DotNet.Interactive/CompositeKernel.cs b/src/Microsoft.DotNet.Interactive/CompositeKernel.cs index d86d4adfec..77709e6bc9 100644 --- a/src/Microsoft.DotNet.Interactive/CompositeKernel.cs +++ b/src/Microsoft.DotNet.Interactive/CompositeKernel.cs @@ -216,10 +216,8 @@ internal override async Task HandleAsync( switch (command) { case Cancel _: - CancelInflightCommands(); - ClearPendingCommands(); - kernel.CancelInflightCommands(); - kernel.ClearPendingCommands(); + CancelCommands(); + kernel.CancelCommands(); break; } diff --git a/src/Microsoft.DotNet.Interactive/Kernel.cs b/src/Microsoft.DotNet.Interactive/Kernel.cs index 60bed033d7..70396c2139 100644 --- a/src/Microsoft.DotNet.Interactive/Kernel.cs +++ b/src/Microsoft.DotNet.Interactive/Kernel.cs @@ -13,6 +13,7 @@ using System.Runtime.CompilerServices; using System.Threading; using System.Threading.Tasks; + using Microsoft.CodeAnalysis.Text; using Microsoft.DotNet.Interactive.Commands; using Microsoft.DotNet.Interactive.Events; @@ -139,9 +140,9 @@ private IReadOnlyList PreprocessCommands(KernelCommand command) { return command switch { - SubmitCode {LanguageNode: null} submitCode => SubmissionParser.SplitSubmission(submitCode), - RequestDiagnostics {LanguageNode: null} requestDiagnostics => SubmissionParser.SplitSubmission(requestDiagnostics), - LanguageServiceCommand {LanguageNode: null} languageServiceCommand => PreprocessLanguageServiceCommand(languageServiceCommand), + SubmitCode { LanguageNode: null } submitCode => SubmissionParser.SplitSubmission(submitCode), + RequestDiagnostics { LanguageNode: null } requestDiagnostics => SubmissionParser.SplitSubmission(requestDiagnostics), + LanguageServiceCommand { LanguageNode: null } languageServiceCommand => PreprocessLanguageServiceCommand(languageServiceCommand), _ => new[] { command } }; } @@ -308,14 +309,13 @@ internal Task SendAsync( } var tcs = new TaskCompletionSource(); - + var operation = new KernelOperation(command, tcs, false); - + switch (command) { case Cancel _: - CancelInflightCommands(); - ClearPendingCommands(); + CancelCommands(); break; default: UndeferCommands(); @@ -358,9 +358,8 @@ private static bool CanCancel(KernelCommand command) _ => true }; } - - internal void CancelInflightCommands() + protected internal void CancelCommands() { foreach (var kernelInvocationContext in KernelInvocationContext.ActiveContexts.Where(c => !c.IsComplete && CanCancel(c.Command))) { @@ -385,10 +384,7 @@ internal void CancelInflightCommands() currentContext?.Cancel(); } - } - internal void ClearPendingCommands() - { _deferredCommands.Clear(); _commandQueue.Clear(); } @@ -472,7 +468,7 @@ private Task HandleRequestCompletionsAsync( return Task.CompletedTask; } - private IReadOnlyList GetDirectiveCompletionItems( + private IEnumerable GetDirectiveCompletionItems( DirectiveNode directiveNode, int requestPosition) { @@ -584,7 +580,7 @@ protected virtual void SetHandlingKernel( protected virtual ChooseKernelDirective CreateChooseKernelDirective() { - return new ChooseKernelDirective(this); + return new(this); } internal ChooseKernelDirective ChooseKernelDirective => _chooseKernelDirective ??= CreateChooseKernelDirective(); From b27cc5f0562e87a54200628598de740610c1dfec Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Wed, 17 Feb 2021 00:23:55 +0000 Subject: [PATCH 24/44] avoid double cancellation request --- src/Microsoft.DotNet.Interactive/Kernel.cs | 1 - .../KernelInvocationContext.cs | 9 +++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.DotNet.Interactive/Kernel.cs b/src/Microsoft.DotNet.Interactive/Kernel.cs index 70396c2139..7b5999d718 100644 --- a/src/Microsoft.DotNet.Interactive/Kernel.cs +++ b/src/Microsoft.DotNet.Interactive/Kernel.cs @@ -364,7 +364,6 @@ protected internal void CancelCommands() foreach (var kernelInvocationContext in KernelInvocationContext.ActiveContexts.Where(c => !c.IsComplete && CanCancel(c.Command))) { kernelInvocationContext.Cancel(); - } using var disposables = new CompositeDisposable(); diff --git a/src/Microsoft.DotNet.Interactive/KernelInvocationContext.cs b/src/Microsoft.DotNet.Interactive/KernelInvocationContext.cs index 7f26e3e779..36ced63c24 100644 --- a/src/Microsoft.DotNet.Interactive/KernelInvocationContext.cs +++ b/src/Microsoft.DotNet.Interactive/KernelInvocationContext.cs @@ -90,10 +90,15 @@ public void Fail( { if (!IsComplete) { + Publish(new CommandFailed(exception, Command, message)); - _events.OnCompleted(); - _cancellationTokenSource.Cancel(false); + + if (_cancellationTokenSource.IsCancellationRequested) + { + _cancellationTokenSource.Cancel(false); + } + IsComplete = true; } } From 47a35f3a28a917c0458844d2ab285f185b00ba82 Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Wed, 17 Feb 2021 00:46:02 +0000 Subject: [PATCH 25/44] add test to ensure commands are runing after cancel is issued --- .../CancelCommandTests.cs | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/src/Microsoft.DotNet.Interactive.Tests/CancelCommandTests.cs b/src/Microsoft.DotNet.Interactive.Tests/CancelCommandTests.cs index 4e1918f8c8..49e3cee7d1 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/CancelCommandTests.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/CancelCommandTests.cs @@ -154,6 +154,79 @@ public void cancel_command_cancels_the_running_command(Language language) .BeOfType(); } + [Theory] + [InlineData(Language.CSharp)] + [InlineData(Language.FSharp)] + [InlineData(Language.PowerShell, Skip = "to address later")] + public void commands_issued_after_cancel_command_are_executed(Language language) + { + var commandToCancelHasRun = false; + var commandToCancelHasBeenCancelled = false; + var commandToRunHasRun = false; + var kernel = CreateKernel(language); + + var cancelCommand = new Cancel(); + + var commandToCancel = new SubmitCode("placeholder") + { + Handler = (command, context) => + { + commandToCancelHasRun = true; + + while (!context.CancellationToken.IsCancellationRequested) + { + + } + + commandToCancelHasBeenCancelled = true; + + return Task.CompletedTask; + } + }; + + var commandToRun = new SubmitCode("placeholder") + { + Handler = (command, context) => + { + commandToRunHasRun = true; + + return Task.CompletedTask; + } + }; + + Task.WhenAll( + Task.Run(async () => + { + await kernel.SendAsync(commandToCancel); + }), + Task.Run(async () => + { + await Task.Delay(40); + await kernel.SendAsync(cancelCommand); + }), + Task.Run(async () => + { + await Task.Delay(100); + await kernel.SendAsync(commandToRun); + })) + .Wait(TimeSpan.FromSeconds(20)); + + + using var _ = new AssertionScope(); + + commandToRunHasRun.Should().BeTrue(); + commandToCancelHasRun.Should().BeTrue(); + commandToCancelHasBeenCancelled.Should().BeTrue(); + + KernelEvents + .Should() + .ContainSingle(c => c.Command == commandToCancel) + .Which + .Exception + .Should() + .BeOfType(); + } + [Theory] [InlineData(Language.CSharp, "await Task.Delay(Microsoft.DotNet.Interactive.KernelInvocationContext.Current.Token); Console.WriteLine(\"done c#\")", "done c#")] From c43018fd372388a965e41a7a5267ca67b2830116 Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Wed, 17 Feb 2021 02:16:18 +0000 Subject: [PATCH 26/44] introduce KernelCommandScheduler --- .../CancelCommandTests.cs | 94 ++++----- src/Microsoft.DotNet.Interactive/Kernel.cs | 139 +----------- .../KernelCommandScheduler.cs | 198 ++++++++++++++++++ 3 files changed, 248 insertions(+), 183 deletions(-) create mode 100644 src/Microsoft.DotNet.Interactive/KernelCommandScheduler.cs diff --git a/src/Microsoft.DotNet.Interactive.Tests/CancelCommandTests.cs b/src/Microsoft.DotNet.Interactive.Tests/CancelCommandTests.cs index 49e3cee7d1..54420c9bea 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/CancelCommandTests.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/CancelCommandTests.cs @@ -158,65 +158,26 @@ public void cancel_command_cancels_the_running_command(Language language) [InlineData(Language.CSharp)] [InlineData(Language.FSharp)] [InlineData(Language.PowerShell, Skip = "to address later")] - public void commands_issued_after_cancel_command_are_executed(Language language) + public async Task commands_issued_after_cancel_command_are_executed(Language language) { - var commandToCancelHasRun = false; - var commandToCancelHasBeenCancelled = false; - var commandToRunHasRun = false; + var kernel = CreateKernel(language); var cancelCommand = new Cancel(); - var commandToCancel = new SubmitCode("placeholder") - { - Handler = (command, context) => - { - commandToCancelHasRun = true; - - while (!context.CancellationToken.IsCancellationRequested) - { - - } - - commandToCancelHasBeenCancelled = true; - - return Task.CompletedTask; - } - }; - - var commandToRun = new SubmitCode("placeholder") - { - Handler = (command, context) => - { - commandToRunHasRun = true; - - return Task.CompletedTask; - } - }; - - Task.WhenAll( - Task.Run(async () => - { - await kernel.SendAsync(commandToCancel); - }), - Task.Run(async () => - { - await Task.Delay(40); - await kernel.SendAsync(cancelCommand); - }), - Task.Run(async () => - { - await Task.Delay(100); - await kernel.SendAsync(commandToRun); - })) - .Wait(TimeSpan.FromSeconds(20)); + var commandToCancel = new CancellableCommand( ); + var commandToRun = new SubmitCode("1"); - using var _ = new AssertionScope(); + kernel.SendAsync(commandToCancel); + await Task.Delay(4000); + await kernel.SendAsync(cancelCommand); + await kernel.SendAsync(commandToRun); - commandToRunHasRun.Should().BeTrue(); - commandToCancelHasRun.Should().BeTrue(); - commandToCancelHasBeenCancelled.Should().BeTrue(); + // using var _ = new AssertionScope(); + + commandToCancel.HasRun.Should().BeTrue(); + commandToCancel.HasBeenCancelled.Should().BeTrue(); KernelEvents .Should() @@ -229,16 +190,16 @@ public void commands_issued_after_cancel_command_are_executed(Language language) [Theory] - [InlineData(Language.CSharp, "await Task.Delay(Microsoft.DotNet.Interactive.KernelInvocationContext.Current.Token); Console.WriteLine(\"done c#\")", "done c#")] + [InlineData(Language.CSharp, "await Task.Delay(Microsoft.DotNet.Interactive.KernelInvocationContext.Current.CancellationToken); Console.WriteLine(\"done c#\")", "done c#")] [InlineData(Language.FSharp, @" System.Threading.Thread.Sleep(3000) -Console.WriteLine(""done c#"")", "done f#")] +Console.WriteLine(""done c#"")", "done f#", Skip = "for the moment")] public async Task user_code_can_react_to_cancel_command_using_cancellation_token(Language language, string code, string expectedValue) { var kernel = CreateKernel(language); var cancelCommand = new Cancel(); var submitCodeCommand = new SubmitCode(code); - + Task.Run(async () => await kernel.SendAsync(submitCodeCommand)); await Task.Delay(100); await kernel.SendAsync(cancelCommand); @@ -254,4 +215,29 @@ public async Task user_code_can_react_to_cancel_command_using_cancellation_token } } + + public class CancellableCommand : KernelCommand + { + public CancellableCommand(string targetKernelName = null, KernelCommand parent = null) : base(targetKernelName, parent) + { + } + + public override Task InvokeAsync(KernelInvocationContext context) + { + HasRun = true; + + while (!context.CancellationToken.IsCancellationRequested) + { + + } + + HasBeenCancelled = true; + + return Task.CompletedTask; + } + + public bool HasBeenCancelled { get; private set; } + + public bool HasRun { get; private set; } + } } \ No newline at end of file diff --git a/src/Microsoft.DotNet.Interactive/Kernel.cs b/src/Microsoft.DotNet.Interactive/Kernel.cs index 7b5999d718..42bc0f89e1 100644 --- a/src/Microsoft.DotNet.Interactive/Kernel.cs +++ b/src/Microsoft.DotNet.Interactive/Kernel.cs @@ -7,7 +7,6 @@ using System.CommandLine; using System.CommandLine.Parsing; using System.Linq; -using System.Reactive; using System.Reactive.Disposables; using System.Reactive.Subjects; using System.Runtime.CompilerServices; @@ -49,6 +48,8 @@ protected Kernel(string name) Pipeline = new KernelCommandPipeline(this); + Scheduler = new KernelCommandScheduler(); + AddSetKernelMiddleware(); AddDirectiveMiddlewareAndCommonCommandHandlers(); @@ -58,6 +59,8 @@ protected Kernel(string name) )); } + internal KernelCommandScheduler Scheduler { get; } + internal KernelCommandPipeline Pipeline { get; } public CompositeKernel ParentKernel { get; internal set; } @@ -76,7 +79,7 @@ public void DeferCommand(KernelCommand command) } command.SetToken($"deferredCommand::{Guid.NewGuid():N}"); - _deferredCommands.Enqueue(command); + Scheduler.DeferCommand(command, this); } private void AddSetKernelMiddleware() @@ -247,41 +250,7 @@ public KernelOperation( public int AsyncContextId { get; } } - private async Task ExecuteCommand(KernelOperation operation) - { - var context = KernelInvocationContext.Establish(operation.Command); - - // only subscribe for the root command - using var _ = - context.Command == operation.Command - ? context.KernelEvents.Subscribe(PublishEvent) - : Disposable.Empty; - - try - { - await Pipeline.SendAsync(operation.Command, context); - - if (operation.Command == context.Command) - { - await context.DisposeAsync(); - } - else - { - context.Complete(operation.Command); - } - - operation.TaskCompletionSource.SetResult(context.Result); - } - catch (Exception exception) - { - if (!context.IsComplete) - { - context.Fail(exception); - } - - operation.TaskCompletionSource.SetException(exception); - } - } + internal virtual async Task HandleAsync( KernelCommand command, @@ -308,110 +277,22 @@ internal Task SendAsync( throw new ArgumentNullException(nameof(command)); } - var tcs = new TaskCompletionSource(); - - var operation = new KernelOperation(command, tcs, false); - - switch (command) - { - case Cancel _: - CancelCommands(); - break; - default: - UndeferCommands(); - break; - } - _commandQueue.Enqueue(operation); - ProcessCommandQueue(_commandQueue, cancellationToken, onDone); - - return tcs.Task; - } - - private void ProcessCommandQueue( - ConcurrentQueue commandQueue, - CancellationToken cancellationToken, - Action onDone) - { - if (commandQueue.TryDequeue(out var currentOperation)) - { - Task.Run(async () => - { - AsyncContext.Id = currentOperation.AsyncContextId; - - await ExecuteCommand(currentOperation); - - ProcessCommandQueue(commandQueue, cancellationToken, onDone); - }, cancellationToken).ConfigureAwait(false); - } - else - { - onDone?.Invoke(); - } - } + return Scheduler.Schedule(command, this, cancellationToken, onDone); - private static bool CanCancel(KernelCommand command) - { - return command switch - { - Quit _ => false, - Cancel _ => false, - _ => true - }; } protected internal void CancelCommands() { - foreach (var kernelInvocationContext in KernelInvocationContext.ActiveContexts.Where(c => !c.IsComplete && CanCancel(c.Command))) - { - kernelInvocationContext.Cancel(); - } - - using var disposables = new CompositeDisposable(); - var inFlightOperations = _commandQueue.Where(operation => !operation.IsDeferred && CanCancel(operation.Command)).ToList(); - foreach (var inFlightOperation in inFlightOperations) - { - KernelInvocationContext currentContext = null; - - - if (inFlightOperation is not null - ) - { - currentContext = KernelInvocationContext.Establish(inFlightOperation.Command); - disposables.Add(currentContext.KernelEvents.Subscribe(PublishEvent)); - inFlightOperation.TaskCompletionSource.SetResult(currentContext.Result); - } - - currentContext?.Cancel(); - } - - _deferredCommands.Clear(); - _commandQueue.Clear(); + Scheduler.CancelCommands(); } internal Task RunDeferredCommandsAsync() { - var tcs = new TaskCompletionSource(); - UndeferCommands(); - ProcessCommandQueue( - _commandQueue, - CancellationToken.None, - () => tcs.SetResult(Unit.Default)); - return tcs.Task; - } + return Scheduler.RunDeferredCommandsAsync(); - private void UndeferCommands() - { - while (_deferredCommands.TryDequeue(out var initCommand)) - { - _commandQueue.Enqueue( - new KernelOperation( - initCommand, - new TaskCompletionSource(), - true)); - } } - protected void PublishEvent(KernelEvent kernelEvent) + protected internal void PublishEvent(KernelEvent kernelEvent) { if (kernelEvent == null) { diff --git a/src/Microsoft.DotNet.Interactive/KernelCommandScheduler.cs b/src/Microsoft.DotNet.Interactive/KernelCommandScheduler.cs new file mode 100644 index 0000000000..b2f4817353 --- /dev/null +++ b/src/Microsoft.DotNet.Interactive/KernelCommandScheduler.cs @@ -0,0 +1,198 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; +using System.Collections.Concurrent; +using System.Linq; +using System.Reactive; +using System.Reactive.Disposables; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.DotNet.Interactive.Commands; +using Microsoft.DotNet.Interactive.Utility; + +namespace Microsoft.DotNet.Interactive +{ + public class KernelCommandScheduler + { + private readonly ConcurrentQueue<(KernelCommand command, Kernel kernel)> _deferredCommands = new(); + + private readonly ConcurrentQueue _commandQueue = new(); + public Task Schedule(KernelCommand command, Kernel kernel, CancellationToken cancellationToken, Action onDone) + { + + + switch (command) + { + case Cancel _: + CancelCommands(); + break; + default: + UndeferCommands(); + break; + } + + var kernelCommandResultSource = new TaskCompletionSource(); + + var operation = new KernelOperation(command, kernelCommandResultSource, kernel,false); + _commandQueue.Enqueue(operation); + + ProcessCommandQueue(_commandQueue, cancellationToken, onDone); + + return kernelCommandResultSource.Task; + } + + private void ProcessCommandQueue( + ConcurrentQueue commandQueue, + CancellationToken cancellationToken, + Action onDone) + { + if (commandQueue.TryDequeue(out var currentOperation)) + { + Task.Run(async () => + { + AsyncContext.Id = currentOperation.AsyncContextId; + + await ExecuteCommand(currentOperation); + + ProcessCommandQueue(commandQueue, cancellationToken, onDone); + }, cancellationToken).ConfigureAwait(false); + } + else + { + onDone?.Invoke(); + } + } + + private async Task ExecuteCommand(KernelOperation operation) + { + var context = KernelInvocationContext.Establish(operation.Command); + + // only subscribe for the root command + using var _ = + context.Command == operation.Command + ? context.KernelEvents.Subscribe(operation.Kernel.PublishEvent) + : Disposable.Empty; + + try + { + await operation.Kernel.Pipeline.SendAsync(operation.Command, context); + + if (operation.Command == context.Command) + { + await context.DisposeAsync(); + } + else + { + context.Complete(operation.Command); + } + + operation.TaskCompletionSource.SetResult(context.Result); + } + catch (Exception exception) + { + if (!context.IsComplete) + { + context.Fail(exception); + } + + operation.TaskCompletionSource.SetException(exception); + } + } + private class KernelOperation + { + public KernelOperation( + KernelCommand command, + TaskCompletionSource taskCompletionSource, + Kernel kernel, + bool isDeferred) + { + Command = command; + TaskCompletionSource = taskCompletionSource; + Kernel = kernel; + IsDeferred = isDeferred; + + AsyncContext.TryEstablish(out var id); + AsyncContextId = id; + } + + public KernelCommand Command { get; } + + public TaskCompletionSource TaskCompletionSource { get; } + public Kernel Kernel { get; } + public bool IsDeferred { get; } + + public int AsyncContextId { get; } + } + + public void DeferCommand(KernelCommand command, Kernel kernel) + { + _deferredCommands.Enqueue((command,kernel)); + } + + internal Task RunDeferredCommandsAsync() + { + var tcs = new TaskCompletionSource(); + UndeferCommands(); + ProcessCommandQueue( + _commandQueue, + CancellationToken.None, + () => tcs.SetResult(Unit.Default)); + return tcs.Task; + } + + private void UndeferCommands() + { + while (_deferredCommands.TryDequeue(out var initCommand)) + { + _commandQueue.Enqueue( + new KernelOperation( + initCommand.command, + new TaskCompletionSource(), + initCommand.kernel, + true)); + } + } + + + + private static bool CanCancel(KernelCommand command) + { + return command switch + { + Quit _ => false, + Cancel _ => false, + _ => true + }; + } + + public void CancelCommands() + { + foreach (var kernelInvocationContext in KernelInvocationContext.ActiveContexts.Where(c => !c.IsComplete && CanCancel(c.Command))) + { + kernelInvocationContext.Cancel(); + } + + using var disposables = new CompositeDisposable(); + var inFlightOperations = _commandQueue.Where(operation => !operation.IsDeferred && CanCancel(operation.Command)).ToList(); + foreach (var inFlightOperation in inFlightOperations) + { + KernelInvocationContext currentContext = null; + + + if (inFlightOperation is not null + ) + { + currentContext = KernelInvocationContext.Establish(inFlightOperation.Command); + disposables.Add(currentContext.KernelEvents.Subscribe(inFlightOperation.Kernel.PublishEvent)); + inFlightOperation.TaskCompletionSource.SetResult(currentContext.Result); + } + + currentContext?.Cancel(); + } + + _deferredCommands.Clear(); + _commandQueue.Clear(); + } + } +} \ No newline at end of file From 77656f1b5feb98fcbeff99632890fde37a13b03c Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Thu, 18 Feb 2021 01:50:55 +0000 Subject: [PATCH 27/44] testing scheduler behaviours deferred command that match the kernel used during scheduling of a command are executed deferred command order is preserved and parent kernel deferred commands are executed when scheduling commands on child kernel do not produce diagnostic events when there are none use callable command --- .../CSharpKernel.cs | 15 +- .../CancelCommandTests.cs | 76 ++----- .../KernelCommandSchedulerTests.cs | 213 ++++++++++++++++++ .../LanguageKernelTestBase.cs | 4 +- .../Utility/CancellableCommand.cs | 33 +++ .../Utility/TestCommand.cs | 24 ++ src/Microsoft.DotNet.Interactive/Kernel.cs | 33 +-- .../KernelCommandScheduler.cs | 61 ++++- src/dotnet-interactive/KernelExtensions.cs | 2 +- 9 files changed, 356 insertions(+), 105 deletions(-) create mode 100644 src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs create mode 100644 src/Microsoft.DotNet.Interactive.Tests/Utility/CancellableCommand.cs create mode 100644 src/Microsoft.DotNet.Interactive.Tests/Utility/TestCommand.cs diff --git a/src/Microsoft.DotNet.Interactive.CSharp/CSharpKernel.cs b/src/Microsoft.DotNet.Interactive.CSharp/CSharpKernel.cs index edbacb02e2..2924e31b54 100644 --- a/src/Microsoft.DotNet.Interactive.CSharp/CSharpKernel.cs +++ b/src/Microsoft.DotNet.Interactive.CSharp/CSharpKernel.cs @@ -254,13 +254,16 @@ await RunAsync( // Publish the compilation diagnostics. This doesn't include the exception. var kernelDiagnostics = diagnostics.Select(Diagnostic.FromCodeAnalysisDiagnostic).ToImmutableArray(); - var formattedDiagnostics = - diagnostics - .Select(d => d.ToString()) - .Select(text => new FormattedValue(PlainTextFormatter.MimeType, text)) - .ToImmutableArray(); + if (kernelDiagnostics.Length > 0) + { + var formattedDiagnostics = + diagnostics + .Select(d => d.ToString()) + .Select(text => new FormattedValue(PlainTextFormatter.MimeType, text)) + .ToImmutableArray(); - context.Publish(new DiagnosticsProduced(kernelDiagnostics, submitCode, formattedDiagnostics)); + context.Publish(new DiagnosticsProduced(kernelDiagnostics, submitCode, formattedDiagnostics)); + } // Report the compilation failure or exception if (exception != null) diff --git a/src/Microsoft.DotNet.Interactive.Tests/CancelCommandTests.cs b/src/Microsoft.DotNet.Interactive.Tests/CancelCommandTests.cs index 54420c9bea..dd631317d5 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/CancelCommandTests.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/CancelCommandTests.cs @@ -14,6 +14,7 @@ using Xunit; using Xunit.Abstractions; + namespace Microsoft.DotNet.Interactive.Tests { public class CancelCommandTests : LanguageKernelTestBase @@ -64,18 +65,10 @@ public async Task cancel_command_cancels_all_deferred_commands_on_composite_kern [InlineData(Language.PowerShell)] public async Task cancel_command_cancels_all_deferred_commands_on_subkernels(Language language) { - var deferredCommandExecuted = false; var kernel = CreateKernel(language); - var deferred = new SubmitCode("placeholder") - { - Handler = (command, context) => - { - deferredCommandExecuted = true; - return Task.CompletedTask; - } - }; + var deferred = new CancellableCommand(); var cancelCommand = new Cancel(); @@ -88,7 +81,7 @@ public async Task cancel_command_cancels_all_deferred_commands_on_subkernels(Lan using var _ = new AssertionScope(); - deferredCommandExecuted.Should().BeFalse(); + deferred.HasRun.Should().BeFalse(); KernelEvents .Should().ContainSingle() @@ -104,28 +97,12 @@ public async Task cancel_command_cancels_all_deferred_commands_on_subkernels(Lan [InlineData(Language.PowerShell, Skip = "to address later")] public void cancel_command_cancels_the_running_command(Language language) { - var submitCodeCommandCancelled = false; - var submitCodeCommandStarted = false; + var kernel = CreateKernel(language); var cancelCommand = new Cancel(); - var submitCodeCommand = new SubmitCode("placeholder") - { - Handler = (command, context) => - { - submitCodeCommandStarted = true; - - while (!context.CancellationToken.IsCancellationRequested) - { - - } - - submitCodeCommandCancelled = true; - - return Task.CompletedTask; - } - }; + var submitCodeCommand = new CancellableCommand(); Task.WhenAll( Task.Run(async () => @@ -142,8 +119,8 @@ public void cancel_command_cancels_the_running_command(Language language) using var _ = new AssertionScope(); - submitCodeCommandStarted.Should().BeTrue(); - submitCodeCommandCancelled.Should().BeTrue(); + submitCodeCommand.HasRun.Should().BeTrue(); + submitCodeCommand.HasBeenCancelled.Should().BeTrue(); KernelEvents .Should() @@ -190,7 +167,14 @@ public async Task commands_issued_after_cancel_command_are_executed(Language lan [Theory] - [InlineData(Language.CSharp, "await Task.Delay(Microsoft.DotNet.Interactive.KernelInvocationContext.Current.CancellationToken); Console.WriteLine(\"done c#\")", "done c#")] + [InlineData(Language.CSharp, @"while(true){ + if(Microsoft.DotNet.Interactive.KernelInvocationContext.Current.CancellationToken.IsCancellationRequested) + { + Console.WriteLine(""done c#""); + break; + } +} +", "done c#")] [InlineData(Language.FSharp, @" System.Threading.Thread.Sleep(3000) Console.WriteLine(""done c#"")", "done f#", Skip = "for the moment")] @@ -200,11 +184,10 @@ public async Task user_code_can_react_to_cancel_command_using_cancellation_token var cancelCommand = new Cancel(); var submitCodeCommand = new SubmitCode(code); - Task.Run(async () => await kernel.SendAsync(submitCodeCommand)); - await Task.Delay(100); + kernel.SendAsync(submitCodeCommand); + await Task.Delay(4000); await kernel.SendAsync(cancelCommand); - - + KernelEvents .Should() .ContainSingle() @@ -216,28 +199,5 @@ public async Task user_code_can_react_to_cancel_command_using_cancellation_token } } - public class CancellableCommand : KernelCommand - { - public CancellableCommand(string targetKernelName = null, KernelCommand parent = null) : base(targetKernelName, parent) - { - } - public override Task InvokeAsync(KernelInvocationContext context) - { - HasRun = true; - - while (!context.CancellationToken.IsCancellationRequested) - { - - } - - HasBeenCancelled = true; - - return Task.CompletedTask; - } - - public bool HasBeenCancelled { get; private set; } - - public bool HasRun { get; private set; } - } } \ No newline at end of file diff --git a/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs b/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs new file mode 100644 index 0000000000..811c3b8e6e --- /dev/null +++ b/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs @@ -0,0 +1,213 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; + +using FluentAssertions; + +using Microsoft.DotNet.Interactive.Commands; +using Microsoft.DotNet.Interactive.Tests.Utility; + +using Pocket; + +using Xunit; +using Xunit.Abstractions; + +namespace Microsoft.DotNet.Interactive.Tests +{ + public class KernelCommandSchedulerTests : IDisposable + { + private readonly CompositeDisposable _disposables = new(); + + public KernelCommandSchedulerTests(ITestOutputHelper output) + { + DisposeAfterTest(output.SubscribeToPocketLogger()); + } + + public void Dispose() + { + try + { + _disposables?.Dispose(); + } + catch (Exception ex) + { + Logger.Log.Error(exception: ex); + } + } + + private void DisposeAfterTest(IDisposable disposable) + { + _disposables.Add(disposable); + } + + private void DisposeAfterTest(Action action) + { + _disposables.Add(action); + } + + [Fact] + public async Task command_execute_on_kernel_specified_at_scheduling_time() + { + var commandsHandledOnKernel1 = new List(); + var commandsHandledOnKernel2 = new List(); + + var scheduler = new KernelCommandScheduler(); + + var kernel1 = new FakeKernel("kernel1") + { + Handle = (command, _) => + { + commandsHandledOnKernel1.Add(command); + return Task.CompletedTask; + } + }; + var kernel2 = new FakeKernel("kernel2") + { + Handle = (command, _) => + { + commandsHandledOnKernel2.Add(command); + return Task.CompletedTask; + } + }; + + var command1 = new SubmitCode("for kernel 1"); + var command2 = new SubmitCode("for kernel 2"); + + await scheduler.Schedule(command1, kernel1); + await scheduler.Schedule(command2, kernel2); + + commandsHandledOnKernel1.Should().ContainSingle().Which.Should().Be(command1); + commandsHandledOnKernel2.Should().ContainSingle().Which.Should().Be(command2); + } + + [Fact] + public async Task scheduling_a_command_will_defer_deferred_commands_scheduled_on_same_kernel() + { + var commandsHandledOnKernel1 = new List(); + + var scheduler = new KernelCommandScheduler(); + + var kernel1 = new FakeKernel("kernel1") + { + Handle = (command, _) => + { + commandsHandledOnKernel1.Add(command); + return Task.CompletedTask; + } + }; + var kernel2 = new FakeKernel("kernel2") + { + Handle = (_, _) => Task.CompletedTask + }; + + var deferredCommand1 = new SubmitCode("deferred for kernel 1"); + var deferredCommand2 = new SubmitCode("deferred for kernel 2"); + var deferredCommand3 = new SubmitCode("deferred for kernel 1"); + var command1 = new SubmitCode("for kernel 1"); + + scheduler.DeferCommand(deferredCommand1, kernel1); + scheduler.DeferCommand(deferredCommand2, kernel2); + scheduler.DeferCommand(deferredCommand3, kernel1); + await scheduler.Schedule(command1, kernel1); + + commandsHandledOnKernel1.Should().NotContain(deferredCommand2); + commandsHandledOnKernel1.Should().BeEquivalentSequenceTo(deferredCommand1, deferredCommand3, command1); + } + + [Fact] + public async Task deferred_command_not_executed_are_still_in_deferred_queue() + { + var commandsHandledOnKernel1 = new List(); + var commandsHandledOnKernel2 = new List(); + + var scheduler = new KernelCommandScheduler(); + + var kernel1 = new FakeKernel("kernel1") + { + Handle = (command, _) => + { + commandsHandledOnKernel1.Add(command); + return Task.CompletedTask; + } + }; + var kernel2 = new FakeKernel("kernel2") + { + Handle = (command, _) => + { + commandsHandledOnKernel2.Add(command); + return Task.CompletedTask; + } + }; + + var deferredCommand1 = new SubmitCode("deferred for kernel 1"); + var deferredCommand2 = new SubmitCode("deferred for kernel 2"); + var deferredCommand3 = new SubmitCode("deferred for kernel 1"); + var command1 = new SubmitCode("for kernel 1"); + var command2 = new SubmitCode("for kernel 2"); + + scheduler.DeferCommand(deferredCommand1, kernel1); + scheduler.DeferCommand(deferredCommand2, kernel2); + scheduler.DeferCommand(deferredCommand3, kernel1); + await scheduler.Schedule(command1, kernel1); + + commandsHandledOnKernel2.Should().BeEmpty(); + commandsHandledOnKernel1.Should().NotContain(deferredCommand2); + commandsHandledOnKernel1.Should().BeEquivalentSequenceTo(deferredCommand1, deferredCommand3, command1); + await scheduler.Schedule(command2, kernel2); + commandsHandledOnKernel2.Should().BeEquivalentSequenceTo(deferredCommand2, command2); + } + + [Fact] + public async Task deferred_command_on_parent_kernel_are_executed_when_scheduling_command_on_child_kernel() + { + var commandHandledList = new List<(KernelCommand command, Kernel kernel)>(); + + var scheduler = new KernelCommandScheduler(); + + var childKernel = new FakeKernel("kernel1") + { + Handle = (command, context) => command.InvokeAsync(context) + }; + var parentKernel = new CompositeKernel + { + childKernel + }; + + parentKernel.DefaultKernelName = childKernel.Name; + + var deferredCommand1 = new TestCommand((command, context) => + { + commandHandledList.Add((command, context.HandlingKernel)); + return Task.CompletedTask; + }, childKernel.Name); + var deferredCommand2 = new TestCommand((command, context) => + { + commandHandledList.Add((command, context.HandlingKernel)); + return Task.CompletedTask; + }, parentKernel.Name); + var deferredCommand3 = new TestCommand((command, context) => + { + commandHandledList.Add((command, context.HandlingKernel)); + return Task.CompletedTask; + }, childKernel.Name); + var command1 = new TestCommand((command, context) => + { + commandHandledList.Add((command, context.HandlingKernel)); + return Task.CompletedTask; + }, childKernel.Name); + + scheduler.DeferCommand(deferredCommand1, childKernel); + scheduler.DeferCommand(deferredCommand2, parentKernel); + scheduler.DeferCommand(deferredCommand3, childKernel); + await scheduler.Schedule(command1, childKernel); + + commandHandledList.Select(e => e.command).Should().BeEquivalentSequenceTo(deferredCommand1, deferredCommand2, deferredCommand3, command1); + + commandHandledList.Select(e => e.kernel).Should().BeEquivalentSequenceTo(childKernel, parentKernel, childKernel, childKernel); + } + } +} \ No newline at end of file diff --git a/src/Microsoft.DotNet.Interactive.Tests/LanguageKernelTestBase.cs b/src/Microsoft.DotNet.Interactive.Tests/LanguageKernelTestBase.cs index 95c54ba600..be8eee10a4 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/LanguageKernelTestBase.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/LanguageKernelTestBase.cs @@ -27,7 +27,7 @@ namespace Microsoft.DotNet.Interactive.Tests [LogTestNamesToPocketLogger] public abstract class LanguageKernelTestBase : IDisposable { - private readonly CompositeDisposable _disposables = new CompositeDisposable(); + private readonly CompositeDisposable _disposables = new(); protected LanguageKernelTestBase(ITestOutputHelper output) { @@ -42,7 +42,7 @@ public void Dispose() } catch (Exception ex) { - Log.Error(exception: ex); + Log.Error(ex); } } diff --git a/src/Microsoft.DotNet.Interactive.Tests/Utility/CancellableCommand.cs b/src/Microsoft.DotNet.Interactive.Tests/Utility/CancellableCommand.cs new file mode 100644 index 0000000000..1bf3fae3a0 --- /dev/null +++ b/src/Microsoft.DotNet.Interactive.Tests/Utility/CancellableCommand.cs @@ -0,0 +1,33 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System.Threading.Tasks; +using Microsoft.DotNet.Interactive.Commands; + +namespace Microsoft.DotNet.Interactive.Tests.Utility +{ + public class CancellableCommand : KernelCommand + { + public CancellableCommand(string targetKernelName = null, KernelCommand parent = null) : base(targetKernelName, parent) + { + } + + public override Task InvokeAsync(KernelInvocationContext context) + { + HasRun = true; + + while (!context.CancellationToken.IsCancellationRequested) + { + + } + + HasBeenCancelled = true; + + return Task.CompletedTask; + } + + public bool HasBeenCancelled { get; private set; } + + public bool HasRun { get; private set; } + } +} \ No newline at end of file diff --git a/src/Microsoft.DotNet.Interactive.Tests/Utility/TestCommand.cs b/src/Microsoft.DotNet.Interactive.Tests/Utility/TestCommand.cs new file mode 100644 index 0000000000..21e9a6b907 --- /dev/null +++ b/src/Microsoft.DotNet.Interactive.Tests/Utility/TestCommand.cs @@ -0,0 +1,24 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; +using System.Threading.Tasks; +using Microsoft.DotNet.Interactive.Commands; + +namespace Microsoft.DotNet.Interactive.Tests.Utility +{ + public class TestCommand : KernelCommand + { + private readonly Func _handler; + + public TestCommand(Func handler, string targetKernelName = null, KernelCommand parent = null) : base(targetKernelName, parent) + { + _handler = handler ?? throw new ArgumentNullException(nameof(handler)); + } + + public override Task InvokeAsync(KernelInvocationContext context) + { + return _handler(this, context); + } + } +} \ No newline at end of file diff --git a/src/Microsoft.DotNet.Interactive/Kernel.cs b/src/Microsoft.DotNet.Interactive/Kernel.cs index 42bc0f89e1..da22aac38a 100644 --- a/src/Microsoft.DotNet.Interactive/Kernel.cs +++ b/src/Microsoft.DotNet.Interactive/Kernel.cs @@ -2,7 +2,6 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; -using System.Collections.Concurrent; using System.Collections.Generic; using System.CommandLine; using System.CommandLine.Parsing; @@ -18,7 +17,6 @@ using Microsoft.DotNet.Interactive.Events; using Microsoft.DotNet.Interactive.Parsing; using Microsoft.DotNet.Interactive.Server; -using Microsoft.DotNet.Interactive.Utility; namespace Microsoft.DotNet.Interactive { @@ -26,9 +24,7 @@ public abstract partial class Kernel : IDisposable { private readonly Subject _kernelEvents = new(); private readonly CompositeDisposable _disposables; - private readonly ConcurrentQueue _deferredCommands = new(); - - private readonly ConcurrentQueue _commandQueue = new(); + private readonly Dictionary _dynamicHandlers = new(); private FrontendEnvironment _frontendEnvironment; private ChooseKernelDirective _chooseKernelDirective; @@ -227,31 +223,6 @@ public void RegisterCommandType() KernelCommandEnvelope.RegisterCommandTypeReplacingIfNecessary(); } - private class KernelOperation - { - public KernelOperation( - KernelCommand command, - TaskCompletionSource taskCompletionSource, - bool isDeferred) - { - Command = command; - TaskCompletionSource = taskCompletionSource; - IsDeferred = isDeferred; - - AsyncContext.TryEstablish(out var id); - AsyncContextId = id; - } - - public KernelCommand Command { get; } - - public TaskCompletionSource TaskCompletionSource { get; } - public bool IsDeferred { get; } - - public int AsyncContextId { get; } - } - - - internal virtual async Task HandleAsync( KernelCommand command, KernelInvocationContext context) @@ -288,7 +259,7 @@ protected internal void CancelCommands() internal Task RunDeferredCommandsAsync() { - return Scheduler.RunDeferredCommandsAsync(); + return Scheduler.RunDeferredCommandsAsync(this); } diff --git a/src/Microsoft.DotNet.Interactive/KernelCommandScheduler.cs b/src/Microsoft.DotNet.Interactive/KernelCommandScheduler.cs index b2f4817353..ccdbaa5aad 100644 --- a/src/Microsoft.DotNet.Interactive/KernelCommandScheduler.cs +++ b/src/Microsoft.DotNet.Interactive/KernelCommandScheduler.cs @@ -8,6 +8,7 @@ using System.Reactive.Disposables; using System.Threading; using System.Threading.Tasks; + using Microsoft.DotNet.Interactive.Commands; using Microsoft.DotNet.Interactive.Utility; @@ -18,7 +19,14 @@ public class KernelCommandScheduler private readonly ConcurrentQueue<(KernelCommand command, Kernel kernel)> _deferredCommands = new(); private readonly ConcurrentQueue _commandQueue = new(); - public Task Schedule(KernelCommand command, Kernel kernel, CancellationToken cancellationToken, Action onDone) + + public Task Schedule(KernelCommand command, Kernel kernel) + { + return Schedule(command, kernel, CancellationToken.None, () => { }); + } + + + public Task Schedule(KernelCommand command, Kernel kernel, CancellationToken cancellationToken, Action onDone) { @@ -28,15 +36,15 @@ public Task Schedule(KernelCommand command, Kernel kernel, CancelCommands(); break; default: - UndeferCommands(); + UndeferCommandsFor(kernel); break; } var kernelCommandResultSource = new TaskCompletionSource(); - var operation = new KernelOperation(command, kernelCommandResultSource, kernel,false); + var operation = new KernelOperation(command, kernelCommandResultSource, kernel, false); _commandQueue.Enqueue(operation); - + ProcessCommandQueue(_commandQueue, cancellationToken, onDone); return kernelCommandResultSource.Task; @@ -127,13 +135,13 @@ public KernelOperation( public void DeferCommand(KernelCommand command, Kernel kernel) { - _deferredCommands.Enqueue((command,kernel)); + _deferredCommands.Enqueue((command, kernel)); } - internal Task RunDeferredCommandsAsync() + internal Task RunDeferredCommandsAsync(Kernel kernel) { var tcs = new TaskCompletionSource(); - UndeferCommands(); + UndeferCommandsFor(kernel); ProcessCommandQueue( _commandQueue, CancellationToken.None, @@ -154,7 +162,46 @@ private void UndeferCommands() } } + private void UndeferCommandsFor(Kernel kernel) + { + var commandsToKeepInDeferredList = new ConcurrentQueue<(KernelCommand command, Kernel kernel)>(); + while (_deferredCommands.TryDequeue(out var deferredCommand)) + { + if (IsInPath(kernel, deferredCommand.kernel)) + { + _commandQueue.Enqueue( + new KernelOperation( + deferredCommand.command, + new TaskCompletionSource(), + deferredCommand.kernel, + true)); + } + else + { + commandsToKeepInDeferredList.Enqueue(deferredCommand); + } + } + + while (commandsToKeepInDeferredList.TryDequeue(out var deferredCommand)) + { + _deferredCommands.Enqueue(deferredCommand); + } + + + bool IsInPath(Kernel toTest, Kernel deferredCommandKernel) + { + while (toTest is not null) + { + if (toTest == deferredCommandKernel) + { + return true; + } + toTest = toTest.ParentKernel; + } + return false; + } + } private static bool CanCancel(KernelCommand command) { diff --git a/src/dotnet-interactive/KernelExtensions.cs b/src/dotnet-interactive/KernelExtensions.cs index e5e763a8c1..9f4b7702ac 100644 --- a/src/dotnet-interactive/KernelExtensions.cs +++ b/src/dotnet-interactive/KernelExtensions.cs @@ -15,7 +15,7 @@ namespace Microsoft.DotNet.Interactive.App { public static class KernelExtensions { - public static T UseAbout(this T kernel) + public static T UseAboutMagicCommand(this T kernel) where T : Kernel { var about = new Command("#!about", "Show version and build information") From 16d0d558ef18f84c6ce3291a0be3d2f97dd8553b Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Thu, 18 Feb 2021 21:53:07 +0000 Subject: [PATCH 28/44] experiment --- .../CancelCommandTests.cs | 16 ++++++++-------- src/Microsoft.DotNet.Interactive/Kernel.cs | 5 +++-- .../KernelCommandScheduler.cs | 16 ++++++---------- 3 files changed, 17 insertions(+), 20 deletions(-) diff --git a/src/Microsoft.DotNet.Interactive.Tests/CancelCommandTests.cs b/src/Microsoft.DotNet.Interactive.Tests/CancelCommandTests.cs index dd631317d5..86f0fa120b 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/CancelCommandTests.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/CancelCommandTests.cs @@ -60,9 +60,9 @@ public async Task cancel_command_cancels_all_deferred_commands_on_composite_kern } [Theory] - [InlineData(Language.CSharp)] - [InlineData(Language.FSharp)] - [InlineData(Language.PowerShell)] + [InlineData(Language.CSharp, Skip = "requires scheduler working")] + [InlineData(Language.FSharp, Skip = "requires scheduler working")] + [InlineData(Language.PowerShell, Skip = "requires scheduler working")] public async Task cancel_command_cancels_all_deferred_commands_on_subkernels(Language language) { @@ -92,8 +92,8 @@ public async Task cancel_command_cancels_all_deferred_commands_on_subkernels(Lan } [Theory] - [InlineData(Language.CSharp)] - [InlineData(Language.FSharp)] + [InlineData(Language.CSharp, Skip = "requires scheduler working")] + [InlineData(Language.FSharp, Skip = "requires scheduler working")] [InlineData(Language.PowerShell, Skip = "to address later")] public void cancel_command_cancels_the_running_command(Language language) { @@ -132,8 +132,8 @@ public void cancel_command_cancels_the_running_command(Language language) } [Theory] - [InlineData(Language.CSharp)] - [InlineData(Language.FSharp)] + [InlineData(Language.CSharp, Skip = "requires scheduler working")] + [InlineData(Language.FSharp, Skip = "requires scheduler working")] [InlineData(Language.PowerShell, Skip = "to address later")] public async Task commands_issued_after_cancel_command_are_executed(Language language) { @@ -174,7 +174,7 @@ public async Task commands_issued_after_cancel_command_are_executed(Language lan break; } } -", "done c#")] +", "done c#", Skip = "requires scheduler working")] [InlineData(Language.FSharp, @" System.Threading.Thread.Sleep(3000) Console.WriteLine(""done c#"")", "done f#", Skip = "for the moment")] diff --git a/src/Microsoft.DotNet.Interactive/Kernel.cs b/src/Microsoft.DotNet.Interactive/Kernel.cs index da22aac38a..1af18dc764 100644 --- a/src/Microsoft.DotNet.Interactive/Kernel.cs +++ b/src/Microsoft.DotNet.Interactive/Kernel.cs @@ -28,6 +28,7 @@ public abstract partial class Kernel : IDisposable private readonly Dictionary _dynamicHandlers = new(); private FrontendEnvironment _frontendEnvironment; private ChooseKernelDirective _chooseKernelDirective; + private readonly KernelCommandScheduler _scheduler; protected Kernel(string name) { @@ -44,7 +45,7 @@ protected Kernel(string name) Pipeline = new KernelCommandPipeline(this); - Scheduler = new KernelCommandScheduler(); + _scheduler = new KernelCommandScheduler(); AddSetKernelMiddleware(); @@ -55,7 +56,7 @@ protected Kernel(string name) )); } - internal KernelCommandScheduler Scheduler { get; } + internal KernelCommandScheduler Scheduler => _scheduler; internal KernelCommandPipeline Pipeline { get; } diff --git a/src/Microsoft.DotNet.Interactive/KernelCommandScheduler.cs b/src/Microsoft.DotNet.Interactive/KernelCommandScheduler.cs index ccdbaa5aad..72dcb7ec50 100644 --- a/src/Microsoft.DotNet.Interactive/KernelCommandScheduler.cs +++ b/src/Microsoft.DotNet.Interactive/KernelCommandScheduler.cs @@ -42,7 +42,8 @@ public Task Schedule(KernelCommand command, Kernel kernel, var kernelCommandResultSource = new TaskCompletionSource(); - var operation = new KernelOperation(command, kernelCommandResultSource, kernel, false); + var operation = new KernelOperation(command, kernelCommandResultSource, kernel); + _commandQueue.Enqueue(operation); ProcessCommandQueue(_commandQueue, cancellationToken, onDone); @@ -112,13 +113,11 @@ private class KernelOperation public KernelOperation( KernelCommand command, TaskCompletionSource taskCompletionSource, - Kernel kernel, - bool isDeferred) + Kernel kernel) { Command = command; TaskCompletionSource = taskCompletionSource; Kernel = kernel; - IsDeferred = isDeferred; AsyncContext.TryEstablish(out var id); AsyncContextId = id; @@ -128,7 +127,6 @@ public KernelOperation( public TaskCompletionSource TaskCompletionSource { get; } public Kernel Kernel { get; } - public bool IsDeferred { get; } public int AsyncContextId { get; } } @@ -157,8 +155,7 @@ private void UndeferCommands() new KernelOperation( initCommand.command, new TaskCompletionSource(), - initCommand.kernel, - true)); + initCommand.kernel)); } } @@ -173,8 +170,7 @@ private void UndeferCommandsFor(Kernel kernel) new KernelOperation( deferredCommand.command, new TaskCompletionSource(), - deferredCommand.kernel, - true)); + deferredCommand.kernel)); } else { @@ -221,7 +217,7 @@ public void CancelCommands() } using var disposables = new CompositeDisposable(); - var inFlightOperations = _commandQueue.Where(operation => !operation.IsDeferred && CanCancel(operation.Command)).ToList(); + var inFlightOperations = _commandQueue.Where(operation => CanCancel(operation.Command)).ToList(); foreach (var inFlightOperation in inFlightOperations) { KernelInvocationContext currentContext = null; From d32c1228209bab7cede211471e61e25ec245ac26 Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Fri, 19 Feb 2021 03:12:40 +0000 Subject: [PATCH 29/44] new scheduler api --- .../KernelCommandSchedulerTests.cs | 330 +++++++++++------- .../KernelScheduler.cs | 127 +++++++ 2 files changed, 322 insertions(+), 135 deletions(-) create mode 100644 src/Microsoft.DotNet.Interactive/KernelScheduler.cs diff --git a/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs b/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs index 811c3b8e6e..c8089c3593 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs @@ -50,164 +50,224 @@ private void DisposeAfterTest(Action action) } [Fact] - public async Task command_execute_on_kernel_specified_at_scheduling_time() + public async Task scheduled_work_is_completed_in_order() { - var commandsHandledOnKernel1 = new List(); - var commandsHandledOnKernel2 = new List(); + var scheduler = new KernelScheduler(); - var scheduler = new KernelCommandScheduler(); + var executionList = new List(); - var kernel1 = new FakeKernel("kernel1") - { - Handle = (command, _) => - { - commandsHandledOnKernel1.Add(command); - return Task.CompletedTask; - } - }; - var kernel2 = new FakeKernel("kernel2") - { - Handle = (command, _) => - { - commandsHandledOnKernel2.Add(command); - return Task.CompletedTask; - } - }; + await scheduler.Schedule(1, (v) => executionList.Add(v)); + await scheduler.Schedule(2, (v) => executionList.Add(v)); + await scheduler.Schedule(3, (v) => executionList.Add(v)); - var command1 = new SubmitCode("for kernel 1"); - var command2 = new SubmitCode("for kernel 2"); - await scheduler.Schedule(command1, kernel1); - await scheduler.Schedule(command2, kernel2); + executionList.Should().BeEquivalentSequenceTo(1, 2, 3); + } - commandsHandledOnKernel1.Should().ContainSingle().Which.Should().Be(command1); - commandsHandledOnKernel2.Should().ContainSingle().Which.Should().Be(command2); + [Fact] + public void scheduled_work_does_not_execute_in_parallel() + { + throw new NotImplementedException(); } [Fact] - public async Task scheduling_a_command_will_defer_deferred_commands_scheduled_on_same_kernel() + public async Task deferred_work_is_executed_before_new_work() { - var commandsHandledOnKernel1 = new List(); + var executionList = new List(); - var scheduler = new KernelCommandScheduler(); + var scheduler = new KernelScheduler(); + scheduler.RegisterDeferredOperationSource( + v => Enumerable.Repeat(v * 10, v), (v) => executionList.Add(v)); - var kernel1 = new FakeKernel("kernel1") - { - Handle = (command, _) => - { - commandsHandledOnKernel1.Add(command); - return Task.CompletedTask; - } - }; - var kernel2 = new FakeKernel("kernel2") - { - Handle = (_, _) => Task.CompletedTask - }; - - var deferredCommand1 = new SubmitCode("deferred for kernel 1"); - var deferredCommand2 = new SubmitCode("deferred for kernel 2"); - var deferredCommand3 = new SubmitCode("deferred for kernel 1"); - var command1 = new SubmitCode("for kernel 1"); + - scheduler.DeferCommand(deferredCommand1, kernel1); - scheduler.DeferCommand(deferredCommand2, kernel2); - scheduler.DeferCommand(deferredCommand3, kernel1); - await scheduler.Schedule(command1, kernel1); + await scheduler.Schedule(1, (v) => executionList.Add(v)); + await scheduler.Schedule(2, (v) => executionList.Add(v)); + await scheduler.Schedule(3, (v) => executionList.Add(v)); - commandsHandledOnKernel1.Should().NotContain(deferredCommand2); - commandsHandledOnKernel1.Should().BeEquivalentSequenceTo(deferredCommand1, deferredCommand3, command1); + executionList.Should().BeEquivalentSequenceTo(10,1,20,20, 2,30,30,30, 3); } [Fact] - public async Task deferred_command_not_executed_are_still_in_deferred_queue() + public async Task awaiting_one_operation_does_not_wait_all() { - var commandsHandledOnKernel1 = new List(); - var commandsHandledOnKernel2 = new List(); + var executionList = new List(); - var scheduler = new KernelCommandScheduler(); + var scheduler = new KernelScheduler(); - var kernel1 = new FakeKernel("kernel1") - { - Handle = (command, _) => - { - commandsHandledOnKernel1.Add(command); - return Task.CompletedTask; - } - }; - var kernel2 = new FakeKernel("kernel2") - { - Handle = (command, _) => - { - commandsHandledOnKernel2.Add(command); - return Task.CompletedTask; - } - }; - - var deferredCommand1 = new SubmitCode("deferred for kernel 1"); - var deferredCommand2 = new SubmitCode("deferred for kernel 2"); - var deferredCommand3 = new SubmitCode("deferred for kernel 1"); - var command1 = new SubmitCode("for kernel 1"); - var command2 = new SubmitCode("for kernel 2"); - - scheduler.DeferCommand(deferredCommand1, kernel1); - scheduler.DeferCommand(deferredCommand2, kernel2); - scheduler.DeferCommand(deferredCommand3, kernel1); - await scheduler.Schedule(command1, kernel1); - - commandsHandledOnKernel2.Should().BeEmpty(); - commandsHandledOnKernel1.Should().NotContain(deferredCommand2); - commandsHandledOnKernel1.Should().BeEquivalentSequenceTo(deferredCommand1, deferredCommand3, command1); - await scheduler.Schedule(command2, kernel2); - commandsHandledOnKernel2.Should().BeEquivalentSequenceTo(deferredCommand2, command2); + + await scheduler.Schedule(1, (v) => executionList.Add(v)); + await scheduler.Schedule(2, (v) => executionList.Add(v)); + scheduler.Schedule(3, (v) => executionList.Add(v)); + + executionList.Should().BeEquivalentSequenceTo(10, 1, 20, 20, 2); } [Fact] - public async Task deferred_command_on_parent_kernel_are_executed_when_scheduling_command_on_child_kernel() + public void new_work_is_executed_after_all_require() { - var commandHandledList = new List<(KernelCommand command, Kernel kernel)>(); - - var scheduler = new KernelCommandScheduler(); - - var childKernel = new FakeKernel("kernel1") - { - Handle = (command, context) => command.InvokeAsync(context) - }; - var parentKernel = new CompositeKernel - { - childKernel - }; - - parentKernel.DefaultKernelName = childKernel.Name; - - var deferredCommand1 = new TestCommand((command, context) => - { - commandHandledList.Add((command, context.HandlingKernel)); - return Task.CompletedTask; - }, childKernel.Name); - var deferredCommand2 = new TestCommand((command, context) => - { - commandHandledList.Add((command, context.HandlingKernel)); - return Task.CompletedTask; - }, parentKernel.Name); - var deferredCommand3 = new TestCommand((command, context) => - { - commandHandledList.Add((command, context.HandlingKernel)); - return Task.CompletedTask; - }, childKernel.Name); - var command1 = new TestCommand((command, context) => - { - commandHandledList.Add((command, context.HandlingKernel)); - return Task.CompletedTask; - }, childKernel.Name); - - scheduler.DeferCommand(deferredCommand1, childKernel); - scheduler.DeferCommand(deferredCommand2, parentKernel); - scheduler.DeferCommand(deferredCommand3, childKernel); - await scheduler.Schedule(command1, childKernel); - - commandHandledList.Select(e => e.command).Should().BeEquivalentSequenceTo(deferredCommand1, deferredCommand2, deferredCommand3, command1); - - commandHandledList.Select(e => e.kernel).Should().BeEquivalentSequenceTo(childKernel, parentKernel, childKernel, childKernel); + throw new NotImplementedException(); } + + //[Fact] + //public async Task command_execute_on_kernel_specified_at_scheduling_time() + //{ + // var commandsHandledOnKernel1 = new List(); + // var commandsHandledOnKernel2 = new List(); + + // var scheduler = new KernelCommandScheduler(); + + // var kernel1 = new FakeKernel("kernel1") + // { + // Handle = (command, _) => + // { + // commandsHandledOnKernel1.Add(command); + // return Task.CompletedTask; + // } + // }; + // var kernel2 = new FakeKernel("kernel2") + // { + // Handle = (command, _) => + // { + // commandsHandledOnKernel2.Add(command); + // return Task.CompletedTask; + // } + // }; + + // var command1 = new SubmitCode("for kernel 1", kernel1.Name); + // var command2 = new SubmitCode("for kernel 2", kernel2.Name); + + // await scheduler.Schedule(command1); + // await scheduler.Schedule(command2); + + // commandsHandledOnKernel1.Should().ContainSingle().Which.Should().Be(command1); + // commandsHandledOnKernel2.Should().ContainSingle().Which.Should().Be(command2); + //} + + //[Fact] + //public async Task scheduling_a_command_will_defer_deferred_commands_scheduled_on_same_kernel() + //{ + // var commandsHandledOnKernel1 = new List(); + + // var scheduler = new KernelCommandScheduler(); + + // var kernel1 = new FakeKernel("kernel1") + // { + // Handle = (command, _) => + // { + // commandsHandledOnKernel1.Add(command); + // return Task.CompletedTask; + // } + // }; + // var kernel2 = new FakeKernel("kernel2") + // { + // Handle = (_, _) => Task.CompletedTask + // }; + + // var deferredCommand1 = new SubmitCode("deferred for kernel 1", kernel1.Name); + // var deferredCommand2 = new SubmitCode("deferred for kernel 2", kernel2.Name); + // var deferredCommand3 = new SubmitCode("deferred for kernel 1", kernel1.Name); + // var command1 = new SubmitCode("for kernel 1", kernel1.Name); + + // scheduler.DeferCommand(deferredCommand1); + // scheduler.DeferCommand(deferredCommand2); + // scheduler.DeferCommand(deferredCommand3); + // await scheduler.Schedule(command1); + + // commandsHandledOnKernel1.Should().NotContain(deferredCommand2); + // commandsHandledOnKernel1.Should().BeEquivalentSequenceTo(deferredCommand1, deferredCommand3, command1); + //} + + //[Fact] + //public async Task deferred_command_not_executed_are_still_in_deferred_queue() + //{ + // var commandsHandledOnKernel1 = new List(); + // var commandsHandledOnKernel2 = new List(); + + // var scheduler = new KernelCommandScheduler(); + + // var kernel1 = new FakeKernel("kernel1") + // { + // Handle = (command, _) => + // { + // commandsHandledOnKernel1.Add(command); + // return Task.CompletedTask; + // } + // }; + // var kernel2 = new FakeKernel("kernel2") + // { + // Handle = (command, _) => + // { + // commandsHandledOnKernel2.Add(command); + // return Task.CompletedTask; + // } + // }; + + // var deferredCommand1 = new SubmitCode("deferred for kernel 1"); + // var deferredCommand2 = new SubmitCode("deferred for kernel 2"); + // var deferredCommand3 = new SubmitCode("deferred for kernel 1"); + // var command1 = new SubmitCode("for kernel 1"); + // var command2 = new SubmitCode("for kernel 2"); + + // scheduler.DeferCommand(deferredCommand1, kernel1); + // scheduler.DeferCommand(deferredCommand2, kernel2); + // scheduler.DeferCommand(deferredCommand3, kernel1); + // await scheduler.Schedule(command1, kernel1); + + // commandsHandledOnKernel2.Should().BeEmpty(); + // commandsHandledOnKernel1.Should().NotContain(deferredCommand2); + // commandsHandledOnKernel1.Should().BeEquivalentSequenceTo(deferredCommand1, deferredCommand3, command1); + // await scheduler.Schedule(command2, kernel2); + // commandsHandledOnKernel2.Should().BeEquivalentSequenceTo(deferredCommand2, command2); + //} + + //[Fact] + //public async Task deferred_command_on_parent_kernel_are_executed_when_scheduling_command_on_child_kernel() + //{ + // var commandHandledList = new List<(KernelCommand command, Kernel kernel)>(); + + // var scheduler = new KernelCommandScheduler(); + + // var childKernel = new FakeKernel("kernel1") + // { + // Handle = (command, context) => command.InvokeAsync(context) + // }; + // var parentKernel = new CompositeKernel + // { + // childKernel + // }; + + // parentKernel.DefaultKernelName = childKernel.Name; + + // var deferredCommand1 = new TestCommand((command, context) => + // { + // commandHandledList.Add((command, context.HandlingKernel)); + // return Task.CompletedTask; + // }, childKernel.Name); + // var deferredCommand2 = new TestCommand((command, context) => + // { + // commandHandledList.Add((command, context.HandlingKernel)); + // return Task.CompletedTask; + // }, parentKernel.Name); + // var deferredCommand3 = new TestCommand((command, context) => + // { + // commandHandledList.Add((command, context.HandlingKernel)); + // return Task.CompletedTask; + // }, childKernel.Name); + // var command1 = new TestCommand((command, context) => + // { + // commandHandledList.Add((command, context.HandlingKernel)); + // return Task.CompletedTask; + // }, childKernel.Name); + + // scheduler.DeferCommand(deferredCommand1, childKernel); + // scheduler.DeferCommand(deferredCommand2, parentKernel); + // scheduler.DeferCommand(deferredCommand3, childKernel); + // await scheduler.Schedule(command1, childKernel); + + // commandHandledList.Select(e => e.command).Should().BeEquivalentSequenceTo(deferredCommand1, deferredCommand2, deferredCommand3, command1); + + // commandHandledList.Select(e => e.kernel).Should().BeEquivalentSequenceTo(childKernel, parentKernel, childKernel, childKernel); + //} } -} \ No newline at end of file + } \ No newline at end of file diff --git a/src/Microsoft.DotNet.Interactive/KernelScheduler.cs b/src/Microsoft.DotNet.Interactive/KernelScheduler.cs new file mode 100644 index 0000000000..f46d03b459 --- /dev/null +++ b/src/Microsoft.DotNet.Interactive/KernelScheduler.cs @@ -0,0 +1,127 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Reactive.Concurrency; +using System.Threading.Tasks; + +namespace Microsoft.DotNet.Interactive +{ + public class KernelScheduler + { + private readonly IScheduler _executionScheduler = TaskPoolScheduler.Default; + + private readonly List _scheduledOperations = new(); + private readonly List<(Func> source , OnExecuteDelegate handler)> _deferredOperationSources = new(); + + public Task Schedule(T value, OnExecuteDelegate onExecuteAsync) + { + var operation = new ScheduledOperation(value, onExecuteAsync); + + lock (_scheduledOperations) + + { + _scheduledOperations.Add(operation); + } + + _executionScheduler.Schedule(ProcessScheduledOperations); + + + return operation.Task; + } + + private void ProcessScheduledOperations() + { + ScheduledOperation operation; + lock (_scheduledOperations) + { if(_scheduledOperations.Count > 0) + { + operation = _scheduledOperations[0]; + _scheduledOperations.RemoveAt(0); + } + else + { + return; + } + } + + if(operation is not null) + { + // get all deferred operations and pump in + foreach (var deferred in _deferredOperationSources.SelectMany(ds => ds.source(operation.Value).Select(d => (value:d, onExecuteAsync: ds.handler)))) + { + var deferredOperation = new ScheduledOperation(deferred.value, deferred.onExecuteAsync); + _executionScheduler.Schedule(() => doWork(deferredOperation)); + } + + _executionScheduler.Schedule(() => doWork(operation) ); + } + + _executionScheduler.Schedule(ProcessScheduledOperations); + + static void doWork(ScheduledOperation operation) + { + try + { + operation.OnExecuteAsync(operation.Value); + operation.CompletionSource.SetResult(default); + } + catch (Exception e) + { + operation.CompletionSource.SetException(e); + } + } + } + + public delegate Task OnExecuteDelegate(T value); + + private class ScheduledOperation + { + public T Value { get; } + public OnExecuteDelegate OnExecuteAsync { get; } + public Task Task => CompletionSource.Task; + + + public ScheduledOperation(T value, OnExecuteDelegate onExecuteAsync) + { + Value = value; + + CompletionSource = new TaskCompletionSource(); + + OnExecuteAsync = onExecuteAsync; + } + + public TaskCompletionSource CompletionSource { get; } + } + + public void RegisterDeferredOperationSource(Func> deferredOperationSource, OnExecuteDelegate onExecuteAsync) + { + _deferredOperationSources.Add((deferredOperationSource, onExecuteAsync)); + } + } + + + + public static class KernelSchedulerExtensions + { + public static Task Schedule(this KernelScheduler kernelScheduler, T value, Action onExecute) + { + return kernelScheduler.Schedule(value, v => + { + onExecute(v); + return Task.CompletedTask; + }); + } + + public static void RegisterDeferredOperationSource(this KernelScheduler kernelScheduler, Func> deferredOperationSource, Action onExecute) + { + kernelScheduler.RegisterDeferredOperationSource(deferredOperationSource, v => + { + onExecute(v); + return Task.CompletedTask; + }); + } + } +} \ No newline at end of file From 37e9b31b37b150e1aaba8f8937b868952b79d073 Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Mon, 22 Feb 2021 10:46:38 +0000 Subject: [PATCH 30/44] scheduled work must be awaited before completing the source --- .../KernelCommandSchedulerTests.cs | 21 ++++++++-- .../KernelScheduler.cs | 39 ++++++++++++------- 2 files changed, 43 insertions(+), 17 deletions(-) diff --git a/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs b/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs index c8089c3593..1c8a502184 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs @@ -96,11 +96,24 @@ public async Task awaiting_one_operation_does_not_wait_all() var scheduler = new KernelScheduler(); - await scheduler.Schedule(1, (v) => executionList.Add(v)); - await scheduler.Schedule(2, (v) => executionList.Add(v)); - scheduler.Schedule(3, (v) => executionList.Add(v)); + await scheduler.Schedule(1, async (v) => + { + await Task.Delay(10); + executionList.Add(v); + }); + await scheduler.Schedule(2, async (v) => + { + await Task.Delay(10); + executionList.Add(v); + }); + + scheduler.Schedule(3, async (v) => + { + await Task.Delay(200); + executionList.Add(v); + }); - executionList.Should().BeEquivalentSequenceTo(10, 1, 20, 20, 2); + executionList.Should().BeEquivalentSequenceTo( 1, 2); } [Fact] diff --git a/src/Microsoft.DotNet.Interactive/KernelScheduler.cs b/src/Microsoft.DotNet.Interactive/KernelScheduler.cs index f46d03b459..a8de68521b 100644 --- a/src/Microsoft.DotNet.Interactive/KernelScheduler.cs +++ b/src/Microsoft.DotNet.Interactive/KernelScheduler.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; -using System.Linq; using System.Reactive.Concurrency; using System.Threading.Tasks; @@ -14,7 +13,7 @@ public class KernelScheduler private readonly IScheduler _executionScheduler = TaskPoolScheduler.Default; private readonly List _scheduledOperations = new(); - private readonly List<(Func> source , OnExecuteDelegate handler)> _deferredOperationSources = new(); + private readonly List _deferredOperationRegistrations = new(); public Task Schedule(T value, OnExecuteDelegate onExecuteAsync) { @@ -50,18 +49,21 @@ private void ProcessScheduledOperations() if(operation is not null) { // get all deferred operations and pump in - foreach (var deferred in _deferredOperationSources.SelectMany(ds => ds.source(operation.Value).Select(d => (value:d, onExecuteAsync: ds.handler)))) + foreach (var deferredOperationRegistration in _deferredOperationRegistrations) { - var deferredOperation = new ScheduledOperation(deferred.value, deferred.onExecuteAsync); - _executionScheduler.Schedule(() => doWork(deferredOperation)); + foreach (var deferred in deferredOperationRegistration.GetDeferredOperations(operation.Value)) + { + var deferredOperation = new ScheduledOperation(deferred, deferredOperationRegistration.OnExecute); + _executionScheduler.Schedule(() => DoWork(deferredOperation)); + } } - _executionScheduler.Schedule(() => doWork(operation) ); + _executionScheduler.Schedule(() => DoWork(operation) ); } _executionScheduler.Schedule(ProcessScheduledOperations); - static void doWork(ScheduledOperation operation) + static void DoWork(ScheduledOperation operation) { try { @@ -77,6 +79,8 @@ static void doWork(ScheduledOperation operation) public delegate Task OnExecuteDelegate(T value); + public delegate IEnumerable GetDeferredOperationsDelegate(T operationToExecute); + private class ScheduledOperation { public T Value { get; } @@ -87,18 +91,27 @@ private class ScheduledOperation public ScheduledOperation(T value, OnExecuteDelegate onExecuteAsync) { Value = value; - CompletionSource = new TaskCompletionSource(); - OnExecuteAsync = onExecuteAsync; } public TaskCompletionSource CompletionSource { get; } } - public void RegisterDeferredOperationSource(Func> deferredOperationSource, OnExecuteDelegate onExecuteAsync) + private class DeferredOperation + { + public GetDeferredOperationsDelegate GetDeferredOperations { get; } + public OnExecuteDelegate OnExecute { get; } + public DeferredOperation(OnExecuteDelegate onExecute, GetDeferredOperationsDelegate getDeferredOperations) + { + OnExecute = onExecute; + GetDeferredOperations = getDeferredOperations; + } + } + + public void RegisterDeferredOperationSource(GetDeferredOperationsDelegate getDeferredOperations, OnExecuteDelegate onExecuteAsync) { - _deferredOperationSources.Add((deferredOperationSource, onExecuteAsync)); + _deferredOperationRegistrations.Add(new DeferredOperation(onExecuteAsync,getDeferredOperations)); } } @@ -115,9 +128,9 @@ public static Task Schedule(this KernelScheduler kernelScheduler, T }); } - public static void RegisterDeferredOperationSource(this KernelScheduler kernelScheduler, Func> deferredOperationSource, Action onExecute) + public static void RegisterDeferredOperationSource(this KernelScheduler kernelScheduler, KernelScheduler.GetDeferredOperationsDelegate getDeferredOperations, Action onExecute) { - kernelScheduler.RegisterDeferredOperationSource(deferredOperationSource, v => + kernelScheduler.RegisterDeferredOperationSource(getDeferredOperations, v => { onExecute(v); return Task.CompletedTask; From 7368e0d29dbfcadd4254441136b9ffd1735de22d Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Mon, 22 Feb 2021 10:54:20 +0000 Subject: [PATCH 31/44] cleanup --- .../KernelCommandSchedulerTests.cs | 12 +++---- .../KernelScheduler.cs | 31 +++---------------- .../KernelSchedulerExtensions.cs | 29 +++++++++++++++++ 3 files changed, 38 insertions(+), 34 deletions(-) create mode 100644 src/Microsoft.DotNet.Interactive/KernelSchedulerExtensions.cs diff --git a/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs b/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs index 1c8a502184..a4f5d251ce 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs @@ -79,8 +79,6 @@ public async Task deferred_work_is_executed_before_new_work() scheduler.RegisterDeferredOperationSource( v => Enumerable.Repeat(v * 10, v), (v) => executionList.Add(v)); - - await scheduler.Schedule(1, (v) => executionList.Add(v)); await scheduler.Schedule(2, (v) => executionList.Add(v)); await scheduler.Schedule(3, (v) => executionList.Add(v)); @@ -107,11 +105,11 @@ await scheduler.Schedule(2, async (v) => executionList.Add(v); }); - scheduler.Schedule(3, async (v) => - { - await Task.Delay(200); - executionList.Add(v); - }); + _ = scheduler.Schedule(3, async (v) => + { + await Task.Delay(200); + executionList.Add(v); + }); executionList.Should().BeEquivalentSequenceTo( 1, 2); } diff --git a/src/Microsoft.DotNet.Interactive/KernelScheduler.cs b/src/Microsoft.DotNet.Interactive/KernelScheduler.cs index a8de68521b..1c51fa5b10 100644 --- a/src/Microsoft.DotNet.Interactive/KernelScheduler.cs +++ b/src/Microsoft.DotNet.Interactive/KernelScheduler.cs @@ -54,20 +54,20 @@ private void ProcessScheduledOperations() foreach (var deferred in deferredOperationRegistration.GetDeferredOperations(operation.Value)) { var deferredOperation = new ScheduledOperation(deferred, deferredOperationRegistration.OnExecute); - _executionScheduler.Schedule(() => DoWork(deferredOperation)); + _executionScheduler.Schedule(async () => await DoWork(deferredOperation)); } } - _executionScheduler.Schedule(() => DoWork(operation) ); + _executionScheduler.Schedule(async () => await DoWork(operation) ); } _executionScheduler.Schedule(ProcessScheduledOperations); - static void DoWork(ScheduledOperation operation) + static async Task DoWork(ScheduledOperation operation) { try { - operation.OnExecuteAsync(operation.Value); + await operation.OnExecuteAsync(operation.Value); operation.CompletionSource.SetResult(default); } catch (Exception e) @@ -114,27 +114,4 @@ public void RegisterDeferredOperationSource(GetDeferredOperationsDelegate getDef _deferredOperationRegistrations.Add(new DeferredOperation(onExecuteAsync,getDeferredOperations)); } } - - - - public static class KernelSchedulerExtensions - { - public static Task Schedule(this KernelScheduler kernelScheduler, T value, Action onExecute) - { - return kernelScheduler.Schedule(value, v => - { - onExecute(v); - return Task.CompletedTask; - }); - } - - public static void RegisterDeferredOperationSource(this KernelScheduler kernelScheduler, KernelScheduler.GetDeferredOperationsDelegate getDeferredOperations, Action onExecute) - { - kernelScheduler.RegisterDeferredOperationSource(getDeferredOperations, v => - { - onExecute(v); - return Task.CompletedTask; - }); - } - } } \ No newline at end of file diff --git a/src/Microsoft.DotNet.Interactive/KernelSchedulerExtensions.cs b/src/Microsoft.DotNet.Interactive/KernelSchedulerExtensions.cs new file mode 100644 index 0000000000..e7bb6af75b --- /dev/null +++ b/src/Microsoft.DotNet.Interactive/KernelSchedulerExtensions.cs @@ -0,0 +1,29 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; +using System.Threading.Tasks; + +namespace Microsoft.DotNet.Interactive +{ + public static class KernelSchedulerExtensions + { + public static Task Schedule(this KernelScheduler kernelScheduler, T value, Action onExecute) + { + return kernelScheduler.Schedule(value, v => + { + onExecute(v); + return Task.CompletedTask; + }); + } + + public static void RegisterDeferredOperationSource(this KernelScheduler kernelScheduler, KernelScheduler.GetDeferredOperationsDelegate getDeferredOperations, Action onExecute) + { + kernelScheduler.RegisterDeferredOperationSource(getDeferredOperations, v => + { + onExecute(v); + return Task.CompletedTask; + }); + } + } +} \ No newline at end of file From 50a9bffcf9be940034c395d89ade283b578b336d Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Mon, 22 Feb 2021 12:02:02 +0000 Subject: [PATCH 32/44] cancel work on scheduler --- .../KernelCommandSchedulerTests.cs | 33 +++++++++++++ .../KernelScheduler.cs | 48 +++++++++++++++++-- 2 files changed, 78 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs b/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs index a4f5d251ce..b3546ee6de 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs @@ -86,6 +86,39 @@ public async Task deferred_work_is_executed_before_new_work() executionList.Should().BeEquivalentSequenceTo(10,1,20,20, 2,30,30,30, 3); } + [Fact] + public void cancel_scheduler_operation_prevents_execution() + { + var executionList = new List(); + + var scheduler = new KernelScheduler(); + scheduler.RegisterDeferredOperationSource( + v => Enumerable.Repeat(v * 10, v), (v) => executionList.Add(v)); + + var t1 = scheduler.Schedule(1, async (v) => + { + await Task.Delay(1000); + executionList.Add(v); + }); + var t2 = scheduler.Schedule(2, async (v) => + { + await Task.Delay(10000); + executionList.Add(v); + }); + var t3 = scheduler.Schedule(3, async (v) => + { + await Task.Delay(1000); + executionList.Add(v); + }); + + scheduler.Cancel(); + + var allTask = Task.WhenAll(t1, t2, t3); + allTask.Wait(2000); + + allTask.Exception.InnerExceptions.Should().ContainSingle(e => e is OperationCanceledException); + } + [Fact] public async Task awaiting_one_operation_does_not_wait_all() { diff --git a/src/Microsoft.DotNet.Interactive/KernelScheduler.cs b/src/Microsoft.DotNet.Interactive/KernelScheduler.cs index 1c51fa5b10..c4a1881f3b 100644 --- a/src/Microsoft.DotNet.Interactive/KernelScheduler.cs +++ b/src/Microsoft.DotNet.Interactive/KernelScheduler.cs @@ -4,12 +4,30 @@ using System; using System.Collections.Generic; using System.Reactive.Concurrency; +using System.Reactive.Disposables; using System.Threading.Tasks; namespace Microsoft.DotNet.Interactive { - public class KernelScheduler + public class DisposableStack : Stack, IDisposable { + public void Dispose() + { + while (Count>0) + { + try + { + Pop().Dispose(); + } + catch (ObjectDisposedException) + { + } + } + } + } + public class KernelScheduler : IDisposable + { + private CompositeDisposable _disposables = new(); private readonly IScheduler _executionScheduler = TaskPoolScheduler.Default; private readonly List _scheduledOperations = new(); @@ -48,17 +66,29 @@ private void ProcessScheduledOperations() if(operation is not null) { + var disposableStack = new DisposableStack(); // get all deferred operations and pump in foreach (var deferredOperationRegistration in _deferredOperationRegistrations) { foreach (var deferred in deferredOperationRegistration.GetDeferredOperations(operation.Value)) { var deferredOperation = new ScheduledOperation(deferred, deferredOperationRegistration.OnExecute); - _executionScheduler.Schedule(async () => await DoWork(deferredOperation)); + disposableStack.Push( _executionScheduler.Schedule(async () => await DoWork(deferredOperation))); } } - _executionScheduler.Schedule(async () => await DoWork(operation) ); + var disposableOperation = new CompositeDisposable + { + Disposable.Create(() => + { + operation.CompletionSource.SetException(new OperationCanceledException()); + }), + _executionScheduler.Schedule(async () => await DoWork(operation)) + }; + + disposableStack.Push(disposableOperation); + + _disposables.Add(disposableStack); } _executionScheduler.Schedule(ProcessScheduledOperations); @@ -113,5 +143,17 @@ public void RegisterDeferredOperationSource(GetDeferredOperationsDelegate getDef { _deferredOperationRegistrations.Add(new DeferredOperation(onExecuteAsync,getDeferredOperations)); } + + public void Cancel() + { + var disposables = _disposables; + _disposables = new CompositeDisposable(); + disposables.Dispose(); + } + + public void Dispose() + { + _disposables.Dispose(); + } } } \ No newline at end of file From 44713b30aa2648c534536223ef4ed32421470a13 Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Mon, 22 Feb 2021 12:03:40 +0000 Subject: [PATCH 33/44] refactoring --- .../KernelCommandSchedulerTests.cs | 40 ++++----- .../KernelScheduler.cs | 86 ++++++++++--------- 2 files changed, 63 insertions(+), 63 deletions(-) diff --git a/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs b/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs index b3546ee6de..b799df1e4c 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs @@ -87,36 +87,30 @@ public async Task deferred_work_is_executed_before_new_work() } [Fact] - public void cancel_scheduler_operation_prevents_execution() + public async Task cancel_scheduler_operation_prevents_execution() { - var executionList = new List(); - var scheduler = new KernelScheduler(); scheduler.RegisterDeferredOperationSource( - v => Enumerable.Repeat(v * 10, v), (v) => executionList.Add(v)); + v => Enumerable.Repeat(v * 10, v), async (v) => await Task.Delay(1000)); - var t1 = scheduler.Schedule(1, async (v) => - { - await Task.Delay(1000); - executionList.Add(v); - }); - var t2 = scheduler.Schedule(2, async (v) => - { - await Task.Delay(10000); - executionList.Add(v); - }); - var t3 = scheduler.Schedule(3, async (v) => - { - await Task.Delay(1000); - executionList.Add(v); - }); + var t1 = scheduler.Schedule(1, (v) => Task.Delay(10000)); + var t2 = scheduler.Schedule(2, (v) => Task.Delay(10000)); + var t3 = scheduler.Schedule(3, (v) => Task.Delay(10000)); - scheduler.Cancel(); + await Task.Delay(100); - var allTask = Task.WhenAll(t1, t2, t3); - allTask.Wait(2000); + scheduler.Cancel(); + Exception exception = null; + try + { + await Task.WhenAll(t1, t2, t3); + } + catch (Exception e) + { + exception = e; + } - allTask.Exception.InnerExceptions.Should().ContainSingle(e => e is OperationCanceledException); + exception.Should().NotBeNull(); } [Fact] diff --git a/src/Microsoft.DotNet.Interactive/KernelScheduler.cs b/src/Microsoft.DotNet.Interactive/KernelScheduler.cs index c4a1881f3b..2e82fc8b76 100644 --- a/src/Microsoft.DotNet.Interactive/KernelScheduler.cs +++ b/src/Microsoft.DotNet.Interactive/KernelScheduler.cs @@ -9,22 +9,7 @@ namespace Microsoft.DotNet.Interactive { - public class DisposableStack : Stack, IDisposable - { - public void Dispose() - { - while (Count>0) - { - try - { - Pop().Dispose(); - } - catch (ObjectDisposedException) - { - } - } - } - } + public class KernelScheduler : IDisposable { private CompositeDisposable _disposables = new(); @@ -81,7 +66,7 @@ private void ProcessScheduledOperations() { Disposable.Create(() => { - operation.CompletionSource.SetException(new OperationCanceledException()); + operation.CompletionSource.TrySetCanceled(); }), _executionScheduler.Schedule(async () => await DoWork(operation)) }; @@ -95,14 +80,51 @@ private void ProcessScheduledOperations() static async Task DoWork(ScheduledOperation operation) { - try + if (!operation.CompletionSource.Task.IsCanceled) { - await operation.OnExecuteAsync(operation.Value); - operation.CompletionSource.SetResult(default); + try + { + await operation.OnExecuteAsync(operation.Value); + operation.CompletionSource.SetResult(default); + } + catch (Exception e) + { + operation.CompletionSource.SetException(e); + } } - catch (Exception e) + } + } + + public void RegisterDeferredOperationSource(GetDeferredOperationsDelegate getDeferredOperations, OnExecuteDelegate onExecuteAsync) + { + _deferredOperationRegistrations.Add(new DeferredOperation(onExecuteAsync,getDeferredOperations)); + } + + public void Cancel() + { + var disposables = _disposables; + _disposables = new CompositeDisposable(); + disposables.Dispose(); + } + + public void Dispose() + { + _disposables.Dispose(); + } + + private class DisposableStack : Stack, IDisposable + { + public void Dispose() + { + while (Count > 0) { - operation.CompletionSource.SetException(e); + try + { + Pop().Dispose(); + } + catch (ObjectDisposedException) + { + } } } } @@ -117,7 +139,7 @@ private class ScheduledOperation public OnExecuteDelegate OnExecuteAsync { get; } public Task Task => CompletionSource.Task; - + public ScheduledOperation(T value, OnExecuteDelegate onExecuteAsync) { Value = value; @@ -125,7 +147,7 @@ public ScheduledOperation(T value, OnExecuteDelegate onExecuteAsync) OnExecuteAsync = onExecuteAsync; } - public TaskCompletionSource CompletionSource { get; } + public TaskCompletionSource CompletionSource { get; } } private class DeferredOperation @@ -139,21 +161,5 @@ public DeferredOperation(OnExecuteDelegate onExecute, GetDeferredOperationsDeleg } } - public void RegisterDeferredOperationSource(GetDeferredOperationsDelegate getDeferredOperations, OnExecuteDelegate onExecuteAsync) - { - _deferredOperationRegistrations.Add(new DeferredOperation(onExecuteAsync,getDeferredOperations)); - } - - public void Cancel() - { - var disposables = _disposables; - _disposables = new CompositeDisposable(); - disposables.Dispose(); - } - - public void Dispose() - { - _disposables.Dispose(); - } } } \ No newline at end of file From c62ff9b8d2c0a124d255107852843cd00c534cd3 Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Mon, 22 Feb 2021 12:13:37 +0000 Subject: [PATCH 34/44] refactor test --- .../KernelCommandSchedulerTests.cs | 67 +++++++++++-------- 1 file changed, 38 insertions(+), 29 deletions(-) diff --git a/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs b/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs index b799df1e4c..ea378da4c5 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs @@ -7,8 +7,6 @@ using System.Threading.Tasks; using FluentAssertions; - -using Microsoft.DotNet.Interactive.Commands; using Microsoft.DotNet.Interactive.Tests.Utility; using Pocket; @@ -56,12 +54,17 @@ public async Task scheduled_work_is_completed_in_order() var executionList = new List(); - await scheduler.Schedule(1, (v) => executionList.Add(v)); - await scheduler.Schedule(2, (v) => executionList.Add(v)); - await scheduler.Schedule(3, (v) => executionList.Add(v)); + await scheduler.Schedule(1, PerformWork); + await scheduler.Schedule(2, PerformWork); + await scheduler.Schedule(3, PerformWork); executionList.Should().BeEquivalentSequenceTo(1, 2, 3); + + void PerformWork(int v) + { + executionList.Add(v); + } } [Fact] @@ -77,25 +80,31 @@ public async Task deferred_work_is_executed_before_new_work() var scheduler = new KernelScheduler(); scheduler.RegisterDeferredOperationSource( - v => Enumerable.Repeat(v * 10, v), (v) => executionList.Add(v)); + v => Enumerable.Repeat(v * 10, v), PerformWork); - await scheduler.Schedule(1, (v) => executionList.Add(v)); - await scheduler.Schedule(2, (v) => executionList.Add(v)); - await scheduler.Schedule(3, (v) => executionList.Add(v)); + await scheduler.Schedule(1, PerformWork); + await scheduler.Schedule(2, PerformWork); + await scheduler.Schedule(3, PerformWork); executionList.Should().BeEquivalentSequenceTo(10,1,20,20, 2,30,30,30, 3); + + void PerformWork(int v) + { + executionList.Add(v); + } } [Fact] public async Task cancel_scheduler_operation_prevents_execution() { + var executionList = new List(); var scheduler = new KernelScheduler(); scheduler.RegisterDeferredOperationSource( - v => Enumerable.Repeat(v * 10, v), async (v) => await Task.Delay(1000)); + v => Enumerable.Repeat(v * 10, v), onExecuteAsync: PerformWorkAsync); - var t1 = scheduler.Schedule(1, (v) => Task.Delay(10000)); - var t2 = scheduler.Schedule(2, (v) => Task.Delay(10000)); - var t3 = scheduler.Schedule(3, (v) => Task.Delay(10000)); + var t1 = scheduler.Schedule(1, PerformWorkAsync); + var t2 = scheduler.Schedule(2, PerformWorkAsync); + var t3 = scheduler.Schedule(3, PerformWorkAsync); await Task.Delay(100); @@ -111,6 +120,13 @@ public async Task cancel_scheduler_operation_prevents_execution() } exception.Should().NotBeNull(); + executionList.Should().BeEmpty(); + + async Task PerformWorkAsync(int v) + { + await Task.Delay(1000); + executionList.Add(v); + } } [Fact] @@ -120,25 +136,18 @@ public async Task awaiting_one_operation_does_not_wait_all() var scheduler = new KernelScheduler(); + await scheduler.Schedule(1, PerformWorkAsync); + await scheduler.Schedule(2, PerformWorkAsync); - await scheduler.Schedule(1, async (v) => - { - await Task.Delay(10); - executionList.Add(v); - }); - await scheduler.Schedule(2, async (v) => - { - await Task.Delay(10); - executionList.Add(v); - }); - - _ = scheduler.Schedule(3, async (v) => - { - await Task.Delay(200); - executionList.Add(v); - }); + _ = scheduler.Schedule(3, PerformWorkAsync); executionList.Should().BeEquivalentSequenceTo( 1, 2); + + async Task PerformWorkAsync(int v) + { + await Task.Delay(200); + executionList.Add(v); + } } [Fact] From 87de3eaef918e9eb637da87753e4c35ff5adc6bc Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Mon, 22 Feb 2021 12:25:53 +0000 Subject: [PATCH 35/44] disable warning for fire and forget --- src/Microsoft.DotNet.Interactive.Tests/CancelCommandTests.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Microsoft.DotNet.Interactive.Tests/CancelCommandTests.cs b/src/Microsoft.DotNet.Interactive.Tests/CancelCommandTests.cs index 86f0fa120b..da1851a76c 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/CancelCommandTests.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/CancelCommandTests.cs @@ -14,6 +14,8 @@ using Xunit; using Xunit.Abstractions; +#pragma warning disable 4014 + namespace Microsoft.DotNet.Interactive.Tests { From 99b10e1ad967115d22ac5d0d5aa7338bc74cabca Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Mon, 22 Feb 2021 12:26:45 +0000 Subject: [PATCH 36/44] removed pragma --- .../CancelCommandTests.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.DotNet.Interactive.Tests/CancelCommandTests.cs b/src/Microsoft.DotNet.Interactive.Tests/CancelCommandTests.cs index da1851a76c..dbb3968cb2 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/CancelCommandTests.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/CancelCommandTests.cs @@ -14,8 +14,6 @@ using Xunit; using Xunit.Abstractions; -#pragma warning disable 4014 - namespace Microsoft.DotNet.Interactive.Tests { @@ -148,7 +146,7 @@ public async Task commands_issued_after_cancel_command_are_executed(Language lan var commandToRun = new SubmitCode("1"); - kernel.SendAsync(commandToCancel); + var _ = kernel.SendAsync(commandToCancel); await Task.Delay(4000); await kernel.SendAsync(cancelCommand); await kernel.SendAsync(commandToRun); @@ -186,7 +184,7 @@ public async Task user_code_can_react_to_cancel_command_using_cancellation_token var cancelCommand = new Cancel(); var submitCodeCommand = new SubmitCode(code); - kernel.SendAsync(submitCodeCommand); + var _ = kernel.SendAsync(submitCodeCommand); await Task.Delay(4000); await kernel.SendAsync(cancelCommand); From 8edf8555e37581ef4b2317f928a44363b08c4987 Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Mon, 22 Feb 2021 17:08:43 +0000 Subject: [PATCH 37/44] disposable scheduler disposed during tests --- .../KernelCommandSchedulerTests.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs b/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs index ea378da4c5..a048d3bd01 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs @@ -50,7 +50,7 @@ private void DisposeAfterTest(Action action) [Fact] public async Task scheduled_work_is_completed_in_order() { - var scheduler = new KernelScheduler(); + using var scheduler = new KernelScheduler(); var executionList = new List(); @@ -78,7 +78,7 @@ public async Task deferred_work_is_executed_before_new_work() { var executionList = new List(); - var scheduler = new KernelScheduler(); + using var scheduler = new KernelScheduler(); scheduler.RegisterDeferredOperationSource( v => Enumerable.Repeat(v * 10, v), PerformWork); @@ -98,7 +98,7 @@ void PerformWork(int v) public async Task cancel_scheduler_operation_prevents_execution() { var executionList = new List(); - var scheduler = new KernelScheduler(); + using var scheduler = new KernelScheduler(); scheduler.RegisterDeferredOperationSource( v => Enumerable.Repeat(v * 10, v), onExecuteAsync: PerformWorkAsync); @@ -134,7 +134,7 @@ public async Task awaiting_one_operation_does_not_wait_all() { var executionList = new List(); - var scheduler = new KernelScheduler(); + using var scheduler = new KernelScheduler(); await scheduler.Schedule(1, PerformWorkAsync); await scheduler.Schedule(2, PerformWorkAsync); From 885ab81f9bd71768bbb11ff3488822bbb845a39f Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Mon, 22 Feb 2021 17:47:49 +0000 Subject: [PATCH 38/44] test parallel execution --- .../KernelCommandSchedulerTests.cs | 35 ++++++++++++++++--- 1 file changed, 30 insertions(+), 5 deletions(-) diff --git a/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs b/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs index a048d3bd01..7f407ad5fd 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Threading; using System.Threading.Tasks; using FluentAssertions; @@ -68,9 +69,32 @@ void PerformWork(int v) } [Fact] - public void scheduled_work_does_not_execute_in_parallel() + public async Task scheduled_work_does_not_execute_in_parallel() { - throw new NotImplementedException(); + using var scheduler = new KernelScheduler(); + var concurrencyCounter = 0; + var maxObservedParallelism = 0; + var tasks = new Task[3]; + + for (var i = 0; i < 3; i++) + { + var task = scheduler.Schedule(i, async _ => + { + Interlocked.Increment(ref concurrencyCounter); + + await Task.Delay(100); + maxObservedParallelism = Math.Max(concurrencyCounter, maxObservedParallelism); + + Interlocked.Decrement(ref concurrencyCounter); + } ); + tasks[i] = task; + } + + await Task.WhenAll(tasks); + + maxObservedParallelism.Should().Be(1); + + } [Fact] @@ -82,9 +106,10 @@ public async Task deferred_work_is_executed_before_new_work() scheduler.RegisterDeferredOperationSource( v => Enumerable.Repeat(v * 10, v), PerformWork); - await scheduler.Schedule(1, PerformWork); - await scheduler.Schedule(2, PerformWork); - await scheduler.Schedule(3, PerformWork); + for (var i = 1; i <= 3; i++) + { + await scheduler.Schedule(i, PerformWork); + } executionList.Should().BeEquivalentSequenceTo(10,1,20,20, 2,30,30,30, 3); From a8c5b86a0e44691df96ad3b3cb3d3bdff283106b Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Tue, 23 Feb 2021 08:08:48 +0000 Subject: [PATCH 39/44] test for parallelism --- .../KernelCommandSchedulerTests.cs | 90 +++---- .../KernelScheduler.cs | 227 +++++++++++++----- .../KernelSchedulerExtensions.cs | 4 +- 3 files changed, 215 insertions(+), 106 deletions(-) diff --git a/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs b/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs index 7f407ad5fd..96ab141c49 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs @@ -93,8 +93,6 @@ public async Task scheduled_work_does_not_execute_in_parallel() await Task.WhenAll(tasks); maxObservedParallelism.Should().Be(1); - - } [Fact] @@ -102,65 +100,63 @@ public async Task deferred_work_is_executed_before_new_work() { var executionList = new List(); + void PerformWork(int v) + { + executionList.Add(v); + } + using var scheduler = new KernelScheduler(); scheduler.RegisterDeferredOperationSource( - v => Enumerable.Repeat(v * 10, v), PerformWork); + (v,_) => Enumerable.Repeat(v * 10, v), PerformWork); for (var i = 1; i <= 3; i++) { await scheduler.Schedule(i, PerformWork); } - executionList.Should().BeEquivalentSequenceTo(10,1,20,20, 2,30,30,30, 3); - - void PerformWork(int v) - { - executionList.Add(v); - } + executionList.Should().BeEquivalentSequenceTo(10, 1, 20, 20, 2, 30, 30, 30, 3); } [Fact] - public async Task cancel_scheduler_operation_prevents_execution() + public void cancel_scheduler_work_prevents_any_scheduled_work_from_executing() { var executionList = new List(); using var scheduler = new KernelScheduler(); - scheduler.RegisterDeferredOperationSource( - v => Enumerable.Repeat(v * 10, v), onExecuteAsync: PerformWorkAsync); - - var t1 = scheduler.Schedule(1, PerformWorkAsync); - var t2 = scheduler.Schedule(2, PerformWorkAsync); - var t3 = scheduler.Schedule(3, PerformWorkAsync); - - await Task.Delay(100); - - scheduler.Cancel(); - Exception exception = null; - try + var barrier = new Barrier(2); + void PerformWork(int v) { - await Task.WhenAll(t1, t2, t3); + barrier.SignalAndWait(5000); + executionList.Add(v); } - catch (Exception e) + + var scheduledWork = new List { - exception = e; - } + scheduler.Schedule(1, PerformWork), + scheduler.Schedule(2, executionList.Add), + scheduler.Schedule(3, executionList.Add) + }; - exception.Should().NotBeNull(); - executionList.Should().BeEmpty(); + barrier.SignalAndWait(); + scheduler.Cancel(); + Task.WhenAll(scheduledWork); - async Task PerformWorkAsync(int v) - { - await Task.Delay(1000); - executionList.Add(v); - } + + executionList.Should().BeEquivalentTo(1); } [Fact] - public async Task awaiting_one_operation_does_not_wait_all() + public async Task awaiting_for_work_to_complete_does_not_wait_for_subsequent_work() { var executionList = new List(); using var scheduler = new KernelScheduler(); + async Task PerformWorkAsync(int v) + { + await Task.Delay(200); + executionList.Add(v); + } + await scheduler.Schedule(1, PerformWorkAsync); await scheduler.Schedule(2, PerformWorkAsync); @@ -168,17 +164,29 @@ public async Task awaiting_one_operation_does_not_wait_all() executionList.Should().BeEquivalentSequenceTo( 1, 2); - async Task PerformWorkAsync(int v) - { - await Task.Delay(200); - executionList.Add(v); - } + } [Fact] - public void new_work_is_executed_after_all_require() + public async Task deferred_work_is_done_based_on_the_scope_of_scheduled_work() { - throw new NotImplementedException(); + var executionList = new List(); + + void PerformWork(int v) + { + executionList.Add(v); + } + + using var scheduler = new KernelScheduler(); + scheduler.RegisterDeferredOperationSource( + (v, scope) => scope == "scope2" ? Enumerable.Repeat(v * 10, v) : Enumerable.Empty(), PerformWork); + + for (var i = 1; i <= 3; i++) + { + await scheduler.Schedule(i, PerformWork, $"scope{i}"); + } + + executionList.Should().BeEquivalentSequenceTo( 1, 20, 20, 2,3); } //[Fact] diff --git a/src/Microsoft.DotNet.Interactive/KernelScheduler.cs b/src/Microsoft.DotNet.Interactive/KernelScheduler.cs index 2e82fc8b76..31aea895aa 100644 --- a/src/Microsoft.DotNet.Interactive/KernelScheduler.cs +++ b/src/Microsoft.DotNet.Interactive/KernelScheduler.cs @@ -2,94 +2,122 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; +using System.Collections.Concurrent; using System.Collections.Generic; -using System.Reactive.Concurrency; -using System.Reactive.Disposables; +using System.Threading; using System.Threading.Tasks; +using Pocket; + namespace Microsoft.DotNet.Interactive { - - public class KernelScheduler : IDisposable + + public class KernelScheduler : IDisposable { - private CompositeDisposable _disposables = new(); - private readonly IScheduler _executionScheduler = TaskPoolScheduler.Default; + private CancellationTokenSource _cancellationTokenSource = new(); + private static readonly Logger Logger = new Logger("Scheduler"); + + private List _scheduledOperations = new(); + private List _deferredOperationRegistrations = new(); - private readonly List _scheduledOperations = new(); - private readonly List _deferredOperationRegistrations = new(); + private readonly object _operationsLock = new(); - public Task Schedule(T value, OnExecuteDelegate onExecuteAsync) + public Task Schedule(T value, OnExecuteDelegate onExecuteAsync, string scope = "default") { - var operation = new ScheduledOperation(value, onExecuteAsync); + var operation = new ScheduledOperation(value, onExecuteAsync, scope); - lock (_scheduledOperations) + lock (_operationsLock) { + _cancellationTokenSource.Token.Register(() => + { + if(!operation.CompletionSource.Task.IsCompleted) + { + operation.CompletionSource.SetCanceled(); + } + }); + _scheduledOperations.Add(operation); + if (_scheduledOperations.Count == 1) + { + var previousSynchronizationContext = SynchronizationContext.Current; + var synchronizationContext = new ClockwiseSynchronizationContext(); + + SynchronizationContext.SetSynchronizationContext(synchronizationContext); + Task.Run(async () => + { + while (_scheduledOperations.Count > 0) + { + await ProcessScheduledOperations(_cancellationTokenSource.Token); + } + }).ContinueWith(_ => + { + SynchronizationContext.SetSynchronizationContext(previousSynchronizationContext); + }); + } } - _executionScheduler.Schedule(ProcessScheduledOperations); - - return operation.Task; } - private void ProcessScheduledOperations() + private async Task ProcessScheduledOperations(CancellationToken cancellationToken) { + using var _ = Logger.OnEnterAndExit(); ScheduledOperation operation; - lock (_scheduledOperations) - { if(_scheduledOperations.Count > 0) + lock (_operationsLock) + { + if (_scheduledOperations.Count > 0) { operation = _scheduledOperations[0]; _scheduledOperations.RemoveAt(0); } else { + return; } } - if(operation is not null) + if (operation is not null) { - var disposableStack = new DisposableStack(); // get all deferred operations and pump in foreach (var deferredOperationRegistration in _deferredOperationRegistrations) { - foreach (var deferred in deferredOperationRegistration.GetDeferredOperations(operation.Value)) + foreach (var deferred in deferredOperationRegistration.GetDeferredOperations(operation.Value, operation.Scope)) { - var deferredOperation = new ScheduledOperation(deferred, deferredOperationRegistration.OnExecute); - disposableStack.Push( _executionScheduler.Schedule(async () => await DoWork(deferredOperation))); + var deferredOperation = new ScheduledOperation(deferred, deferredOperationRegistration.OnExecute, operation.Scope); + + cancellationToken.Register(() => + { + if (!deferredOperation.CompletionSource.Task.IsCompleted) + { + deferredOperation.CompletionSource.SetCanceled(); + } + }); + + await DoWork(deferredOperation); } } - var disposableOperation = new CompositeDisposable - { - Disposable.Create(() => - { - operation.CompletionSource.TrySetCanceled(); - }), - _executionScheduler.Schedule(async () => await DoWork(operation)) - }; - - disposableStack.Push(disposableOperation); - - _disposables.Add(disposableStack); + + await DoWork(operation); } - _executionScheduler.Schedule(ProcessScheduledOperations); - static async Task DoWork(ScheduledOperation operation) + + async Task DoWork(ScheduledOperation scheduleOperation) { - if (!operation.CompletionSource.Task.IsCanceled) + using var _ = Logger.OnEnterAndExit("DoWork"); + if (!scheduleOperation.CompletionSource.Task.IsCanceled) { try { - await operation.OnExecuteAsync(operation.Value); - operation.CompletionSource.SetResult(default); + await scheduleOperation.OnExecuteAsync(scheduleOperation.Value); + scheduleOperation.CompletionSource.SetResult(default); } catch (Exception e) { - operation.CompletionSource.SetException(e); + scheduleOperation.CompletionSource.SetException(e); } } } @@ -97,54 +125,53 @@ static async Task DoWork(ScheduledOperation operation) public void RegisterDeferredOperationSource(GetDeferredOperationsDelegate getDeferredOperations, OnExecuteDelegate onExecuteAsync) { - _deferredOperationRegistrations.Add(new DeferredOperation(onExecuteAsync,getDeferredOperations)); + _deferredOperationRegistrations.Add(new DeferredOperation(onExecuteAsync, getDeferredOperations)); } public void Cancel() { - var disposables = _disposables; - _disposables = new CompositeDisposable(); - disposables.Dispose(); - } + lock (_operationsLock) + { - public void Dispose() - { - _disposables.Dispose(); - } - private class DisposableStack : Stack, IDisposable - { - public void Dispose() - { - while (Count > 0) + if (SynchronizationContext.Current is ClockwiseSynchronizationContext synchronizationContext) { - try - { - Pop().Dispose(); - } - catch (ObjectDisposedException) - { - } + synchronizationContext.Cancel(); } + + _scheduledOperations = new List(); + _deferredOperationRegistrations = new List(); + + _cancellationTokenSource.Cancel(); + _cancellationTokenSource = new CancellationTokenSource(); } } + public void Dispose() + { + Cancel(); + } + + + public delegate Task OnExecuteDelegate(T value); - public delegate IEnumerable GetDeferredOperationsDelegate(T operationToExecute); + public delegate IEnumerable GetDeferredOperationsDelegate(T operationToExecute, string queueName); private class ScheduledOperation { public T Value { get; } public OnExecuteDelegate OnExecuteAsync { get; } + public string Scope { get; } public Task Task => CompletionSource.Task; - public ScheduledOperation(T value, OnExecuteDelegate onExecuteAsync) + public ScheduledOperation(T value, OnExecuteDelegate onExecuteAsync, string scope) { Value = value; CompletionSource = new TaskCompletionSource(); OnExecuteAsync = onExecuteAsync; + Scope = scope; } public TaskCompletionSource CompletionSource { get; } @@ -162,4 +189,78 @@ public DeferredOperation(OnExecuteDelegate onExecute, GetDeferredOperationsDeleg } } + + internal sealed class ClockwiseSynchronizationContext : SynchronizationContext, IDisposable + { + private static readonly Logger Logger = new Logger("SynchronizationContext"); + + private readonly BlockingCollection _queue = new(); + + public ClockwiseSynchronizationContext() + { + var thread = new Thread(Run); + + thread.Start(); + } + + public override void Post(SendOrPostCallback callback, object state) + { + if (callback == null) + { + throw new ArgumentNullException(nameof(callback)); + } + + var workItem = new WorkItem(callback, state); + + try + { + _queue.Add(workItem); + } + catch (InvalidOperationException) + { + throw new ObjectDisposedException($"The {nameof(ClockwiseSynchronizationContext)} has been disposed."); + } + } + + public override void Send(SendOrPostCallback callback, object state) => + throw new NotSupportedException($"Synchronous Send is not supported by {nameof(ClockwiseSynchronizationContext)}."); + + public void Cancel() + { + Cancelled = true; + } + + public bool Cancelled { get; private set; } + + private void Run() + { + SetSynchronizationContext(this); + + foreach (var workItem in _queue.GetConsumingEnumerable()) + { + if (!Cancelled) + { + workItem.Run(); + } + + } + } + + public void Dispose() => _queue.CompleteAdding(); + + private struct WorkItem + { + public WorkItem(SendOrPostCallback callback, object state) + { + Callback = callback; + State = state; + } + + private readonly SendOrPostCallback Callback; + + private readonly object State; + + public void Run() => Callback(State); + } + } } \ No newline at end of file diff --git a/src/Microsoft.DotNet.Interactive/KernelSchedulerExtensions.cs b/src/Microsoft.DotNet.Interactive/KernelSchedulerExtensions.cs index e7bb6af75b..9c778f8b10 100644 --- a/src/Microsoft.DotNet.Interactive/KernelSchedulerExtensions.cs +++ b/src/Microsoft.DotNet.Interactive/KernelSchedulerExtensions.cs @@ -8,13 +8,13 @@ namespace Microsoft.DotNet.Interactive { public static class KernelSchedulerExtensions { - public static Task Schedule(this KernelScheduler kernelScheduler, T value, Action onExecute) + public static Task Schedule(this KernelScheduler kernelScheduler, T value, Action onExecute, string scope = "default") { return kernelScheduler.Schedule(value, v => { onExecute(v); return Task.CompletedTask; - }); + },scope); } public static void RegisterDeferredOperationSource(this KernelScheduler kernelScheduler, KernelScheduler.GetDeferredOperationsDelegate getDeferredOperations, Action onExecute) From 2c470e98639abb5696852727b55dc77d815f59eb Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Tue, 23 Feb 2021 13:55:36 +0000 Subject: [PATCH 40/44] test exception handling for code in scheduled work remove blank lines --- .../KernelCommandSchedulerTests.cs | 306 ++++++++---------- .../KernelScheduler.cs | 44 +-- 2 files changed, 162 insertions(+), 188 deletions(-) diff --git a/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs b/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs index 96ab141c49..aa2437e22b 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs @@ -8,6 +8,7 @@ using System.Threading.Tasks; using FluentAssertions; + using Microsoft.DotNet.Interactive.Tests.Utility; using Pocket; @@ -75,10 +76,10 @@ public async Task scheduled_work_does_not_execute_in_parallel() var concurrencyCounter = 0; var maxObservedParallelism = 0; var tasks = new Task[3]; - + for (var i = 0; i < 3; i++) { - var task = scheduler.Schedule(i, async _ => + var task = scheduler.Schedule(i, async _ => { Interlocked.Increment(ref concurrencyCounter); @@ -86,10 +87,10 @@ public async Task scheduled_work_does_not_execute_in_parallel() maxObservedParallelism = Math.Max(concurrencyCounter, maxObservedParallelism); Interlocked.Decrement(ref concurrencyCounter); - } ); + }); tasks[i] = task; } - + await Task.WhenAll(tasks); maxObservedParallelism.Should().Be(1); @@ -107,7 +108,7 @@ void PerformWork(int v) using var scheduler = new KernelScheduler(); scheduler.RegisterDeferredOperationSource( - (v,_) => Enumerable.Repeat(v * 10, v), PerformWork); + (v, _) => Enumerable.Repeat(v * 10, v), PerformWork); for (var i = 1; i <= 3; i++) { @@ -144,6 +145,132 @@ void PerformWork(int v) executionList.Should().BeEquivalentTo(1); } + [Fact] + public async Task cancelled_work_prevents_any_scheduled_work_from_executing() + { + var executionList = new List(); + using var scheduler = new KernelScheduler(); + var barrier = new Barrier(2); + + async Task PerformWork(int v) + { + barrier.SignalAndWait(); + await Task.Delay(3000); + executionList.Add(v); + } + + var scheduledWork = new List + { + scheduler.Schedule(1, PerformWork), + scheduler.Schedule(2, executionList.Add), + scheduler.Schedule(3, executionList.Add) + }; + + barrier.SignalAndWait(); + scheduler.Cancel(); + try + { + await Task.WhenAll(scheduledWork); + } + catch (TaskCanceledException) + { + + } + + executionList.Should().BeEmpty(); + } + + [Fact] + public void cancelling_work_throws_exception() + { + var executionList = new List(); + using var scheduler = new KernelScheduler(); + var barrier = new Barrier(2); + + async Task PerformWork(int v) + { + barrier.SignalAndWait(); + await Task.Delay(3000); + executionList.Add(v); + } + + var scheduledWork = new List + { + scheduler.Schedule(1, PerformWork), + scheduler.Schedule(2, executionList.Add), + scheduler.Schedule(3, executionList.Add) + }; + + barrier.SignalAndWait(); + scheduler.Cancel(); + var operation = new Action( () => Task.WhenAll(scheduledWork).Wait(5000)); + + operation.Should().Throw(); + } + + [Fact] + public async Task exception_in_scheduled_work_halts_execution() + { + var executionList = new List(); + using var scheduler = new KernelScheduler(); + var barrier = new Barrier(2); + + void PerformWork(int v) + { + barrier.SignalAndWait(); + throw new InvalidOperationException("test exception"); + } + + var scheduledWork = new List + { + scheduler.Schedule(1, PerformWork), + scheduler.Schedule(2, executionList.Add), + scheduler.Schedule(3, executionList.Add) + }; + + barrier.SignalAndWait(); + try + { + await Task.WhenAll(scheduledWork); + } + catch(InvalidOperationException) + { + + } + + executionList.Should().BeEmpty(); + } + + [Fact] + public void exception_in_scheduled_work_is_propagated() + { + var executionList = new List(); + using var scheduler = new KernelScheduler(); + var barrier = new Barrier(2); + + void PerformWork(int v) + { + barrier.SignalAndWait(); + throw new InvalidOperationException("test exception"); + } + + var scheduledWork = new List + { + scheduler.Schedule(1, PerformWork), + scheduler.Schedule(2, executionList.Add), + scheduler.Schedule(3, executionList.Add) + }; + + barrier.SignalAndWait(); + var operation = new Action(() => Task.WhenAll(scheduledWork).Wait(5000)); + + operation.Should().Throw() + .Which + .Message + .Should() + .Be("test exception"); + } + [Fact] public async Task awaiting_for_work_to_complete_does_not_wait_for_subsequent_work() { @@ -162,9 +289,9 @@ async Task PerformWorkAsync(int v) _ = scheduler.Schedule(3, PerformWorkAsync); - executionList.Should().BeEquivalentSequenceTo( 1, 2); + executionList.Should().BeEquivalentSequenceTo(1, 2); + - } [Fact] @@ -186,168 +313,7 @@ void PerformWork(int v) await scheduler.Schedule(i, PerformWork, $"scope{i}"); } - executionList.Should().BeEquivalentSequenceTo( 1, 20, 20, 2,3); + executionList.Should().BeEquivalentSequenceTo(1, 20, 20, 2, 3); } - - //[Fact] - //public async Task command_execute_on_kernel_specified_at_scheduling_time() - //{ - // var commandsHandledOnKernel1 = new List(); - // var commandsHandledOnKernel2 = new List(); - - // var scheduler = new KernelCommandScheduler(); - - // var kernel1 = new FakeKernel("kernel1") - // { - // Handle = (command, _) => - // { - // commandsHandledOnKernel1.Add(command); - // return Task.CompletedTask; - // } - // }; - // var kernel2 = new FakeKernel("kernel2") - // { - // Handle = (command, _) => - // { - // commandsHandledOnKernel2.Add(command); - // return Task.CompletedTask; - // } - // }; - - // var command1 = new SubmitCode("for kernel 1", kernel1.Name); - // var command2 = new SubmitCode("for kernel 2", kernel2.Name); - - // await scheduler.Schedule(command1); - // await scheduler.Schedule(command2); - - // commandsHandledOnKernel1.Should().ContainSingle().Which.Should().Be(command1); - // commandsHandledOnKernel2.Should().ContainSingle().Which.Should().Be(command2); - //} - - //[Fact] - //public async Task scheduling_a_command_will_defer_deferred_commands_scheduled_on_same_kernel() - //{ - // var commandsHandledOnKernel1 = new List(); - - // var scheduler = new KernelCommandScheduler(); - - // var kernel1 = new FakeKernel("kernel1") - // { - // Handle = (command, _) => - // { - // commandsHandledOnKernel1.Add(command); - // return Task.CompletedTask; - // } - // }; - // var kernel2 = new FakeKernel("kernel2") - // { - // Handle = (_, _) => Task.CompletedTask - // }; - - // var deferredCommand1 = new SubmitCode("deferred for kernel 1", kernel1.Name); - // var deferredCommand2 = new SubmitCode("deferred for kernel 2", kernel2.Name); - // var deferredCommand3 = new SubmitCode("deferred for kernel 1", kernel1.Name); - // var command1 = new SubmitCode("for kernel 1", kernel1.Name); - - // scheduler.DeferCommand(deferredCommand1); - // scheduler.DeferCommand(deferredCommand2); - // scheduler.DeferCommand(deferredCommand3); - // await scheduler.Schedule(command1); - - // commandsHandledOnKernel1.Should().NotContain(deferredCommand2); - // commandsHandledOnKernel1.Should().BeEquivalentSequenceTo(deferredCommand1, deferredCommand3, command1); - //} - - //[Fact] - //public async Task deferred_command_not_executed_are_still_in_deferred_queue() - //{ - // var commandsHandledOnKernel1 = new List(); - // var commandsHandledOnKernel2 = new List(); - - // var scheduler = new KernelCommandScheduler(); - - // var kernel1 = new FakeKernel("kernel1") - // { - // Handle = (command, _) => - // { - // commandsHandledOnKernel1.Add(command); - // return Task.CompletedTask; - // } - // }; - // var kernel2 = new FakeKernel("kernel2") - // { - // Handle = (command, _) => - // { - // commandsHandledOnKernel2.Add(command); - // return Task.CompletedTask; - // } - // }; - - // var deferredCommand1 = new SubmitCode("deferred for kernel 1"); - // var deferredCommand2 = new SubmitCode("deferred for kernel 2"); - // var deferredCommand3 = new SubmitCode("deferred for kernel 1"); - // var command1 = new SubmitCode("for kernel 1"); - // var command2 = new SubmitCode("for kernel 2"); - - // scheduler.DeferCommand(deferredCommand1, kernel1); - // scheduler.DeferCommand(deferredCommand2, kernel2); - // scheduler.DeferCommand(deferredCommand3, kernel1); - // await scheduler.Schedule(command1, kernel1); - - // commandsHandledOnKernel2.Should().BeEmpty(); - // commandsHandledOnKernel1.Should().NotContain(deferredCommand2); - // commandsHandledOnKernel1.Should().BeEquivalentSequenceTo(deferredCommand1, deferredCommand3, command1); - // await scheduler.Schedule(command2, kernel2); - // commandsHandledOnKernel2.Should().BeEquivalentSequenceTo(deferredCommand2, command2); - //} - - //[Fact] - //public async Task deferred_command_on_parent_kernel_are_executed_when_scheduling_command_on_child_kernel() - //{ - // var commandHandledList = new List<(KernelCommand command, Kernel kernel)>(); - - // var scheduler = new KernelCommandScheduler(); - - // var childKernel = new FakeKernel("kernel1") - // { - // Handle = (command, context) => command.InvokeAsync(context) - // }; - // var parentKernel = new CompositeKernel - // { - // childKernel - // }; - - // parentKernel.DefaultKernelName = childKernel.Name; - - // var deferredCommand1 = new TestCommand((command, context) => - // { - // commandHandledList.Add((command, context.HandlingKernel)); - // return Task.CompletedTask; - // }, childKernel.Name); - // var deferredCommand2 = new TestCommand((command, context) => - // { - // commandHandledList.Add((command, context.HandlingKernel)); - // return Task.CompletedTask; - // }, parentKernel.Name); - // var deferredCommand3 = new TestCommand((command, context) => - // { - // commandHandledList.Add((command, context.HandlingKernel)); - // return Task.CompletedTask; - // }, childKernel.Name); - // var command1 = new TestCommand((command, context) => - // { - // commandHandledList.Add((command, context.HandlingKernel)); - // return Task.CompletedTask; - // }, childKernel.Name); - - // scheduler.DeferCommand(deferredCommand1, childKernel); - // scheduler.DeferCommand(deferredCommand2, parentKernel); - // scheduler.DeferCommand(deferredCommand3, childKernel); - // await scheduler.Schedule(command1, childKernel); - - // commandHandledList.Select(e => e.command).Should().BeEquivalentSequenceTo(deferredCommand1, deferredCommand2, deferredCommand3, command1); - - // commandHandledList.Select(e => e.kernel).Should().BeEquivalentSequenceTo(childKernel, parentKernel, childKernel, childKernel); - //} } - } \ No newline at end of file +} \ No newline at end of file diff --git a/src/Microsoft.DotNet.Interactive/KernelScheduler.cs b/src/Microsoft.DotNet.Interactive/KernelScheduler.cs index 31aea895aa..a4fbcc740c 100644 --- a/src/Microsoft.DotNet.Interactive/KernelScheduler.cs +++ b/src/Microsoft.DotNet.Interactive/KernelScheduler.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Concurrent; using System.Collections.Generic; +using System.Linq.Expressions; using System.Threading; using System.Threading.Tasks; @@ -73,37 +74,43 @@ private async Task ProcessScheduledOperations(CancellationToken cancellationToke } else { - return; } } - if (operation is not null) + try { - // get all deferred operations and pump in - foreach (var deferredOperationRegistration in _deferredOperationRegistrations) + if (operation is not null) { - foreach (var deferred in deferredOperationRegistration.GetDeferredOperations(operation.Value, operation.Scope)) + // get all deferred operations and pump in + foreach (var deferredOperationRegistration in _deferredOperationRegistrations) { - var deferredOperation = new ScheduledOperation(deferred, deferredOperationRegistration.OnExecute, operation.Scope); - - cancellationToken.Register(() => + foreach (var deferred in deferredOperationRegistration.GetDeferredOperations(operation.Value, + operation.Scope)) { - if (!deferredOperation.CompletionSource.Task.IsCompleted) + var deferredOperation = new ScheduledOperation(deferred, + deferredOperationRegistration.OnExecute, operation.Scope); + + cancellationToken.Register(() => { - deferredOperation.CompletionSource.SetCanceled(); - } - }); + if (!deferredOperation.CompletionSource.Task.IsCompleted) + { + deferredOperation.CompletionSource.SetCanceled(); + } + }); - await DoWork(deferredOperation); + await DoWork(deferredOperation); + } } - } - - await DoWork(operation); + await DoWork(operation); + } + } + catch + { + Cancel(); + throw; } - - async Task DoWork(ScheduledOperation scheduleOperation) { @@ -118,6 +125,7 @@ async Task DoWork(ScheduledOperation scheduleOperation) catch (Exception e) { scheduleOperation.CompletionSource.SetException(e); + throw; } } } From 314e74022c5a1e0c6074128775600f2c218817f8 Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Tue, 23 Feb 2021 14:24:42 +0000 Subject: [PATCH 41/44] remove namespace --- src/Microsoft.DotNet.Interactive/KernelScheduler.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.DotNet.Interactive/KernelScheduler.cs b/src/Microsoft.DotNet.Interactive/KernelScheduler.cs index a4fbcc740c..a61901107e 100644 --- a/src/Microsoft.DotNet.Interactive/KernelScheduler.cs +++ b/src/Microsoft.DotNet.Interactive/KernelScheduler.cs @@ -4,7 +4,6 @@ using System; using System.Collections.Concurrent; using System.Collections.Generic; -using System.Linq.Expressions; using System.Threading; using System.Threading.Tasks; @@ -32,7 +31,7 @@ public Task Schedule(T value, OnExecuteDelegate onExecuteAsync, string scope { _cancellationTokenSource.Token.Register(() => { - if(!operation.CompletionSource.Task.IsCompleted) + if (!operation.CompletionSource.Task.IsCompleted) { operation.CompletionSource.SetCanceled(); } @@ -43,7 +42,7 @@ public Task Schedule(T value, OnExecuteDelegate onExecuteAsync, string scope { var previousSynchronizationContext = SynchronizationContext.Current; var synchronizationContext = new ClockwiseSynchronizationContext(); - + SynchronizationContext.SetSynchronizationContext(synchronizationContext); Task.Run(async () => { @@ -150,7 +149,7 @@ public void Cancel() _scheduledOperations = new List(); _deferredOperationRegistrations = new List(); - _cancellationTokenSource.Cancel(); + _cancellationTokenSource.Cancel(); _cancellationTokenSource = new CancellationTokenSource(); } } From d7c3353ed2839b1c17487d99985c99444781f8c0 Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Tue, 23 Feb 2021 23:08:47 +0000 Subject: [PATCH 42/44] change api surface --- .../KernelCommandSchedulerTests.cs | 88 ++++++++++++++----- .../KernelScheduler.cs | 8 +- .../KernelSchedulerExtensions.cs | 55 ++++++++++-- 3 files changed, 117 insertions(+), 34 deletions(-) diff --git a/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs b/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs index aa2437e22b..a324ccc044 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs @@ -63,9 +63,10 @@ public async Task scheduled_work_is_completed_in_order() executionList.Should().BeEquivalentSequenceTo(1, 2, 3); - void PerformWork(int v) + Task PerformWork(int v) { executionList.Add(v); + return Task.FromResult(v); } } @@ -79,7 +80,7 @@ public async Task scheduled_work_does_not_execute_in_parallel() for (var i = 0; i < 3; i++) { - var task = scheduler.Schedule(i, async _ => + var task = scheduler.Schedule(i, async v => { Interlocked.Increment(ref concurrencyCounter); @@ -87,6 +88,7 @@ public async Task scheduled_work_does_not_execute_in_parallel() maxObservedParallelism = Math.Max(concurrencyCounter, maxObservedParallelism); Interlocked.Decrement(ref concurrencyCounter); + return v; }); tasks[i] = task; } @@ -101,9 +103,10 @@ public async Task deferred_work_is_executed_before_new_work() { var executionList = new List(); - void PerformWork(int v) + Task PerformWork(int v) { executionList.Add(v); + return Task.FromResult(v); } using var scheduler = new KernelScheduler(); @@ -124,17 +127,26 @@ public void cancel_scheduler_work_prevents_any_scheduled_work_from_executing() var executionList = new List(); using var scheduler = new KernelScheduler(); var barrier = new Barrier(2); - void PerformWork(int v) + Task PerformWork(int v) { barrier.SignalAndWait(5000); executionList.Add(v); + return Task.FromResult(v); } var scheduledWork = new List { scheduler.Schedule(1, PerformWork), - scheduler.Schedule(2, executionList.Add), - scheduler.Schedule(3, executionList.Add) + scheduler.Schedule(2, v => + { + executionList.Add(v); + return Task.FromResult(v); + }), + scheduler.Schedule(3, v => + { + executionList.Add(v); + return Task.FromResult(v); + }) }; barrier.SignalAndWait(); @@ -152,18 +164,27 @@ public async Task cancelled_work_prevents_any_scheduled_work_from_executing() using var scheduler = new KernelScheduler(); var barrier = new Barrier(2); - async Task PerformWork(int v) + async Task PerformWork(int v) { barrier.SignalAndWait(); await Task.Delay(3000); executionList.Add(v); + return v; } var scheduledWork = new List { scheduler.Schedule(1, PerformWork), - scheduler.Schedule(2, executionList.Add), - scheduler.Schedule(3, executionList.Add) + scheduler.Schedule(2, v => + { + executionList.Add(v); + return Task.FromResult(v); + }), + scheduler.Schedule(3, v => + { + executionList.Add(v); + return Task.FromResult(v); + }) }; barrier.SignalAndWait(); @@ -187,18 +208,27 @@ public void cancelling_work_throws_exception() using var scheduler = new KernelScheduler(); var barrier = new Barrier(2); - async Task PerformWork(int v) + async Task PerformWork(int v) { barrier.SignalAndWait(); await Task.Delay(3000); executionList.Add(v); + return v; } var scheduledWork = new List { scheduler.Schedule(1, PerformWork), - scheduler.Schedule(2, executionList.Add), - scheduler.Schedule(3, executionList.Add) + scheduler.Schedule(2, v => + { + executionList.Add(v); + return Task.FromResult(v); + }), + scheduler.Schedule(3, v => + { + executionList.Add(v); + return Task.FromResult(v); + }) }; barrier.SignalAndWait(); @@ -215,7 +245,7 @@ public async Task exception_in_scheduled_work_halts_execution() using var scheduler = new KernelScheduler(); var barrier = new Barrier(2); - void PerformWork(int v) + Task PerformWork(int v) { barrier.SignalAndWait(); throw new InvalidOperationException("test exception"); @@ -224,8 +254,16 @@ void PerformWork(int v) var scheduledWork = new List { scheduler.Schedule(1, PerformWork), - scheduler.Schedule(2, executionList.Add), - scheduler.Schedule(3, executionList.Add) + scheduler.Schedule(2, v => + { + executionList.Add(v); + return Task.FromResult(v); + }), + scheduler.Schedule(3, v => + { + executionList.Add(v); + return Task.FromResult(v); + }) }; barrier.SignalAndWait(); @@ -248,7 +286,7 @@ public void exception_in_scheduled_work_is_propagated() using var scheduler = new KernelScheduler(); var barrier = new Barrier(2); - void PerformWork(int v) + Task PerformWork(int v) { barrier.SignalAndWait(); throw new InvalidOperationException("test exception"); @@ -257,8 +295,16 @@ void PerformWork(int v) var scheduledWork = new List { scheduler.Schedule(1, PerformWork), - scheduler.Schedule(2, executionList.Add), - scheduler.Schedule(3, executionList.Add) + scheduler.Schedule(2, v => + { + executionList.Add(v); + return Task.FromResult(v); + }), + scheduler.Schedule(3, v => + { + executionList.Add(v); + return Task.FromResult(v); + }) }; barrier.SignalAndWait(); @@ -278,10 +324,11 @@ public async Task awaiting_for_work_to_complete_does_not_wait_for_subsequent_wor using var scheduler = new KernelScheduler(); - async Task PerformWorkAsync(int v) + async Task PerformWorkAsync(int v) { await Task.Delay(200); executionList.Add(v); + return v; } await scheduler.Schedule(1, PerformWorkAsync); @@ -299,9 +346,10 @@ public async Task deferred_work_is_done_based_on_the_scope_of_scheduled_work() { var executionList = new List(); - void PerformWork(int v) + Task PerformWork(int v) { executionList.Add(v); + return Task.FromResult(v); } using var scheduler = new KernelScheduler(); diff --git a/src/Microsoft.DotNet.Interactive/KernelScheduler.cs b/src/Microsoft.DotNet.Interactive/KernelScheduler.cs index a61901107e..6b4a1eebc2 100644 --- a/src/Microsoft.DotNet.Interactive/KernelScheduler.cs +++ b/src/Microsoft.DotNet.Interactive/KernelScheduler.cs @@ -118,8 +118,8 @@ async Task DoWork(ScheduledOperation scheduleOperation) { try { - await scheduleOperation.OnExecuteAsync(scheduleOperation.Value); - scheduleOperation.CompletionSource.SetResult(default); + var operationResult = await scheduleOperation.OnExecuteAsync(scheduleOperation.Value); + scheduleOperation.CompletionSource.SetResult(operationResult); } catch (Exception e) { @@ -159,9 +159,7 @@ public void Dispose() Cancel(); } - - - public delegate Task OnExecuteDelegate(T value); + public delegate Task OnExecuteDelegate(T value); public delegate IEnumerable GetDeferredOperationsDelegate(T operationToExecute, string queueName); diff --git a/src/Microsoft.DotNet.Interactive/KernelSchedulerExtensions.cs b/src/Microsoft.DotNet.Interactive/KernelSchedulerExtensions.cs index 9c778f8b10..bb603f6e5a 100644 --- a/src/Microsoft.DotNet.Interactive/KernelSchedulerExtensions.cs +++ b/src/Microsoft.DotNet.Interactive/KernelSchedulerExtensions.cs @@ -2,28 +2,65 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; +using System.Reactive.Disposables; using System.Threading.Tasks; +using Microsoft.DotNet.Interactive.Commands; namespace Microsoft.DotNet.Interactive { public static class KernelSchedulerExtensions { - public static Task Schedule(this KernelScheduler kernelScheduler, T value, Action onExecute, string scope = "default") - { - return kernelScheduler.Schedule(value, v => - { - onExecute(v); - return Task.CompletedTask; - },scope); - } + public static void RegisterDeferredOperationSource(this KernelScheduler kernelScheduler, KernelScheduler.GetDeferredOperationsDelegate getDeferredOperations, Action onExecute) { kernelScheduler.RegisterDeferredOperationSource(getDeferredOperations, v => { onExecute(v); - return Task.CompletedTask; + return Task.FromResult(default(U)); }); } + + internal static Task Schedule(this KernelScheduler kernelScheduler, KernelCommand kernelCommand, Kernel kernel) + { + + return kernelScheduler.Schedule( + kernelCommand, async command => + { + var context = KernelInvocationContext.Establish(command); + + // only subscribe for the root command + using var _ = + context.Command == command + ? context.KernelEvents.Subscribe(kernel.PublishEvent) + : Disposable.Empty; + + try + { + await kernel.Pipeline.SendAsync(command, context); + + if (command == context.Command) + { + await context.DisposeAsync(); + } + else + { + context.Complete(command); + } + + return context.Result; + } + catch (Exception exception) + { + if (!context.IsComplete) + { + context.Fail(exception); + } + + throw; + } + } + , kernelCommand.TargetKernelName); + } } } \ No newline at end of file From 682d49c7085f427c9445cdc3f7e53cfe79e102cf Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Tue, 23 Feb 2021 23:10:47 +0000 Subject: [PATCH 43/44] remove extension methods --- .../KernelSchedulerExtensions.cs | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/Microsoft.DotNet.Interactive/KernelSchedulerExtensions.cs b/src/Microsoft.DotNet.Interactive/KernelSchedulerExtensions.cs index bb603f6e5a..dda522c1d2 100644 --- a/src/Microsoft.DotNet.Interactive/KernelSchedulerExtensions.cs +++ b/src/Microsoft.DotNet.Interactive/KernelSchedulerExtensions.cs @@ -11,16 +11,6 @@ namespace Microsoft.DotNet.Interactive public static class KernelSchedulerExtensions { - - public static void RegisterDeferredOperationSource(this KernelScheduler kernelScheduler, KernelScheduler.GetDeferredOperationsDelegate getDeferredOperations, Action onExecute) - { - kernelScheduler.RegisterDeferredOperationSource(getDeferredOperations, v => - { - onExecute(v); - return Task.FromResult(default(U)); - }); - } - internal static Task Schedule(this KernelScheduler kernelScheduler, KernelCommand kernelCommand, Kernel kernel) { From 88cc568027448c6242db26c803af607bd415941d Mon Sep 17 00:00:00 2001 From: Diego Colombo Date: Wed, 24 Feb 2021 00:49:45 +0000 Subject: [PATCH 44/44] all broken here --- ...edulerTests.cs => KernelSchedulerTests.cs} | 6 +- .../CompositeKernel.cs | 33 +----- src/Microsoft.DotNet.Interactive/Kernel.cs | 106 ++++++++++++++---- .../KernelCommandScheduler.cs | 21 +--- .../KernelScheduler.cs | 2 +- .../KernelSchedulerExtensions.cs | 56 --------- .../KernelSupportsNugetExtensions.cs | 2 - 7 files changed, 92 insertions(+), 134 deletions(-) rename src/Microsoft.DotNet.Interactive.Tests/{KernelCommandSchedulerTests.cs => KernelSchedulerTests.cs} (98%) delete mode 100644 src/Microsoft.DotNet.Interactive/KernelSchedulerExtensions.cs diff --git a/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs b/src/Microsoft.DotNet.Interactive.Tests/KernelSchedulerTests.cs similarity index 98% rename from src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs rename to src/Microsoft.DotNet.Interactive.Tests/KernelSchedulerTests.cs index a324ccc044..bf874247d6 100644 --- a/src/Microsoft.DotNet.Interactive.Tests/KernelCommandSchedulerTests.cs +++ b/src/Microsoft.DotNet.Interactive.Tests/KernelSchedulerTests.cs @@ -18,11 +18,11 @@ namespace Microsoft.DotNet.Interactive.Tests { - public class KernelCommandSchedulerTests : IDisposable + public class KernelSchedulerTests : IDisposable { private readonly CompositeDisposable _disposables = new(); - public KernelCommandSchedulerTests(ITestOutputHelper output) + public KernelSchedulerTests(ITestOutputHelper output) { DisposeAfterTest(output.SubscribeToPocketLogger()); } @@ -35,7 +35,7 @@ public void Dispose() } catch (Exception ex) { - Logger.Log.Error(exception: ex); + Logger.Log.Error(exception: ex); } } diff --git a/src/Microsoft.DotNet.Interactive/CompositeKernel.cs b/src/Microsoft.DotNet.Interactive/CompositeKernel.cs index 77709e6bc9..32c72eb67d 100644 --- a/src/Microsoft.DotNet.Interactive/CompositeKernel.cs +++ b/src/Microsoft.DotNet.Interactive/CompositeKernel.cs @@ -73,6 +73,7 @@ public void Add(Kernel kernel, IReadOnlyCollection aliases = null) } kernel.ParentKernel = this; + kernel.SetScheduler(Scheduler); kernel.AddMiddleware(LoadExtensions); AddChooseKernelDirective(kernel, aliases); @@ -202,38 +203,6 @@ private Kernel GetHandlingKernel( return kernel ?? this; } - internal override async Task HandleAsync( - KernelCommand command, - KernelInvocationContext context) - { - var kernel = context.HandlingKernel; - - if (kernel is null) - { - throw new NoSuitableKernelException(command); - } - - switch (command) - { - case Cancel _: - CancelCommands(); - kernel.CancelCommands(); - break; - } - - await kernel.RunDeferredCommandsAsync(); - - if (kernel != this) - { - // route to a subkernel - await kernel.Pipeline.SendAsync(command, context); - } - else - { - await base.HandleAsync(command, context); - } - } - private protected override IEnumerable GetDirectiveParsersForCompletion( DirectiveNode directiveNode, int requestPosition) diff --git a/src/Microsoft.DotNet.Interactive/Kernel.cs b/src/Microsoft.DotNet.Interactive/Kernel.cs index 1af18dc764..3bec15a9a6 100644 --- a/src/Microsoft.DotNet.Interactive/Kernel.cs +++ b/src/Microsoft.DotNet.Interactive/Kernel.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.CommandLine; using System.CommandLine.Parsing; @@ -24,11 +25,12 @@ public abstract partial class Kernel : IDisposable { private readonly Subject _kernelEvents = new(); private readonly CompositeDisposable _disposables; - + private readonly ConcurrentQueue _deferredCommands = new(); private readonly Dictionary _dynamicHandlers = new(); private FrontendEnvironment _frontendEnvironment; private ChooseKernelDirective _chooseKernelDirective; - private readonly KernelCommandScheduler _scheduler; + + private KernelScheduler _commandScheduler = null; protected Kernel(string name) { @@ -45,8 +47,7 @@ protected Kernel(string name) Pipeline = new KernelCommandPipeline(this); - _scheduler = new KernelCommandScheduler(); - + AddSetKernelMiddleware(); AddDirectiveMiddlewareAndCommonCommandHandlers(); @@ -56,7 +57,17 @@ protected Kernel(string name) )); } - internal KernelCommandScheduler Scheduler => _scheduler; + internal KernelScheduler Scheduler + { + get + { + if(_commandScheduler is null) + { + SetScheduler(new ()); + } + return _commandScheduler; + } + } internal KernelCommandPipeline Pipeline { get; } @@ -76,7 +87,13 @@ public void DeferCommand(KernelCommand command) } command.SetToken($"deferredCommand::{Guid.NewGuid():N}"); - Scheduler.DeferCommand(command, this); + + if(string.IsNullOrWhiteSpace(command.TargetKernelName)) + { + command.TargetKernelName = Name; + } + + _deferredCommands.Enqueue(command); } private void AddSetKernelMiddleware() @@ -235,35 +252,20 @@ internal virtual async Task HandleAsync( public Task SendAsync( KernelCommand command, CancellationToken cancellationToken) - { - return SendAsync(command, cancellationToken, null); - } - - internal Task SendAsync( - KernelCommand command, - CancellationToken cancellationToken, - Action onDone) { if (command == null) { throw new ArgumentNullException(nameof(command)); } - return Scheduler.Schedule(command, this, cancellationToken, onDone); - + return Scheduler.Schedule(command, OnExecuteAsync, Name); + } protected internal void CancelCommands() { - Scheduler.CancelCommands(); - } - - internal Task RunDeferredCommandsAsync() - { - return Scheduler.RunDeferredCommandsAsync(this); - + Scheduler.Cancel(); } - protected internal void PublishEvent(KernelEvent kernelEvent) { if (kernelEvent == null) @@ -436,5 +438,61 @@ protected virtual ChooseKernelDirective CreateChooseKernelDirective() } internal ChooseKernelDirective ChooseKernelDirective => _chooseKernelDirective ??= CreateChooseKernelDirective(); + + internal void SetScheduler(KernelScheduler scheduler) + { + + _commandScheduler = scheduler; + + IEnumerable GetDeferredOperations(KernelCommand command, string scope) + { + if (scope != Name) + { + yield break; + } + + while (_deferredCommands.TryDequeue(out var kernelCommand)) + { + yield return kernelCommand; + } + } + + Scheduler.RegisterDeferredOperationSource(GetDeferredOperations, OnExecuteAsync); + } + + internal async Task OnExecuteAsync(KernelCommand command) + { + var context = KernelInvocationContext.Establish(command); + + // only subscribe for the root command + using var _ = context.Command == command + ? context.KernelEvents.Subscribe(PublishEvent) + : Disposable.Empty; + + try + { + await Pipeline.SendAsync(command, context); + + if (command == context.Command) + { + await context.DisposeAsync(); + } + else + { + context.Complete(command); + } + + return context.Result; + } + catch (Exception exception) + { + if (!context.IsComplete) + { + context.Fail(exception); + } + + throw; + } + } } } \ No newline at end of file diff --git a/src/Microsoft.DotNet.Interactive/KernelCommandScheduler.cs b/src/Microsoft.DotNet.Interactive/KernelCommandScheduler.cs index 72dcb7ec50..67d58ddb55 100644 --- a/src/Microsoft.DotNet.Interactive/KernelCommandScheduler.cs +++ b/src/Microsoft.DotNet.Interactive/KernelCommandScheduler.cs @@ -20,16 +20,11 @@ public class KernelCommandScheduler private readonly ConcurrentQueue _commandQueue = new(); - public Task Schedule(KernelCommand command, Kernel kernel) - { - return Schedule(command, kernel, CancellationToken.None, () => { }); - } - public Task Schedule(KernelCommand command, Kernel kernel, CancellationToken cancellationToken, Action onDone) + public Task Schedule(KernelCommand command, Kernel kernel, CancellationToken cancellationToken) { - switch (command) { case Cancel _: @@ -46,15 +41,14 @@ public Task Schedule(KernelCommand command, Kernel kernel, _commandQueue.Enqueue(operation); - ProcessCommandQueue(_commandQueue, cancellationToken, onDone); + ProcessCommandQueue(_commandQueue, cancellationToken); return kernelCommandResultSource.Task; } private void ProcessCommandQueue( ConcurrentQueue commandQueue, - CancellationToken cancellationToken, - Action onDone) + CancellationToken cancellationToken) { if (commandQueue.TryDequeue(out var currentOperation)) { @@ -64,13 +58,9 @@ private void ProcessCommandQueue( await ExecuteCommand(currentOperation); - ProcessCommandQueue(commandQueue, cancellationToken, onDone); + ProcessCommandQueue(commandQueue, cancellationToken); }, cancellationToken).ConfigureAwait(false); } - else - { - onDone?.Invoke(); - } } private async Task ExecuteCommand(KernelOperation operation) @@ -142,8 +132,7 @@ internal Task RunDeferredCommandsAsync(Kernel kernel) UndeferCommandsFor(kernel); ProcessCommandQueue( _commandQueue, - CancellationToken.None, - () => tcs.SetResult(Unit.Default)); + CancellationToken.None); return tcs.Task; } diff --git a/src/Microsoft.DotNet.Interactive/KernelScheduler.cs b/src/Microsoft.DotNet.Interactive/KernelScheduler.cs index 6b4a1eebc2..31d241b976 100644 --- a/src/Microsoft.DotNet.Interactive/KernelScheduler.cs +++ b/src/Microsoft.DotNet.Interactive/KernelScheduler.cs @@ -161,7 +161,7 @@ public void Dispose() public delegate Task OnExecuteDelegate(T value); - public delegate IEnumerable GetDeferredOperationsDelegate(T operationToExecute, string queueName); + public delegate IEnumerable GetDeferredOperationsDelegate(T state, string scope); private class ScheduledOperation { diff --git a/src/Microsoft.DotNet.Interactive/KernelSchedulerExtensions.cs b/src/Microsoft.DotNet.Interactive/KernelSchedulerExtensions.cs deleted file mode 100644 index dda522c1d2..0000000000 --- a/src/Microsoft.DotNet.Interactive/KernelSchedulerExtensions.cs +++ /dev/null @@ -1,56 +0,0 @@ -// Copyright (c) .NET Foundation and contributors. All rights reserved. -// Licensed under the MIT license. See LICENSE file in the project root for full license information. - -using System; -using System.Reactive.Disposables; -using System.Threading.Tasks; -using Microsoft.DotNet.Interactive.Commands; - -namespace Microsoft.DotNet.Interactive -{ - public static class KernelSchedulerExtensions - { - - internal static Task Schedule(this KernelScheduler kernelScheduler, KernelCommand kernelCommand, Kernel kernel) - { - - return kernelScheduler.Schedule( - kernelCommand, async command => - { - var context = KernelInvocationContext.Establish(command); - - // only subscribe for the root command - using var _ = - context.Command == command - ? context.KernelEvents.Subscribe(kernel.PublishEvent) - : Disposable.Empty; - - try - { - await kernel.Pipeline.SendAsync(command, context); - - if (command == context.Command) - { - await context.DisposeAsync(); - } - else - { - context.Complete(command); - } - - return context.Result; - } - catch (Exception exception) - { - if (!context.IsComplete) - { - context.Fail(exception); - } - - throw; - } - } - , kernelCommand.TargetKernelName); - } - } -} \ No newline at end of file diff --git a/src/Microsoft.DotNet.Interactive/KernelSupportsNugetExtensions.cs b/src/Microsoft.DotNet.Interactive/KernelSupportsNugetExtensions.cs index 8a6559575c..b16e85eee7 100644 --- a/src/Microsoft.DotNet.Interactive/KernelSupportsNugetExtensions.cs +++ b/src/Microsoft.DotNet.Interactive/KernelSupportsNugetExtensions.cs @@ -263,8 +263,6 @@ internal static KernelCommandInvocation DoNugetRestore() }; await invocationContext.QueueAction(restore); - var kernel = invocationContext.HandlingKernel; - await kernel.RunDeferredCommandsAsync(); }; static string InstallingPackageMessage(PackageReference package)