From 5dd3a493b82075532828bcee0315238c5ada1b20 Mon Sep 17 00:00:00 2001 From: Diego Date: Fri, 23 Aug 2019 17:06:40 +0100 Subject: [PATCH 01/20] wip --- .../FSharpWorkspaceShim.fsproj | 4 +- .../ExecuteRequestHandlerTests.cs | 4 -- .../InterruptRequestHandler.cs | 40 +++++++++++++++++++ .../JupyterRequestContextHandler.cs | 5 +++ .../Protocol/InterruptReply.cs | 11 +++++ .../Protocol/InterruptRequest.cs | 11 +++++ .../Protocol/MessageTypeValues.cs | 4 ++ 7 files changed, 72 insertions(+), 7 deletions(-) create mode 100644 Microsoft.DotNet.Interactive.Jupyter/InterruptRequestHandler.cs create mode 100644 Microsoft.DotNet.Interactive.Jupyter/Protocol/InterruptReply.cs create mode 100644 Microsoft.DotNet.Interactive.Jupyter/Protocol/InterruptRequest.cs diff --git a/FSharpWorkspaceShim/FSharpWorkspaceShim.fsproj b/FSharpWorkspaceShim/FSharpWorkspaceShim.fsproj index 41b0ba9c3..be8b7ff92 100644 --- a/FSharpWorkspaceShim/FSharpWorkspaceShim.fsproj +++ b/FSharpWorkspaceShim/FSharpWorkspaceShim.fsproj @@ -9,9 +9,7 @@ false - - - + diff --git a/Microsoft.DotNet.Interactive.Jupyter.Tests/ExecuteRequestHandlerTests.cs b/Microsoft.DotNet.Interactive.Jupyter.Tests/ExecuteRequestHandlerTests.cs index e25aa3a08..138bd6d51 100644 --- a/Microsoft.DotNet.Interactive.Jupyter.Tests/ExecuteRequestHandlerTests.cs +++ b/Microsoft.DotNet.Interactive.Jupyter.Tests/ExecuteRequestHandlerTests.cs @@ -7,10 +7,6 @@ using Microsoft.DotNet.Interactive.Jupyter.Protocol; using WorkspaceServer.Kernel; using Xunit; -using System.Reactive.Linq; -using Microsoft.DotNet.Interactive.Events; -using FluentAssertions.Extensions; -using System.Reactive.Concurrency; namespace Microsoft.DotNet.Interactive.Jupyter.Tests { diff --git a/Microsoft.DotNet.Interactive.Jupyter/InterruptRequestHandler.cs b/Microsoft.DotNet.Interactive.Jupyter/InterruptRequestHandler.cs new file mode 100644 index 000000000..cbaafb3f7 --- /dev/null +++ b/Microsoft.DotNet.Interactive.Jupyter/InterruptRequestHandler.cs @@ -0,0 +1,40 @@ +// 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.Reactive.Concurrency; +using System.Threading.Tasks; +using Microsoft.DotNet.Interactive.Events; +using Microsoft.DotNet.Interactive.Jupyter.Protocol; + +namespace Microsoft.DotNet.Interactive.Jupyter +{ + public class InterruptRequestHandler : RequestHandlerBase + { + + public InterruptRequestHandler(IKernel kernel, IScheduler scheduler = null) + : base(kernel, scheduler ?? CurrentThreadScheduler.Instance) + { + + } + + protected override void OnKernelEvent(IKernelEvent @event) + { + throw new System.NotImplementedException(); + } + + public async Task Handle(JupyterRequestContext context) + { + var interruptRequest = GetJupyterRequest(context); + + context.RequestHandlerStatus.SetAsBusy(); + + var command = new InterrupEvaluation(); + + var openRequest = new InflightRequest(context, interruptRequest, 0); + + InFlightRequests[command] = openRequest; + + await Kernel.SendAsync(command); + } + } +} \ No newline at end of file diff --git a/Microsoft.DotNet.Interactive.Jupyter/JupyterRequestContextHandler.cs b/Microsoft.DotNet.Interactive.Jupyter/JupyterRequestContextHandler.cs index 54498cdea..b7fd2d773 100644 --- a/Microsoft.DotNet.Interactive.Jupyter/JupyterRequestContextHandler.cs +++ b/Microsoft.DotNet.Interactive.Jupyter/JupyterRequestContextHandler.cs @@ -15,6 +15,7 @@ public class JupyterRequestContextHandler : ICommandHandler Handle( case MessageTypeValues.CompleteRequest: await _completeHandler.Handle(delivery.Command); break; + case MessageTypeValues.InterruptRequest: + await _interruptHandler.Handle(delivery.Command); + break; } return delivery.Complete(); diff --git a/Microsoft.DotNet.Interactive.Jupyter/Protocol/InterruptReply.cs b/Microsoft.DotNet.Interactive.Jupyter/Protocol/InterruptReply.cs new file mode 100644 index 000000000..5765bbdd3 --- /dev/null +++ b/Microsoft.DotNet.Interactive.Jupyter/Protocol/InterruptReply.cs @@ -0,0 +1,11 @@ +// 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. + +namespace Microsoft.DotNet.Interactive.Jupyter.Protocol +{ + [JupyterMessageType(MessageTypeValues.InputReply)] + public class InterruptReply : JupyterMessageContent + { + + } +} \ No newline at end of file diff --git a/Microsoft.DotNet.Interactive.Jupyter/Protocol/InterruptRequest.cs b/Microsoft.DotNet.Interactive.Jupyter/Protocol/InterruptRequest.cs new file mode 100644 index 000000000..b34872baf --- /dev/null +++ b/Microsoft.DotNet.Interactive.Jupyter/Protocol/InterruptRequest.cs @@ -0,0 +1,11 @@ +// 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. + +namespace Microsoft.DotNet.Interactive.Jupyter.Protocol +{ + [JupyterMessageType(MessageTypeValues.InterruptRequest)] + public class InterruptRequest : JupyterMessageContent + { + + } +} \ No newline at end of file diff --git a/Microsoft.DotNet.Interactive.Jupyter/Protocol/MessageTypeValues.cs b/Microsoft.DotNet.Interactive.Jupyter/Protocol/MessageTypeValues.cs index 0be82eca5..b7216767f 100644 --- a/Microsoft.DotNet.Interactive.Jupyter/Protocol/MessageTypeValues.cs +++ b/Microsoft.DotNet.Interactive.Jupyter/Protocol/MessageTypeValues.cs @@ -62,5 +62,9 @@ public class MessageTypeValues public const string CommInfoRequest = "comm_info_request"; public const string CommInfoReply = "comm_info_reply"; + + public const string InterruptRequest = "interrupt_request"; + + public const string InterruptReply = "interrupt_reply"; } } From 67e7aa2fe1a91013891c9b6d3277b7219950893a Mon Sep 17 00:00:00 2001 From: Diego Date: Fri, 23 Aug 2019 17:06:55 +0100 Subject: [PATCH 02/20] add test --- .../InterruptRequestHandlerTests.cs | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 Microsoft.DotNet.Interactive.Jupyter.Tests/InterruptRequestHandlerTests.cs diff --git a/Microsoft.DotNet.Interactive.Jupyter.Tests/InterruptRequestHandlerTests.cs b/Microsoft.DotNet.Interactive.Jupyter.Tests/InterruptRequestHandlerTests.cs new file mode 100644 index 000000000..8e4464b1f --- /dev/null +++ b/Microsoft.DotNet.Interactive.Jupyter.Tests/InterruptRequestHandlerTests.cs @@ -0,0 +1,10 @@ +// 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. + +namespace Microsoft.DotNet.Interactive.Jupyter.Tests +{ + public class InterruptRequestHandlerTests + { + + } +} \ No newline at end of file From df3ee45421cc59fd8d9dbde9e47fd257f7266f4b Mon Sep 17 00:00:00 2001 From: Diego Date: Fri, 23 Aug 2019 17:49:45 +0100 Subject: [PATCH 03/20] InterruptExecution command --- .../InterruptRequestHandler.cs | 3 ++- .../JupyterRequestContextHandler.cs | 2 +- .../Commands/InterruptExecution.cs | 10 ++++++++++ .../Commands/RequestCompletion.cs | 2 -- 4 files changed, 13 insertions(+), 4 deletions(-) create mode 100644 Microsoft.DotNet.Interactive/Commands/InterruptExecution.cs diff --git a/Microsoft.DotNet.Interactive.Jupyter/InterruptRequestHandler.cs b/Microsoft.DotNet.Interactive.Jupyter/InterruptRequestHandler.cs index cbaafb3f7..6b8adf7d4 100644 --- a/Microsoft.DotNet.Interactive.Jupyter/InterruptRequestHandler.cs +++ b/Microsoft.DotNet.Interactive.Jupyter/InterruptRequestHandler.cs @@ -3,6 +3,7 @@ using System.Reactive.Concurrency; using System.Threading.Tasks; +using Microsoft.DotNet.Interactive.Commands; using Microsoft.DotNet.Interactive.Events; using Microsoft.DotNet.Interactive.Jupyter.Protocol; @@ -28,7 +29,7 @@ public async Task Handle(JupyterRequestContext context) context.RequestHandlerStatus.SetAsBusy(); - var command = new InterrupEvaluation(); + var command = new InterruptExecution(); var openRequest = new InflightRequest(context, interruptRequest, 0); diff --git a/Microsoft.DotNet.Interactive.Jupyter/JupyterRequestContextHandler.cs b/Microsoft.DotNet.Interactive.Jupyter/JupyterRequestContextHandler.cs index b7fd2d773..3f22cdc03 100644 --- a/Microsoft.DotNet.Interactive.Jupyter/JupyterRequestContextHandler.cs +++ b/Microsoft.DotNet.Interactive.Jupyter/JupyterRequestContextHandler.cs @@ -15,7 +15,7 @@ public class JupyterRequestContextHandler : ICommandHandler Date: Fri, 23 Aug 2019 18:42:51 +0100 Subject: [PATCH 04/20] wip --- .../Events/CodeSubmissionInterrupted.cs | 15 +++++++++++ WorkspaceServer/Kernel/CSharpKernel.cs | 26 +++++++++---------- 2 files changed, 27 insertions(+), 14 deletions(-) create mode 100644 Microsoft.DotNet.Interactive/Events/CodeSubmissionInterrupted.cs diff --git a/Microsoft.DotNet.Interactive/Events/CodeSubmissionInterrupted.cs b/Microsoft.DotNet.Interactive/Events/CodeSubmissionInterrupted.cs new file mode 100644 index 000000000..943a7e020 --- /dev/null +++ b/Microsoft.DotNet.Interactive/Events/CodeSubmissionInterrupted.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 Microsoft.DotNet.Interactive.Commands; + +namespace Microsoft.DotNet.Interactive.Events +{ + public class CodeSubmissionInterrupted:KernelEventBase + { + public CodeSubmissionInterrupted(SubmitCode command) : base(command) + { + + } + } +} \ No newline at end of file diff --git a/WorkspaceServer/Kernel/CSharpKernel.cs b/WorkspaceServer/Kernel/CSharpKernel.cs index 50141acb6..cf4a0327e 100644 --- a/WorkspaceServer/Kernel/CSharpKernel.cs +++ b/WorkspaceServer/Kernel/CSharpKernel.cs @@ -7,6 +7,7 @@ using System.Linq; using System.Reflection; using System.Text; +using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Completion; @@ -36,6 +37,7 @@ public class CSharpKernel : KernelBase protected ScriptOptions ScriptOptions; private ImmutableArray _metadataReferences; private WorkspaceFixture _fixture; + private CancellationTokenSource _evaluationInFlight; public CSharpKernel() { @@ -104,14 +106,15 @@ private async Task HandleSubmitCode( using var console = await ConsoleOutput.Capture(); using var _ = console.SubscribeToStandardOutput(std => PublishOutput(std, context, submitCode)); - + _evaluationInFlight = new CancellationTokenSource(); try { if (_scriptState == null) { _scriptState = await CSharpScript.RunAsync( code, - ScriptOptions); + ScriptOptions, + cancellationToken: _evaluationInFlight.Token); } else { @@ -122,7 +125,7 @@ private async Task HandleSubmitCode( { exception = e; return true; - }); + }, cancellationToken: _evaluationInFlight.Token); } } catch (Exception e) @@ -130,17 +133,14 @@ private async Task HandleSubmitCode( exception = e; } - if (exception != null) + if (_evaluationInFlight.IsCancellationRequested) { - var message = string.Join("\n", (_scriptState?.Script?.GetDiagnostics() ?? - Enumerable.Empty()).Select(d => d.GetMessage())); - - context.OnNext(new CodeSubmissionEvaluationFailed(exception, message, submitCode)); - context.OnError(exception); + context.OnNext(new CodeSubmissionInterrupted(submitCode)); + context.OnCompleted(); } else { - if (HasReturnValue) + if (exception != null) { var formattedValues = FormattedValue.FromObject(_scriptState.ReturnValue); context.OnNext( @@ -150,11 +150,9 @@ private async Task HandleSubmitCode( true, formattedValues)); } - - context.OnNext(new CodeSubmissionEvaluated(submitCode)); - - context.OnCompleted(); } + + _evaluationInFlight = null; } private void PublishOutput( From 638ac34f3b0166db472957e60611be9c4f4cb45b Mon Sep 17 00:00:00 2001 From: Diego Date: Tue, 27 Aug 2019 20:17:43 +0100 Subject: [PATCH 05/20] format file --- WorkspaceServer/Kernel/CSharpKernel.cs | 56 +++++++++++++------------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/WorkspaceServer/Kernel/CSharpKernel.cs b/WorkspaceServer/Kernel/CSharpKernel.cs index fedb59809..271fecec0 100644 --- a/WorkspaceServer/Kernel/CSharpKernel.cs +++ b/WorkspaceServer/Kernel/CSharpKernel.cs @@ -139,37 +139,37 @@ private async Task HandleSubmitCode( } else { - if (exception != null) - { - string message = null; - - if (exception is CompilationErrorException compilationError) + if (exception != null) { - message = - string.Join(Environment.NewLine, - compilationError.Diagnostics.Select(d => d.ToString())); - } + string message = null; - context.OnNext(new CommandFailed(exception, submitCode, message)); - context.OnError(exception); - } - else - { - if (HasReturnValue) + if (exception is CompilationErrorException compilationError) + { + message = + string.Join(Environment.NewLine, + compilationError.Diagnostics.Select(d => d.ToString())); + } + + context.OnNext(new CommandFailed(exception, submitCode, message)); + context.OnError(exception); + } + else { - var formattedValues = FormattedValue.FromObject(_scriptState.ReturnValue); - context.OnNext( - new ValueProduced( - _scriptState.ReturnValue, - submitCode, - true, - formattedValues)); + if (HasReturnValue) + { + var formattedValues = FormattedValue.FromObject(_scriptState.ReturnValue); + context.OnNext( + new ValueProduced( + _scriptState.ReturnValue, + submitCode, + true, + formattedValues)); + } + + context.OnNext(new CodeSubmissionEvaluated(submitCode)); + + context.OnCompleted(); } - - context.OnNext(new CodeSubmissionEvaluated(submitCode)); - - context.OnCompleted(); - } } _evaluationInFlight = null; @@ -229,7 +229,7 @@ private async Task> GetCompletionList(string code, i var compilation = scriptState.Script.GetCompilation(); metadataReferences = metadataReferences.AddRange(compilation.References); var originalCode = forcedState ? string.Empty : scriptState.Script.Code ?? string.Empty; - + var buffer = new StringBuilder(originalCode); if (!string.IsNullOrWhiteSpace(originalCode) && !originalCode.EndsWith(Environment.NewLine)) { From a65def90819afc66317a1010222dbca9c243ebef Mon Sep 17 00:00:00 2001 From: Diego Date: Thu, 29 Aug 2019 17:14:13 +0100 Subject: [PATCH 06/20] split files --- .../Commands/SubmissionType.cs | 11 +++++++++++ Microsoft.DotNet.Interactive/Commands/SubmitCode.cs | 7 +------ 2 files changed, 12 insertions(+), 6 deletions(-) create mode 100644 Microsoft.DotNet.Interactive/Commands/SubmissionType.cs diff --git a/Microsoft.DotNet.Interactive/Commands/SubmissionType.cs b/Microsoft.DotNet.Interactive/Commands/SubmissionType.cs new file mode 100644 index 000000000..5d376f524 --- /dev/null +++ b/Microsoft.DotNet.Interactive/Commands/SubmissionType.cs @@ -0,0 +1,11 @@ +// 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. + +namespace Microsoft.DotNet.Interactive.Commands +{ + public enum SubmissionType + { + Execution, + AnalysisOnly + } +} \ No newline at end of file diff --git a/Microsoft.DotNet.Interactive/Commands/SubmitCode.cs b/Microsoft.DotNet.Interactive/Commands/SubmitCode.cs index 6c62eb721..62bbcedee 100644 --- a/Microsoft.DotNet.Interactive/Commands/SubmitCode.cs +++ b/Microsoft.DotNet.Interactive/Commands/SubmitCode.cs @@ -20,14 +20,9 @@ public SubmitCode( public string Code { get; set; } public string TargetKernelName { get; set; } + public SubmissionType SubmissionType { get; } public override string ToString() => $"{base.ToString()}: {Code.TruncateForDisplay()}"; } - - public enum SubmissionType - { - Execution, - AnalysisOnly - } } \ No newline at end of file From 3a740e452c6fb385e1fbcca13f21f8918da99b07 Mon Sep 17 00:00:00 2001 From: Diego Date: Fri, 30 Aug 2019 14:49:26 +0100 Subject: [PATCH 07/20] rename command and event --- .../InterruptRequestHandler.cs | 27 +++++++++++++++++-- .../IsCompleteRequestHandler.cs | 2 -- ...terruptExecution.cs => InterruptKernel.cs} | 2 +- ...ionInterrupted.cs => KernelInterrupted.cs} | 4 +-- 4 files changed, 28 insertions(+), 7 deletions(-) rename Microsoft.DotNet.Interactive/Commands/{InterruptExecution.cs => InterruptKernel.cs} (81%) rename Microsoft.DotNet.Interactive/Events/{CodeSubmissionInterrupted.cs => KernelInterrupted.cs} (70%) diff --git a/Microsoft.DotNet.Interactive.Jupyter/InterruptRequestHandler.cs b/Microsoft.DotNet.Interactive.Jupyter/InterruptRequestHandler.cs index 6b8adf7d4..01b583cb1 100644 --- a/Microsoft.DotNet.Interactive.Jupyter/InterruptRequestHandler.cs +++ b/Microsoft.DotNet.Interactive.Jupyter/InterruptRequestHandler.cs @@ -20,7 +20,30 @@ public InterruptRequestHandler(IKernel kernel, IScheduler scheduler = null) protected override void OnKernelEvent(IKernelEvent @event) { - throw new System.NotImplementedException(); + switch (@event) + { + case KernelInterrupted kernelInterrupted: + OnKernelInterrupted(kernelInterrupted); + break; + } + } + + private void OnKernelInterrupted(KernelInterrupted kernelInterrupted) + { + if (InFlightRequests.TryRemove(kernelInterrupted.Command, out var openRequest)) + { + // reply + var interruptReplyPayload = new InterruptReply(); + + // send to server + var executeReply = Message.CreateResponse( + interruptReplyPayload, + openRequest.Context.Request); + + openRequest.Context.ServerChannel.Send(executeReply); + openRequest.Context.RequestHandlerStatus.SetAsIdle(); + openRequest.Dispose(); + } } public async Task Handle(JupyterRequestContext context) @@ -29,7 +52,7 @@ public async Task Handle(JupyterRequestContext context) context.RequestHandlerStatus.SetAsBusy(); - var command = new InterruptExecution(); + var command = new InterruptKernel(); var openRequest = new InflightRequest(context, interruptRequest, 0); diff --git a/Microsoft.DotNet.Interactive.Jupyter/IsCompleteRequestHandler.cs b/Microsoft.DotNet.Interactive.Jupyter/IsCompleteRequestHandler.cs index 054fe360f..c0ddf9e70 100644 --- a/Microsoft.DotNet.Interactive.Jupyter/IsCompleteRequestHandler.cs +++ b/Microsoft.DotNet.Interactive.Jupyter/IsCompleteRequestHandler.cs @@ -63,7 +63,5 @@ private void OnKernelEvent(IKernelEvent @event, bool isComplete) openRequest.Dispose(); } } - - } } \ No newline at end of file diff --git a/Microsoft.DotNet.Interactive/Commands/InterruptExecution.cs b/Microsoft.DotNet.Interactive/Commands/InterruptKernel.cs similarity index 81% rename from Microsoft.DotNet.Interactive/Commands/InterruptExecution.cs rename to Microsoft.DotNet.Interactive/Commands/InterruptKernel.cs index a37a4a53f..ce2ab47b2 100644 --- a/Microsoft.DotNet.Interactive/Commands/InterruptExecution.cs +++ b/Microsoft.DotNet.Interactive/Commands/InterruptKernel.cs @@ -3,7 +3,7 @@ namespace Microsoft.DotNet.Interactive.Commands { - public class InterruptExecution : KernelCommandBase + public class InterruptKernel : KernelCommandBase { } diff --git a/Microsoft.DotNet.Interactive/Events/CodeSubmissionInterrupted.cs b/Microsoft.DotNet.Interactive/Events/KernelInterrupted.cs similarity index 70% rename from Microsoft.DotNet.Interactive/Events/CodeSubmissionInterrupted.cs rename to Microsoft.DotNet.Interactive/Events/KernelInterrupted.cs index 943a7e020..8de201a79 100644 --- a/Microsoft.DotNet.Interactive/Events/CodeSubmissionInterrupted.cs +++ b/Microsoft.DotNet.Interactive/Events/KernelInterrupted.cs @@ -5,9 +5,9 @@ namespace Microsoft.DotNet.Interactive.Events { - public class CodeSubmissionInterrupted:KernelEventBase + public class KernelInterrupted:KernelEventBase { - public CodeSubmissionInterrupted(SubmitCode command) : base(command) + public KernelInterrupted(IKernelCommand command) : base(command) { } From 87224c375ba815f074b8304d36c1baa7cb5d4a8c Mon Sep 17 00:00:00 2001 From: Diego Date: Fri, 30 Aug 2019 14:55:35 +0100 Subject: [PATCH 08/20] wip --- MLS.Agent/CommandLine/KernelServerCommand.cs | 1 - Microsoft.DotNet.Interactive/KernelStreamClient.cs | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/MLS.Agent/CommandLine/KernelServerCommand.cs b/MLS.Agent/CommandLine/KernelServerCommand.cs index 9b1b16bb6..36d4c261f 100644 --- a/MLS.Agent/CommandLine/KernelServerCommand.cs +++ b/MLS.Agent/CommandLine/KernelServerCommand.cs @@ -5,7 +5,6 @@ using System.CommandLine; using System.Threading.Tasks; using Microsoft.DotNet.Interactive; -using WorkspaceServer.Kernel; namespace MLS.Agent.CommandLine { diff --git a/Microsoft.DotNet.Interactive/KernelStreamClient.cs b/Microsoft.DotNet.Interactive/KernelStreamClient.cs index 33d896afd..90de16f28 100644 --- a/Microsoft.DotNet.Interactive/KernelStreamClient.cs +++ b/Microsoft.DotNet.Interactive/KernelStreamClient.cs @@ -18,6 +18,7 @@ public class KernelStreamClient private readonly TextReader _input; private readonly TextWriter _output; private readonly CommandDeserializer _deserializer = new CommandDeserializer(); + private readonly JsonSerializerSettings _jsonSerializerSettings = new JsonSerializerSettings { ContractResolver = new CamelCasePropertyNamesContractResolver() From b8730a5cebc5ec807b69fe3e6a78dbfc49d07572 Mon Sep 17 00:00:00 2001 From: Diego Date: Tue, 3 Sep 2019 10:15:01 +0100 Subject: [PATCH 09/20] rename command and event --- .../InterruptRequestHandler.cs | 10 ++++---- ...terruptKernel.cs => InterruptExecution.cs} | 2 +- ...Interrupted.cs => ExecutionInterrupted.cs} | 4 ++-- WorkspaceServer/Kernel/CSharpKernel.cs | 24 ++++++++++++++++--- 4 files changed, 29 insertions(+), 11 deletions(-) rename Microsoft.DotNet.Interactive/Commands/{InterruptKernel.cs => InterruptExecution.cs} (81%) rename Microsoft.DotNet.Interactive/Events/{KernelInterrupted.cs => ExecutionInterrupted.cs} (70%) diff --git a/Microsoft.DotNet.Interactive.Jupyter/InterruptRequestHandler.cs b/Microsoft.DotNet.Interactive.Jupyter/InterruptRequestHandler.cs index 01b583cb1..6183767df 100644 --- a/Microsoft.DotNet.Interactive.Jupyter/InterruptRequestHandler.cs +++ b/Microsoft.DotNet.Interactive.Jupyter/InterruptRequestHandler.cs @@ -22,15 +22,15 @@ protected override void OnKernelEvent(IKernelEvent @event) { switch (@event) { - case KernelInterrupted kernelInterrupted: - OnKernelInterrupted(kernelInterrupted); + case ExecutionInterrupted kernelInterrupted: + OnExecutionInterrupted(kernelInterrupted); break; } } - private void OnKernelInterrupted(KernelInterrupted kernelInterrupted) + private void OnExecutionInterrupted(ExecutionInterrupted executionInterrupted) { - if (InFlightRequests.TryRemove(kernelInterrupted.Command, out var openRequest)) + if (InFlightRequests.TryRemove(executionInterrupted.Command, out var openRequest)) { // reply var interruptReplyPayload = new InterruptReply(); @@ -52,7 +52,7 @@ public async Task Handle(JupyterRequestContext context) context.RequestHandlerStatus.SetAsBusy(); - var command = new InterruptKernel(); + var command = new InterruptExecution(); var openRequest = new InflightRequest(context, interruptRequest, 0); diff --git a/Microsoft.DotNet.Interactive/Commands/InterruptKernel.cs b/Microsoft.DotNet.Interactive/Commands/InterruptExecution.cs similarity index 81% rename from Microsoft.DotNet.Interactive/Commands/InterruptKernel.cs rename to Microsoft.DotNet.Interactive/Commands/InterruptExecution.cs index ce2ab47b2..a37a4a53f 100644 --- a/Microsoft.DotNet.Interactive/Commands/InterruptKernel.cs +++ b/Microsoft.DotNet.Interactive/Commands/InterruptExecution.cs @@ -3,7 +3,7 @@ namespace Microsoft.DotNet.Interactive.Commands { - public class InterruptKernel : KernelCommandBase + public class InterruptExecution : KernelCommandBase { } diff --git a/Microsoft.DotNet.Interactive/Events/KernelInterrupted.cs b/Microsoft.DotNet.Interactive/Events/ExecutionInterrupted.cs similarity index 70% rename from Microsoft.DotNet.Interactive/Events/KernelInterrupted.cs rename to Microsoft.DotNet.Interactive/Events/ExecutionInterrupted.cs index 8de201a79..12e50a78a 100644 --- a/Microsoft.DotNet.Interactive/Events/KernelInterrupted.cs +++ b/Microsoft.DotNet.Interactive/Events/ExecutionInterrupted.cs @@ -5,9 +5,9 @@ namespace Microsoft.DotNet.Interactive.Events { - public class KernelInterrupted:KernelEventBase + public class ExecutionInterrupted:KernelEventBase { - public KernelInterrupted(IKernelCommand command) : base(command) + public ExecutionInterrupted(IKernelCommand command) : base(command) { } diff --git a/WorkspaceServer/Kernel/CSharpKernel.cs b/WorkspaceServer/Kernel/CSharpKernel.cs index f21f7a56e..6297c5b1f 100644 --- a/WorkspaceServer/Kernel/CSharpKernel.cs +++ b/WorkspaceServer/Kernel/CSharpKernel.cs @@ -33,7 +33,8 @@ public class CSharpKernel : KernelBase private static readonly MethodInfo _hasReturnValueMethod = typeof(Script) .GetMethod("HasReturnValue", BindingFlags.Instance | BindingFlags.NonPublic); - protected CSharpParseOptions ParseOptions = new CSharpParseOptions(LanguageVersion.Default, kind: SourceCodeKind.Script); + protected CSharpParseOptions ParseOptions = + new CSharpParseOptions(LanguageVersion.Default, kind: SourceCodeKind.Script); private ScriptState _scriptState; @@ -87,6 +88,13 @@ protected override async Task HandleAsync( await HandleRequestCompletion(requestCompletion, invocationContext, _scriptState); }; break; + + case InterruptExecution interruptExecution: + interruptExecution.Handler = async invocationContext => + { + await HandleInterruptExecution(interruptExecution, invocationContext); + }; + break; } } @@ -95,9 +103,19 @@ public Task IsCompleteSubmissionAsync(string code) var syntaxTree = SyntaxFactory.ParseSyntaxTree(code, ParseOptions); return Task.FromResult(SyntaxFactory.IsCompleteSubmission(syntaxTree)); } - - private async Task HandleSubmitCode( + private async Task HandleInterruptExecution( + InterruptExecution interruptExecution, + KernelInvocationContext context) + { + var reply = new ExecutionInterrupted(interruptExecution); + context.Publish(reply); + throw new NotImplementedException(); + } + + + + private async Task HandleSubmitCode( SubmitCode submitCode, KernelInvocationContext context) { From 3759d1539730c3267df8e02d87bd81897e34c560 Mon Sep 17 00:00:00 2001 From: Diego Date: Tue, 3 Sep 2019 10:15:10 +0100 Subject: [PATCH 10/20] add handler tests --- .../InterruptRequestHandlerTests.cs | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/Microsoft.DotNet.Interactive.Jupyter.Tests/InterruptRequestHandlerTests.cs b/Microsoft.DotNet.Interactive.Jupyter.Tests/InterruptRequestHandlerTests.cs index 8e4464b1f..094934f8a 100644 --- a/Microsoft.DotNet.Interactive.Jupyter.Tests/InterruptRequestHandlerTests.cs +++ b/Microsoft.DotNet.Interactive.Jupyter.Tests/InterruptRequestHandlerTests.cs @@ -1,10 +1,66 @@ // 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 Microsoft.DotNet.Interactive.Jupyter.Protocol; +using Recipes; +using WorkspaceServer.Kernel; +using Xunit; + namespace Microsoft.DotNet.Interactive.Jupyter.Tests { public class InterruptRequestHandlerTests { + private readonly MessageSender _ioPubChannel; + private readonly MessageSender _serverChannel; + private readonly RecordingSocket _serverRecordingSocket; + private readonly RecordingSocket _ioRecordingSocket; + private readonly KernelStatus _kernelStatus; + + public InterruptRequestHandlerTests() + { + var signatureValidator = new SignatureValidator("key", "HMACSHA256"); + _serverRecordingSocket = new RecordingSocket(); + _serverChannel = new MessageSender(_serverRecordingSocket, signatureValidator); + _ioRecordingSocket = new RecordingSocket(); + _ioPubChannel = new MessageSender(_ioRecordingSocket, signatureValidator); + _kernelStatus = new KernelStatus(); + } + + [Fact] + public void cannot_handle_requests_that_are_not_InterruptRequest() + { + var kernel = new CSharpKernel(); + var handler = new InterruptRequestHandler(kernel); + var request = Message.Create(new DisplayData(), null); + Func messageHandling = () => handler.Handle(new JupyterRequestContext(_serverChannel, _ioPubChannel, request, _kernelStatus)); + messageHandling.Should().ThrowExactly(); + } + + [Fact] + public async Task handles_InterruptRequest() + { + var kernel = new CSharpKernel(); + var handler = new InterruptRequestHandler(kernel); + var request = Message.Create(new InterruptRequest(), null); + await handler.Handle(new JupyterRequestContext(_serverChannel, _ioPubChannel, request, _kernelStatus)); + } + + [Fact] + public async Task sends_InterruptReply() + { + var kernel = new CSharpKernel(); + var handler = new IsCompleteRequestHandler(kernel); + var request = Message.Create(new InterruptRequest(), null); + await handler.Handle(new JupyterRequestContext(_serverChannel, _ioPubChannel, request, _kernelStatus)); + _serverRecordingSocket.DecodedMessages.SingleOrDefault(message => + message.Contains(MessageTypeValues.InterruptReply)) + .Should() + .NotBeNullOrWhiteSpace(); + } } } \ No newline at end of file From de5291d7e08f0dc6eb6b0939b4de08aa7733a87d Mon Sep 17 00:00:00 2001 From: Diego Date: Tue, 3 Sep 2019 11:19:28 +0100 Subject: [PATCH 11/20] wire handler and kernel --- .../InterruptRequestHandlerTests.cs | 2 +- .../InterruptRequestHandler.cs | 4 ++-- .../Protocol/InterruptReply.cs | 2 +- WorkspaceServer/Kernel/CSharpKernel.cs | 14 +++++++++++--- 4 files changed, 15 insertions(+), 7 deletions(-) diff --git a/Microsoft.DotNet.Interactive.Jupyter.Tests/InterruptRequestHandlerTests.cs b/Microsoft.DotNet.Interactive.Jupyter.Tests/InterruptRequestHandlerTests.cs index 094934f8a..4fe4eb32a 100644 --- a/Microsoft.DotNet.Interactive.Jupyter.Tests/InterruptRequestHandlerTests.cs +++ b/Microsoft.DotNet.Interactive.Jupyter.Tests/InterruptRequestHandlerTests.cs @@ -53,7 +53,7 @@ public async Task handles_InterruptRequest() public async Task sends_InterruptReply() { var kernel = new CSharpKernel(); - var handler = new IsCompleteRequestHandler(kernel); + var handler = new InterruptRequestHandler(kernel); var request = Message.Create(new InterruptRequest(), null); await handler.Handle(new JupyterRequestContext(_serverChannel, _ioPubChannel, request, _kernelStatus)); diff --git a/Microsoft.DotNet.Interactive.Jupyter/InterruptRequestHandler.cs b/Microsoft.DotNet.Interactive.Jupyter/InterruptRequestHandler.cs index 6183767df..61f287d98 100644 --- a/Microsoft.DotNet.Interactive.Jupyter/InterruptRequestHandler.cs +++ b/Microsoft.DotNet.Interactive.Jupyter/InterruptRequestHandler.cs @@ -36,11 +36,11 @@ private void OnExecutionInterrupted(ExecutionInterrupted executionInterrupted) var interruptReplyPayload = new InterruptReply(); // send to server - var executeReply = Message.CreateResponse( + var interruptReply = Message.CreateResponse( interruptReplyPayload, openRequest.Context.Request); - openRequest.Context.ServerChannel.Send(executeReply); + openRequest.Context.ServerChannel.Send(interruptReply); openRequest.Context.RequestHandlerStatus.SetAsIdle(); openRequest.Dispose(); } diff --git a/Microsoft.DotNet.Interactive.Jupyter/Protocol/InterruptReply.cs b/Microsoft.DotNet.Interactive.Jupyter/Protocol/InterruptReply.cs index 5765bbdd3..530d73223 100644 --- a/Microsoft.DotNet.Interactive.Jupyter/Protocol/InterruptReply.cs +++ b/Microsoft.DotNet.Interactive.Jupyter/Protocol/InterruptReply.cs @@ -3,7 +3,7 @@ namespace Microsoft.DotNet.Interactive.Jupyter.Protocol { - [JupyterMessageType(MessageTypeValues.InputReply)] + [JupyterMessageType(MessageTypeValues.InterruptReply)] public class InterruptReply : JupyterMessageContent { diff --git a/WorkspaceServer/Kernel/CSharpKernel.cs b/WorkspaceServer/Kernel/CSharpKernel.cs index 6297c5b1f..6e710e555 100644 --- a/WorkspaceServer/Kernel/CSharpKernel.cs +++ b/WorkspaceServer/Kernel/CSharpKernel.cs @@ -7,6 +7,7 @@ using System.Linq; using System.Reflection; using System.Text; +using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.Completion; @@ -41,6 +42,7 @@ public class CSharpKernel : KernelBase protected ScriptOptions ScriptOptions; private ImmutableArray _metadataReferences; private WorkspaceFixture _fixture; + private CancellationTokenSource _cancellationSource; public CSharpKernel() { @@ -109,8 +111,9 @@ private async Task HandleInterruptExecution( KernelInvocationContext context) { var reply = new ExecutionInterrupted(interruptExecution); + _cancellationSource?.Cancel(); + _cancellationSource = null; context.Publish(reply); - throw new NotImplementedException(); } @@ -143,6 +146,7 @@ private async Task HandleSubmitCode( Exception exception = null; + _cancellationSource = new CancellationTokenSource(); using var console = await ConsoleOutput.Capture(); using var _ = console.SubscribeToStandardOutput(std => PublishOutput(std, context, submitCode)); @@ -152,7 +156,8 @@ private async Task HandleSubmitCode( { _scriptState = await CSharpScript.RunAsync( code, - ScriptOptions); + ScriptOptions, + cancellationToken:_cancellationSource.Token); } else { @@ -163,7 +168,8 @@ private async Task HandleSubmitCode( { exception = e; return true; - }); + }, + cancellationToken: _cancellationSource.Token); } } catch (Exception e) @@ -171,6 +177,8 @@ private async Task HandleSubmitCode( exception = e; } + _cancellationSource = null; + if (exception != null) { string message = null; From 7ed2d2f909efd5cc83786268c6cb063931861063 Mon Sep 17 00:00:00 2001 From: Diego Date: Tue, 3 Sep 2019 14:55:22 +0100 Subject: [PATCH 12/20] is this the right thing to do??? --- .../FSharpKernel.fs | 9 ++++++++ .../Kernel/CSharpKernelTests.cs | 22 +++++++++++++++++++ WorkspaceServer/Kernel/CSharpKernel.cs | 13 ++++++++--- 3 files changed, 41 insertions(+), 3 deletions(-) diff --git a/Microsoft.DotNet.Interactive.FSharp/FSharpKernel.fs b/Microsoft.DotNet.Interactive.FSharp/FSharpKernel.fs index 429709bcc..5161de78e 100644 --- a/Microsoft.DotNet.Interactive.FSharp/FSharpKernel.fs +++ b/Microsoft.DotNet.Interactive.FSharp/FSharpKernel.fs @@ -35,10 +35,19 @@ type FSharpKernel() = context.Publish(CodeSubmissionEvaluated(codeSubmission)) context.Complete() } + + let handleInterruptExecution (interruptExecution: InterruptExecution) (context: KernelInvocationContext) = + async { + let reply = ExecutionInterrupted(interruptExecution) + context.Publish(reply) + context.Complete() + } + override __.HandleAsync(command: IKernelCommand, _context: KernelInvocationContext): Task = async { match command with | :? SubmitCode as submitCode -> submitCode.Handler <- fun invocationContext -> (handleSubmitCode submitCode invocationContext) |> Async.StartAsTask :> Task + | :? InterruptExecution as interruptExecution -> interruptExecution.Handler <- fun invocationContext -> (handleInterruptExecution interruptExecution invocationContext) |> Async.StartAsTask :> Task | _ -> () } |> Async.StartAsTask :> Task diff --git a/WorkspaceServer.Tests/Kernel/CSharpKernelTests.cs b/WorkspaceServer.Tests/Kernel/CSharpKernelTests.cs index d1173dd92..64530f8ae 100644 --- a/WorkspaceServer.Tests/Kernel/CSharpKernelTests.cs +++ b/WorkspaceServer.Tests/Kernel/CSharpKernelTests.cs @@ -202,6 +202,28 @@ public async Task it_produces_values_when_executing_Console_output() new DisplayedValueProduced("value three", kernelCommand, new[] { new FormattedValue("text/plain", "value three"), })); } + [Fact] + public async Task it_can_cancel_execution() + { + var kernel = CreateKernel(); + + var kernelCommand = new SubmitCode(@"while (true) { Console.Write(""value one""); }"); + var codeSubmission = kernel.SendAsync(kernelCommand); + var interruptionCommand = new InterruptExecution(); + await kernel.SendAsync(interruptionCommand); + await codeSubmission; + + KernelEvents + .ValuesOnly() + .Single(e => e is ExecutionInterrupted); + + KernelEvents + .ValuesOnly() + .OfType() + .Should() + .BeEquivalentTo(new CommandFailed(null, interruptionCommand, "Operation cancelled")); + } + [Fact] public async Task it_produces_a_final_value_if_the_code_expression_evaluates() { diff --git a/WorkspaceServer/Kernel/CSharpKernel.cs b/WorkspaceServer/Kernel/CSharpKernel.cs index 6e710e555..f8cb672c3 100644 --- a/WorkspaceServer/Kernel/CSharpKernel.cs +++ b/WorkspaceServer/Kernel/CSharpKernel.cs @@ -69,6 +69,8 @@ private void SetupScriptOptions() typeof(CSharpKernel).Assembly, typeof(PocketView).Assembly, typeof(XPlot.Plotly.PlotlyChart).Assembly); + + } protected override async Task HandleAsync( @@ -112,8 +114,8 @@ private async Task HandleInterruptExecution( { var reply = new ExecutionInterrupted(interruptExecution); _cancellationSource?.Cancel(); - _cancellationSource = null; context.Publish(reply); + context.Complete(); } @@ -146,7 +148,7 @@ private async Task HandleSubmitCode( Exception exception = null; - _cancellationSource = new CancellationTokenSource(); + var cancellationSource = _cancellationSource = new CancellationTokenSource(); using var console = await ConsoleOutput.Capture(); using var _ = console.SubscribeToStandardOutput(std => PublishOutput(std, context, submitCode)); @@ -177,7 +179,12 @@ private async Task HandleSubmitCode( exception = e; } - _cancellationSource = null; + + if (cancellationSource.Token.IsCancellationRequested) + { + context.Publish(new CommandFailed(null, submitCode, "Operation cancelled")); + context.Complete(); + } if (exception != null) { From 2effa6bf1f440503c64f44e95221537d24b7f98d Mon Sep 17 00:00:00 2001 From: Diego Date: Tue, 3 Sep 2019 16:03:41 +0100 Subject: [PATCH 13/20] wip --- WorkspaceServer/Kernel/CSharpKernel.cs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/WorkspaceServer/Kernel/CSharpKernel.cs b/WorkspaceServer/Kernel/CSharpKernel.cs index f8cb672c3..62b73fc1e 100644 --- a/WorkspaceServer/Kernel/CSharpKernel.cs +++ b/WorkspaceServer/Kernel/CSharpKernel.cs @@ -27,6 +27,14 @@ namespace WorkspaceServer.Kernel { + internal static class ScriptExecutionExtensions + { + public static Task UnlessCancelled(this Task execution) + { + + } + } + public class CSharpKernel : KernelBase { internal const string KernelName = "csharp"; @@ -118,8 +126,6 @@ private async Task HandleInterruptExecution( context.Complete(); } - - private async Task HandleSubmitCode( SubmitCode submitCode, KernelInvocationContext context) @@ -159,7 +165,7 @@ private async Task HandleSubmitCode( _scriptState = await CSharpScript.RunAsync( code, ScriptOptions, - cancellationToken:_cancellationSource.Token); + cancellationToken:cancellationSource.Token); } else { @@ -171,7 +177,7 @@ private async Task HandleSubmitCode( exception = e; return true; }, - cancellationToken: _cancellationSource.Token); + cancellationToken: cancellationSource.Token); } } catch (Exception e) From 47ba7511eba3b2306d2ba37b70f2d962547e211b Mon Sep 17 00:00:00 2001 From: Diego Date: Tue, 3 Sep 2019 20:03:28 +0100 Subject: [PATCH 14/20] WIP --- .../Kernel/CSharpKernelTests.cs | 4 +- WorkspaceServer/Kernel/CSharpKernel.cs | 99 ++++++++++++------- 2 files changed, 63 insertions(+), 40 deletions(-) diff --git a/WorkspaceServer.Tests/Kernel/CSharpKernelTests.cs b/WorkspaceServer.Tests/Kernel/CSharpKernelTests.cs index 64530f8ae..4c98cee4c 100644 --- a/WorkspaceServer.Tests/Kernel/CSharpKernelTests.cs +++ b/WorkspaceServer.Tests/Kernel/CSharpKernelTests.cs @@ -207,8 +207,8 @@ public async Task it_can_cancel_execution() { var kernel = CreateKernel(); - var kernelCommand = new SubmitCode(@"while (true) { Console.Write(""value one""); }"); - var codeSubmission = kernel.SendAsync(kernelCommand); + var submitCodeCommand = new SubmitCode(@"System.Threading.Thread.Sleep(90000000);"); + var codeSubmission = kernel.SendAsync(submitCodeCommand); var interruptionCommand = new InterruptExecution(); await kernel.SendAsync(interruptionCommand); await codeSubmission; diff --git a/WorkspaceServer/Kernel/CSharpKernel.cs b/WorkspaceServer/Kernel/CSharpKernel.cs index 62b73fc1e..36c80590e 100644 --- a/WorkspaceServer/Kernel/CSharpKernel.cs +++ b/WorkspaceServer/Kernel/CSharpKernel.cs @@ -29,8 +29,27 @@ namespace WorkspaceServer.Kernel { internal static class ScriptExecutionExtensions { - public static Task UnlessCancelled(this Task execution) + public static async Task> UnlessCancelled(this Task> source, + CancellationToken cancellationToken, + Action onCancelled) { + var completed = await Task.WhenAny( + source, + Task.Run(async () => + { + while (!cancellationToken.IsCancellationRequested) + { + await Task.Delay(100, cancellationToken); + } + return (ScriptState) null; + }, cancellationToken)); + + if (completed != source) + { + onCancelled(); + } + + return completed.Result; } } @@ -130,6 +149,7 @@ private async Task HandleSubmitCode( SubmitCode submitCode, KernelInvocationContext context) { + var cancellationSource = _cancellationSource = new CancellationTokenSource(); var codeSubmissionReceived = new CodeSubmissionReceived( submitCode.Code, submitCode); @@ -153,23 +173,25 @@ private async Task HandleSubmitCode( } Exception exception = null; - - var cancellationSource = _cancellationSource = new CancellationTokenSource(); using var console = await ConsoleOutput.Capture(); using var _ = console.SubscribeToStandardOutput(std => PublishOutput(std, context, submitCode)); - + var scriptState = _scriptState; try { - if (_scriptState == null) + if (scriptState == null) { - _scriptState = await CSharpScript.RunAsync( + scriptState = await CSharpScript.RunAsync( code, ScriptOptions, - cancellationToken:cancellationSource.Token); + cancellationToken:cancellationSource.Token) + .UnlessCancelled(cancellationSource.Token,() => + { + context.Publish(new CommandFailed(null, submitCode, "Operation cancelled")); + }); } else { - _scriptState = await _scriptState.ContinueWithAsync( + scriptState = await _scriptState.ContinueWithAsync( code, ScriptOptions, e => @@ -177,7 +199,11 @@ private async Task HandleSubmitCode( exception = e; return true; }, - cancellationToken: cancellationSource.Token); + cancellationToken: cancellationSource.Token) + .UnlessCancelled(cancellationSource.Token,() => + { + context.Publish(new CommandFailed(null, submitCode, "Operation cancelled")); + }); } } catch (Exception e) @@ -185,42 +211,39 @@ private async Task HandleSubmitCode( exception = e; } - - if (cancellationSource.Token.IsCancellationRequested) + if (!cancellationSource.Token.IsCancellationRequested) { - context.Publish(new CommandFailed(null, submitCode, "Operation cancelled")); - context.Complete(); - } + _scriptState = scriptState; + if (exception != null) + { + string message = null; - if (exception != null) - { - string message = null; + if (exception is CompilationErrorException compilationError) + { + message = + string.Join(Environment.NewLine, + compilationError.Diagnostics.Select(d => d.ToString())); + } - if (exception is CompilationErrorException compilationError) - { - message = - string.Join(Environment.NewLine, - compilationError.Diagnostics.Select(d => d.ToString())); + context.Publish(new CommandFailed(exception, submitCode, message)); } - - context.Publish(new CommandFailed(exception, submitCode, message)); - context.Complete(); - } - else - { - if (HasReturnValue) + else { - var formattedValues = FormattedValue.FromObject(_scriptState.ReturnValue); - context.Publish( - new ReturnValueProduced( - _scriptState.ReturnValue, - submitCode, - formattedValues)); + if (_scriptState != null && HasReturnValue) + { + var formattedValues = FormattedValue.FromObject(_scriptState.ReturnValue); + context.Publish( + new ReturnValueProduced( + _scriptState.ReturnValue, + submitCode, + formattedValues)); + } + + context.Publish(new CodeSubmissionEvaluated(submitCode)); } - - context.Publish(new CodeSubmissionEvaluated(submitCode)); - context.Complete(); } + + context.Complete(); } private void PublishOutput( From 16c1788ea88a973490bbe2492f35a7b53fee1cc5 Mon Sep 17 00:00:00 2001 From: Diego Date: Tue, 3 Sep 2019 20:19:17 +0100 Subject: [PATCH 15/20] add locks --- WorkspaceServer/Kernel/CSharpKernel.cs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/WorkspaceServer/Kernel/CSharpKernel.cs b/WorkspaceServer/Kernel/CSharpKernel.cs index 36c80590e..fc5754ce0 100644 --- a/WorkspaceServer/Kernel/CSharpKernel.cs +++ b/WorkspaceServer/Kernel/CSharpKernel.cs @@ -70,9 +70,11 @@ public class CSharpKernel : KernelBase private ImmutableArray _metadataReferences; private WorkspaceFixture _fixture; private CancellationTokenSource _cancellationSource; + private readonly object _cancellationSourceLock = new object(); public CSharpKernel() { + _cancellationSource = new CancellationTokenSource(); _metadataReferences = ImmutableArray.Empty; SetupScriptOptions(); Name = KernelName; @@ -140,7 +142,12 @@ private async Task HandleInterruptExecution( KernelInvocationContext context) { var reply = new ExecutionInterrupted(interruptExecution); - _cancellationSource?.Cancel(); + lock (_cancellationSourceLock) + { + _cancellationSource.Cancel(); + _cancellationSource = new CancellationTokenSource(); + } + context.Publish(reply); context.Complete(); } @@ -149,7 +156,11 @@ private async Task HandleSubmitCode( SubmitCode submitCode, KernelInvocationContext context) { - var cancellationSource = _cancellationSource = new CancellationTokenSource(); + CancellationTokenSource cancellationSource; + lock (_cancellationSourceLock) + { + cancellationSource = _cancellationSource; + } var codeSubmissionReceived = new CodeSubmissionReceived( submitCode.Code, submitCode); From d2d8562e14307c4830ee1432b59163fd180a62c1 Mon Sep 17 00:00:00 2001 From: Diego Date: Tue, 3 Sep 2019 20:24:09 +0100 Subject: [PATCH 16/20] fixed event publishing order --- WorkspaceServer/Kernel/CSharpKernel.cs | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/WorkspaceServer/Kernel/CSharpKernel.cs b/WorkspaceServer/Kernel/CSharpKernel.cs index fc5754ce0..01aef54ad 100644 --- a/WorkspaceServer/Kernel/CSharpKernel.cs +++ b/WorkspaceServer/Kernel/CSharpKernel.cs @@ -29,9 +29,9 @@ namespace WorkspaceServer.Kernel { internal static class ScriptExecutionExtensions { - public static async Task> UnlessCancelled(this Task> source, - CancellationToken cancellationToken, - Action onCancelled) + public static async Task> UnlessCancelled( + this Task> source, + CancellationToken cancellationToken) { var completed = await Task.WhenAny( source, @@ -44,11 +44,7 @@ public static async Task> UnlessCancelled(this Task) null; }, cancellationToken)); - if (completed != source) - { - onCancelled(); - } - + return completed.Result; } @@ -195,10 +191,7 @@ private async Task HandleSubmitCode( code, ScriptOptions, cancellationToken:cancellationSource.Token) - .UnlessCancelled(cancellationSource.Token,() => - { - context.Publish(new CommandFailed(null, submitCode, "Operation cancelled")); - }); + .UnlessCancelled(cancellationSource.Token); } else { @@ -211,10 +204,7 @@ private async Task HandleSubmitCode( return true; }, cancellationToken: cancellationSource.Token) - .UnlessCancelled(cancellationSource.Token,() => - { - context.Publish(new CommandFailed(null, submitCode, "Operation cancelled")); - }); + .UnlessCancelled(cancellationSource.Token); } } catch (Exception e) @@ -253,6 +243,10 @@ private async Task HandleSubmitCode( context.Publish(new CodeSubmissionEvaluated(submitCode)); } } + else + { + context.Publish(new CommandFailed(null, submitCode, "Operation cancelled")); + } context.Complete(); } From 75575507c02b02bc533b3dff7ea431bdb3e84de9 Mon Sep 17 00:00:00 2001 From: Diego Date: Tue, 3 Sep 2019 21:03:26 +0100 Subject: [PATCH 17/20] separate files --- WorkspaceServer/Kernel/CSharpKernel.cs | 23 ------------- .../Kernel/ScriptExecutionExtensions.cs | 32 +++++++++++++++++++ 2 files changed, 32 insertions(+), 23 deletions(-) create mode 100644 WorkspaceServer/Kernel/ScriptExecutionExtensions.cs diff --git a/WorkspaceServer/Kernel/CSharpKernel.cs b/WorkspaceServer/Kernel/CSharpKernel.cs index 01aef54ad..5b032c9f6 100644 --- a/WorkspaceServer/Kernel/CSharpKernel.cs +++ b/WorkspaceServer/Kernel/CSharpKernel.cs @@ -27,29 +27,6 @@ namespace WorkspaceServer.Kernel { - internal static class ScriptExecutionExtensions - { - public static async Task> UnlessCancelled( - this Task> source, - CancellationToken cancellationToken) - { - var completed = await Task.WhenAny( - source, - Task.Run(async () => - { - while (!cancellationToken.IsCancellationRequested) - { - await Task.Delay(100, cancellationToken); - } - return (ScriptState) null; - }, cancellationToken)); - - - return completed.Result; - - } - } - public class CSharpKernel : KernelBase { internal const string KernelName = "csharp"; diff --git a/WorkspaceServer/Kernel/ScriptExecutionExtensions.cs b/WorkspaceServer/Kernel/ScriptExecutionExtensions.cs new file mode 100644 index 000000000..fa3e9a164 --- /dev/null +++ b/WorkspaceServer/Kernel/ScriptExecutionExtensions.cs @@ -0,0 +1,32 @@ +// 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; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis.Scripting; + +namespace WorkspaceServer.Kernel +{ + internal static class ScriptExecutionExtensions + { + public static async Task> UnlessCancelled( + this Task> source, + CancellationToken cancellationToken) + { + var completed = await Task.WhenAny( + source, + Task.Run(async () => + { + while (!cancellationToken.IsCancellationRequested) + { + await Task.Delay(100, cancellationToken); + } + return (ScriptState) null; + }, cancellationToken)); + + + return completed.Result; + + } + } +} \ No newline at end of file From 42c965dd99ea2b17fcb920fa2e1cd1aee279e828 Mon Sep 17 00:00:00 2001 From: Diego Date: Tue, 3 Sep 2019 21:44:20 +0100 Subject: [PATCH 18/20] code formatted --- WorkspaceServer/Kernel/CSharpKernel.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/WorkspaceServer/Kernel/CSharpKernel.cs b/WorkspaceServer/Kernel/CSharpKernel.cs index 5b032c9f6..48fd94bb2 100644 --- a/WorkspaceServer/Kernel/CSharpKernel.cs +++ b/WorkspaceServer/Kernel/CSharpKernel.cs @@ -72,7 +72,7 @@ private void SetupScriptOptions() typeof(PocketView).Assembly, typeof(XPlot.Plotly.PlotlyChart).Assembly); - + } protected override async Task HandleAsync( @@ -125,9 +125,9 @@ private async Task HandleInterruptExecution( context.Complete(); } - private async Task HandleSubmitCode( - SubmitCode submitCode, - KernelInvocationContext context) + private async Task HandleSubmitCode( + SubmitCode submitCode, + KernelInvocationContext context) { CancellationTokenSource cancellationSource; lock (_cancellationSourceLock) @@ -142,7 +142,7 @@ private async Task HandleSubmitCode( var code = submitCode.Code; var isComplete = await IsCompleteSubmissionAsync(submitCode.Code); - if(isComplete) + if (isComplete) { context.Publish(new CompleteCodeSubmissionReceived(submitCode)); } @@ -167,7 +167,7 @@ private async Task HandleSubmitCode( scriptState = await CSharpScript.RunAsync( code, ScriptOptions, - cancellationToken:cancellationSource.Token) + cancellationToken: cancellationSource.Token) .UnlessCancelled(cancellationSource.Token); } else From 1a3a1cce040fab9a001b939f5b5f7add744126eb Mon Sep 17 00:00:00 2001 From: Diego Date: Tue, 3 Sep 2019 22:01:05 +0100 Subject: [PATCH 19/20] rename command and event for current command cancellation --- .../FSharpKernel.fs | 6 +- .../InterruptRequestHandler.cs | 8 +-- ...ptExecution.cs => CancelCurrentCommand.cs} | 2 +- ...errupted.cs => CurrentCommandCancelled.cs} | 4 +- .../Kernel/CSharpKernelTests.cs | 6 +- WorkspaceServer/Kernel/CSharpKernel.cs | 62 ++++++++++--------- .../Kernel/CSharpKernelExtensions.cs | 2 +- .../Kernel/ScriptExecutionExtensions.cs | 2 +- 8 files changed, 48 insertions(+), 44 deletions(-) rename Microsoft.DotNet.Interactive/Commands/{InterruptExecution.cs => CancelCurrentCommand.cs} (80%) rename Microsoft.DotNet.Interactive/Events/{ExecutionInterrupted.cs => CurrentCommandCancelled.cs} (70%) diff --git a/Microsoft.DotNet.Interactive.FSharp/FSharpKernel.fs b/Microsoft.DotNet.Interactive.FSharp/FSharpKernel.fs index 5161de78e..9aabe49c4 100644 --- a/Microsoft.DotNet.Interactive.FSharp/FSharpKernel.fs +++ b/Microsoft.DotNet.Interactive.FSharp/FSharpKernel.fs @@ -36,9 +36,9 @@ type FSharpKernel() = context.Complete() } - let handleInterruptExecution (interruptExecution: InterruptExecution) (context: KernelInvocationContext) = + let handleCancelCurrentCommand (cancelCurrentCommand: CancelCurrentCommand) (context: KernelInvocationContext) = async { - let reply = ExecutionInterrupted(interruptExecution) + let reply = CurrentCommandCancelled(cancelCurrentCommand) context.Publish(reply) context.Complete() } @@ -47,7 +47,7 @@ type FSharpKernel() = async { match command with | :? SubmitCode as submitCode -> submitCode.Handler <- fun invocationContext -> (handleSubmitCode submitCode invocationContext) |> Async.StartAsTask :> Task - | :? InterruptExecution as interruptExecution -> interruptExecution.Handler <- fun invocationContext -> (handleInterruptExecution interruptExecution invocationContext) |> Async.StartAsTask :> Task + | :? CancelCurrentCommand as cancelCurrentCommand -> cancelCurrentCommand.Handler <- fun invocationContext -> (handleCancelCurrentCommand cancelCurrentCommand invocationContext) |> Async.StartAsTask :> Task | _ -> () } |> Async.StartAsTask :> Task diff --git a/Microsoft.DotNet.Interactive.Jupyter/InterruptRequestHandler.cs b/Microsoft.DotNet.Interactive.Jupyter/InterruptRequestHandler.cs index 61f287d98..cfa710fdb 100644 --- a/Microsoft.DotNet.Interactive.Jupyter/InterruptRequestHandler.cs +++ b/Microsoft.DotNet.Interactive.Jupyter/InterruptRequestHandler.cs @@ -22,15 +22,15 @@ protected override void OnKernelEvent(IKernelEvent @event) { switch (@event) { - case ExecutionInterrupted kernelInterrupted: + case CurrentCommandCancelled kernelInterrupted: OnExecutionInterrupted(kernelInterrupted); break; } } - private void OnExecutionInterrupted(ExecutionInterrupted executionInterrupted) + private void OnExecutionInterrupted(CurrentCommandCancelled currentCommandCancelled) { - if (InFlightRequests.TryRemove(executionInterrupted.Command, out var openRequest)) + if (InFlightRequests.TryRemove(currentCommandCancelled.Command, out var openRequest)) { // reply var interruptReplyPayload = new InterruptReply(); @@ -52,7 +52,7 @@ public async Task Handle(JupyterRequestContext context) context.RequestHandlerStatus.SetAsBusy(); - var command = new InterruptExecution(); + var command = new CancelCurrentCommand(); var openRequest = new InflightRequest(context, interruptRequest, 0); diff --git a/Microsoft.DotNet.Interactive/Commands/InterruptExecution.cs b/Microsoft.DotNet.Interactive/Commands/CancelCurrentCommand.cs similarity index 80% rename from Microsoft.DotNet.Interactive/Commands/InterruptExecution.cs rename to Microsoft.DotNet.Interactive/Commands/CancelCurrentCommand.cs index a37a4a53f..92b17d300 100644 --- a/Microsoft.DotNet.Interactive/Commands/InterruptExecution.cs +++ b/Microsoft.DotNet.Interactive/Commands/CancelCurrentCommand.cs @@ -3,7 +3,7 @@ namespace Microsoft.DotNet.Interactive.Commands { - public class InterruptExecution : KernelCommandBase + public class CancelCurrentCommand : KernelCommandBase { } diff --git a/Microsoft.DotNet.Interactive/Events/ExecutionInterrupted.cs b/Microsoft.DotNet.Interactive/Events/CurrentCommandCancelled.cs similarity index 70% rename from Microsoft.DotNet.Interactive/Events/ExecutionInterrupted.cs rename to Microsoft.DotNet.Interactive/Events/CurrentCommandCancelled.cs index 12e50a78a..8dad375e2 100644 --- a/Microsoft.DotNet.Interactive/Events/ExecutionInterrupted.cs +++ b/Microsoft.DotNet.Interactive/Events/CurrentCommandCancelled.cs @@ -5,9 +5,9 @@ namespace Microsoft.DotNet.Interactive.Events { - public class ExecutionInterrupted:KernelEventBase + public class CurrentCommandCancelled:KernelEventBase { - public ExecutionInterrupted(IKernelCommand command) : base(command) + public CurrentCommandCancelled(IKernelCommand command) : base(command) { } diff --git a/WorkspaceServer.Tests/Kernel/CSharpKernelTests.cs b/WorkspaceServer.Tests/Kernel/CSharpKernelTests.cs index 3b2f5f9fb..9e3529061 100644 --- a/WorkspaceServer.Tests/Kernel/CSharpKernelTests.cs +++ b/WorkspaceServer.Tests/Kernel/CSharpKernelTests.cs @@ -210,19 +210,19 @@ public async Task it_can_cancel_execution() var submitCodeCommand = new SubmitCode(@"System.Threading.Thread.Sleep(90000000);"); var codeSubmission = kernel.SendAsync(submitCodeCommand); - var interruptionCommand = new InterruptExecution(); + var interruptionCommand = new CancelCurrentCommand(); await kernel.SendAsync(interruptionCommand); await codeSubmission; KernelEvents .ValuesOnly() - .Single(e => e is ExecutionInterrupted); + .Single(e => e is CurrentCommandCancelled); KernelEvents .ValuesOnly() .OfType() .Should() - .BeEquivalentTo(new CommandFailed(null, interruptionCommand, "Operation cancelled")); + .BeEquivalentTo(new CommandFailed(null, interruptionCommand, "Command cancelled")); } [Fact] diff --git a/WorkspaceServer/Kernel/CSharpKernel.cs b/WorkspaceServer/Kernel/CSharpKernel.cs index 48fd94bb2..ee08a6dce 100644 --- a/WorkspaceServer/Kernel/CSharpKernel.cs +++ b/WorkspaceServer/Kernel/CSharpKernel.cs @@ -95,10 +95,10 @@ protected override async Task HandleAsync( }; break; - case InterruptExecution interruptExecution: + case CancelCurrentCommand interruptExecution: interruptExecution.Handler = async invocationContext => { - await HandleInterruptExecution(interruptExecution, invocationContext); + await HandleCancelCurrentCommand(interruptExecution, invocationContext); }; break; } @@ -110,11 +110,11 @@ public Task IsCompleteSubmissionAsync(string code) return Task.FromResult(SyntaxFactory.IsCompleteSubmission(syntaxTree)); } - private async Task HandleInterruptExecution( - InterruptExecution interruptExecution, + private async Task HandleCancelCurrentCommand( + CancelCurrentCommand cancelCurrentCommand, KernelInvocationContext context) { - var reply = new ExecutionInterrupted(interruptExecution); + var reply = new CurrentCommandCancelled(cancelCurrentCommand); lock (_cancellationSourceLock) { _cancellationSource.Cancel(); @@ -160,34 +160,38 @@ private async Task HandleSubmitCode( using var console = await ConsoleOutput.Capture(); using var _ = console.SubscribeToStandardOutput(std => PublishOutput(std, context, submitCode)); var scriptState = _scriptState; - try + + if (!cancellationSource.IsCancellationRequested) { - if (scriptState == null) + try { - scriptState = await CSharpScript.RunAsync( - code, - ScriptOptions, - cancellationToken: cancellationSource.Token) - .UnlessCancelled(cancellationSource.Token); + if (scriptState == null) + { + scriptState = await CSharpScript.RunAsync( + code, + ScriptOptions, + cancellationToken: cancellationSource.Token) + .UntilCancelled(cancellationSource.Token); + } + else + { + scriptState = await _scriptState.ContinueWithAsync( + code, + ScriptOptions, + e => + { + exception = e; + return true; + }, + cancellationToken: cancellationSource.Token) + .UntilCancelled(cancellationSource.Token); + } } - else + catch (Exception e) { - scriptState = await _scriptState.ContinueWithAsync( - code, - ScriptOptions, - e => - { - exception = e; - return true; - }, - cancellationToken: cancellationSource.Token) - .UnlessCancelled(cancellationSource.Token); + exception = e; } } - catch (Exception e) - { - exception = e; - } if (!cancellationSource.Token.IsCancellationRequested) { @@ -222,7 +226,7 @@ private async Task HandleSubmitCode( } else { - context.Publish(new CommandFailed(null, submitCode, "Operation cancelled")); + context.Publish(new CommandFailed(null, submitCode, "Command cancelled")); } context.Complete(); @@ -261,7 +265,7 @@ private async Task HandleRequestCompletion( context.Publish(new CompletionRequestCompleted(completionList, requestCompletion)); } - public void AddMetatadaReferences(IEnumerable references) + public void AddMetadataReferences(IEnumerable references) { _metadataReferences.AddRange(references); ScriptOptions = ScriptOptions.AddReferences(references); diff --git a/WorkspaceServer/Kernel/CSharpKernelExtensions.cs b/WorkspaceServer/Kernel/CSharpKernelExtensions.cs index 2090fbaeb..72bc8a7e8 100644 --- a/WorkspaceServer/Kernel/CSharpKernelExtensions.cs +++ b/WorkspaceServer/Kernel/CSharpKernelExtensions.cs @@ -80,7 +80,7 @@ public static CSharpKernel UseNugetDirective(this CSharpKernel kernel, INativeAs } } - kernel.AddMetatadaReferences(refs); + kernel.AddMetadataReferences(refs); } context.Publish(new NuGetPackageAdded(package)); diff --git a/WorkspaceServer/Kernel/ScriptExecutionExtensions.cs b/WorkspaceServer/Kernel/ScriptExecutionExtensions.cs index fa3e9a164..3e8c45a2f 100644 --- a/WorkspaceServer/Kernel/ScriptExecutionExtensions.cs +++ b/WorkspaceServer/Kernel/ScriptExecutionExtensions.cs @@ -9,7 +9,7 @@ namespace WorkspaceServer.Kernel { internal static class ScriptExecutionExtensions { - public static async Task> UnlessCancelled( + public static async Task> UntilCancelled( this Task> source, CancellationToken cancellationToken) { From 3ab8f75ad62bfa5768ac0b393c9c44e2b52ec7dc Mon Sep 17 00:00:00 2001 From: Diego Date: Tue, 3 Sep 2019 23:32:24 +0100 Subject: [PATCH 20/20] unify cancellation check on source --- WorkspaceServer/Kernel/CSharpKernel.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/WorkspaceServer/Kernel/CSharpKernel.cs b/WorkspaceServer/Kernel/CSharpKernel.cs index ee08a6dce..a2657dca1 100644 --- a/WorkspaceServer/Kernel/CSharpKernel.cs +++ b/WorkspaceServer/Kernel/CSharpKernel.cs @@ -193,7 +193,7 @@ private async Task HandleSubmitCode( } } - if (!cancellationSource.Token.IsCancellationRequested) + if (!cancellationSource.IsCancellationRequested) { _scriptState = scriptState; if (exception != null)