diff --git a/src/InteractiveWindow/Editor/InteractiveWindow.cs b/src/InteractiveWindow/Editor/InteractiveWindow.cs index b8022440cc1028644aecf0dbcb8ad9badc0bfce6..21c359a9a642f382139bcd20f0447319686566ce 100644 --- a/src/InteractiveWindow/Editor/InteractiveWindow.cs +++ b/src/InteractiveWindow/Editor/InteractiveWindow.cs @@ -19,6 +19,7 @@ using Microsoft.VisualStudio.Text.Editor.OptionsExtensionMethods; using Microsoft.VisualStudio.Text.Projection; using Microsoft.VisualStudio.Utilities; +using Roslyn.Utilities; namespace Microsoft.VisualStudio.InteractiveWindow { @@ -93,10 +94,7 @@ private enum ReplSpanKind //// Standard input. //// - // non-null if reading from stdin - position in the _inputBuffer where we map stdin - private int? _stdInputStart; // TODO (https://github.com/dotnet/roslyn/issues/4044): this variable is not used in thread-safe manner - private SnapshotSpan? _inputValue; - private readonly AutoResetEvent _inputEvent = new AutoResetEvent(false); + private readonly SemaphoreSlim _inputReaderSemaphore = new SemaphoreSlim(initialCount: 1, maxCount: 1); //// //// Output. @@ -245,7 +243,7 @@ Task IInteractiveWindowOperations.ResetAsync(bool initialize) void IInteractiveWindowOperations.ClearHistory() { - UIThread(uiOnly => uiOnly.History.Clear()); + UIThread(uiOnly => uiOnly.ClearHistory()); } void IInteractiveWindowOperations.ClearView() @@ -383,7 +381,7 @@ private void IndentCurrentLine(SnapshotPoint caretPosition) return GetPositionInBuffer(point, _currentLanguageBuffer); } - private SnapshotPoint? GetPositionInStdInputBuffer(SnapshotPoint point) + private SnapshotPoint? GetPositionInStandardInputBuffer(SnapshotPoint point) { Debug.Assert(_standardInputBuffer != null); return GetPositionInBuffer(point, _standardInputBuffer); @@ -436,7 +434,7 @@ private static string GetWhiteSpaceForVirtualSpace(int virtualSpaces, int? tabSi bool IInteractiveWindow.IsRunning => _dangerous_uiOnly.State != State.WaitingForInput; /// Only consistent on the UI thread. - bool IInteractiveWindow.IsResetting => _dangerous_uiOnly.State == State.Resetting; + bool IInteractiveWindow.IsResetting => _dangerous_uiOnly.State == State.Resetting || _dangerous_uiOnly.State == State.ResettingAndReadingStandardInput; /// Only consistent on the UI thread. bool IInteractiveWindow.IsInitializing => _dangerous_uiOnly.State == State.Starting || _dangerous_uiOnly.State == State.Initializing; @@ -571,85 +569,32 @@ private void CaretPositionChanged(object sender, CaretPositionChangedEventArgs e #region Active Code and Standard Input - // TODO: What happens if multiple non-UI threads call this method? (https://github.com/dotnet/roslyn/issues/3984) TextReader IInteractiveWindow.ReadStandardInput() { // shouldn't be called on the UI thread because we'll hang RequiresNonUIThread(); + return ReadStandardInputAsync().GetAwaiter().GetResult(); + } - State previousState = default(State); // Compiler doesn't know these lambdas run sequentially. - UIThread(uiOnly => + private async Task ReadStandardInputAsync() + { + try { - previousState = uiOnly.State; - - uiOnly.State = State.ReadingStandardInput; - - _buffer.Flush(); - - if (previousState == State.WaitingForInput) + // True because this is a public API and we want to use the same + // thread as the caller (esp for blocking). + await _inputReaderSemaphore.WaitAsync().ConfigureAwait(true); // Only one thread can read from standard input at a time. + try { - var snapshot = _projectionBuffer.CurrentSnapshot; - var spanCount = snapshot.SpanCount; - if (spanCount > 0 && GetSpanKind(snapshot.GetSourceSpan(spanCount - 1)) == ReplSpanKind.Language) - { - // we need to remove our input prompt. - uiOnly.RemoveLastInputPrompt(); - } + return await UIThread(uiOnly => uiOnly.ReadStandardInputAsync()).ConfigureAwait(true); } - - uiOnly.AddStandardInputSpan(); - - Caret.EnsureVisible(); - uiOnly.ResetCursor(); - - uiOnly.UncommittedInput = null; - _stdInputStart = _standardInputBuffer.CurrentSnapshot.Length; - }); - - _inputEvent.WaitOne(); - _stdInputStart = null; - - UIThread(uiOnly => - { - var sourceSpans = _projectionBuffer.CurrentSnapshot.GetSourceSpans(); - // if the user cleared the screen we cancelled the input, so we won't have our span here. - // We can also have an interleaving output span, so we'll search back for the last input span. - int i = uiOnly.IndexOfLastStandardInputSpan(sourceSpans); - if (i != -1) + finally { - uiOnly.RemoveProtection(_standardInputBuffer, uiOnly.StandardInputProtection); - - // replace previous span w/ a span that won't grow... - var oldSpan = sourceSpans[i]; - var newSpan = new CustomTrackingSpan(oldSpan.Snapshot, oldSpan.Span, PointTrackingMode.Negative, PointTrackingMode.Negative); - - uiOnly.ReplaceProjectionSpan(i, newSpan); - uiOnly.ApplyProtection(_standardInputBuffer, uiOnly.StandardInputProtection, allowAppend: true); - - uiOnly.NewOutputBuffer(); - - // TODO: Do we need to restore the state if reading is cancelled? (https://github.com/dotnet/roslyn/issues/3984) - if (previousState == State.WaitingForInput) - { - uiOnly.PrepareForInput(); // Will update _uiOnly.State. - } - else - { - uiOnly.State = previousState; - } + _inputReaderSemaphore.Release(); } - }); - - // input has been cancelled: - if (_inputValue.HasValue) - { - var value = _inputValue.GetValueOrDefault(); - UIThread(uiOnly => uiOnly.History.Add(value)); - return new StringReader(value.GetText()); } - else + catch (Exception e) when (ReportAndPropagateException(e)) { - return null; + throw ExceptionUtilities.Unreachable; } } diff --git a/src/InteractiveWindow/Editor/InteractiveWindowResources.Designer.cs b/src/InteractiveWindow/Editor/InteractiveWindowResources.Designer.cs index 6413e5261b468eb499b5476c0a2d755af6c5d501..6f7f0ab11991a88f1aff62f5e5f7dc80d1618d9f 100644 --- a/src/InteractiveWindow/Editor/InteractiveWindowResources.Designer.cs +++ b/src/InteractiveWindow/Editor/InteractiveWindowResources.Designer.cs @@ -96,6 +96,15 @@ internal class InteractiveWindowResources { } } + /// + /// Looks up a localized string similar to The interactive window has not yet been initialized.. + /// + internal static string NotInitialized { + get { + return ResourceManager.GetString("NotInitialized", resourceCulture); + } + } + /// /// Looks up a localized string similar to This method may not be called on the UI thread (to avoid hangs).. /// diff --git a/src/InteractiveWindow/Editor/InteractiveWindowResources.resx b/src/InteractiveWindow/Editor/InteractiveWindowResources.resx index 200a303bbbfdf68875bf20dab457e205d70ba13a..7e51814c567c900b0e7971314bc3b37998684945 100644 --- a/src/InteractiveWindow/Editor/InteractiveWindowResources.resx +++ b/src/InteractiveWindow/Editor/InteractiveWindowResources.resx @@ -129,6 +129,9 @@ The command of type '{0}' has no command names. + + The interactive window has not yet been initialized. + This method may not be called on the UI thread (to avoid hangs). diff --git a/src/InteractiveWindow/Editor/InteractiveWindow_UIThread.cs b/src/InteractiveWindow/Editor/InteractiveWindow_UIThread.cs index 11f669ab74ab7c8c09868a0456612e0e5c68be4d..79d781c78aa239c3fbbfecf7d2384e7a7894a4b2 100644 --- a/src/InteractiveWindow/Editor/InteractiveWindow_UIThread.cs +++ b/src/InteractiveWindow/Editor/InteractiveWindow_UIThread.cs @@ -4,8 +4,10 @@ using System.Collections.Generic; using System.Collections.ObjectModel; using System.Diagnostics; +using System.IO; using System.Linq; using System.Text; +using System.Threading; using System.Threading.Tasks; using System.Windows; using System.Windows.Controls; @@ -155,27 +157,34 @@ private sealed class UIThreadOnly { private readonly InteractiveWindow _window; - private readonly IInteractiveWindowEditorFactoryService _host; + private readonly IInteractiveWindowEditorFactoryService _host; // TODO (acasey): _factory - private readonly IReadOnlyRegion[] _outputProtection; - - public readonly History History; + private readonly History History = new History(); // TODO (acasey): _history private string _historySearch; // Pending submissions to be processed whenever the REPL is ready to accept submissions. - private readonly Queue _pendingSubmissions; + private readonly Queue _pendingSubmissions = new Queue(); private DispatcherTimer _executionTimer; private Cursor _oldCursor; private int _currentOutputProjectionSpan; - private int _outputTrackingCaretPosition; + private int _outputTrackingCaretPosition = -1; private readonly IRtfBuilderService _rtfBuilderService; // Read-only regions protecting initial span of the corresponding buffers: - public readonly IReadOnlyRegion[] StandardInputProtection; + private readonly IReadOnlyRegion[] _standardInputProtection = new IReadOnlyRegion[2]; + private readonly IReadOnlyRegion[] _outputProtection = new IReadOnlyRegion[2]; + + private string UncommittedInput; // TODO (acasey): _uncommittedInput + + /// Always access through and . + private SnapshotSpan? _standardInputValue; + /// Don't reference directly. + private readonly SemaphoreSlim _standardInputValueGuard = new SemaphoreSlim(initialCount: 0, maxCount: 1); - public string UncommittedInput; + // State captured when we started reading standard input. + private int _standardInputStart = -1; private IEditorOperations _editorOperations; public IEditorOperations EditorOperations @@ -207,31 +216,32 @@ public State State } } - public UIThreadOnly(InteractiveWindow window, IInteractiveWindowEditorFactoryService host, IRtfBuilderService rtfBuilderService) + public UIThreadOnly(InteractiveWindow window, IInteractiveWindowEditorFactoryService factory, IRtfBuilderService rtfBuilderService) { _window = window; - _host = host; + _host = factory; _rtfBuilderService = rtfBuilderService; - History = new History(); - StandardInputProtection = new IReadOnlyRegion[2]; - _outputProtection = new IReadOnlyRegion[2]; - _pendingSubmissions = new Queue(); - _outputTrackingCaretPosition = -1; } + private bool ReadingStandardInput => + State == State.ExecutingInputAndReadingStandardInput || + State == State.WaitingForInputAndReadingStandardInput || + State == State.ResettingAndReadingStandardInput; + public async Task ResetAsync(bool initialize) { try { - Debug.Assert(State != State.Resetting, "The button should have been disabled."); - - if (_window._stdInputStart != null) + if (ReadingStandardInput) { + MakeStandardInputReadonly(); CancelStandardInput(); } _window._buffer.Flush(); + // Nothing to clear in WaitingForInputAndReadingStandardInput, since we cleared + // the language prompt when we entered that state. if (State == State.WaitingForInput) { var snapshot = _window._projectionBuffer.CurrentSnapshot; @@ -257,11 +267,18 @@ public async Task ResetAsync(bool initialize) } } + public void ClearHistory() + { + History.Clear(); + } + public void ClearView() { - if (_window._stdInputStart != null) + if (ReadingStandardInput) { CancelStandardInput(); + State = GetStateBeforeReadingStandardInput(State); + Debug.Assert(State == State.ExecutingInput || State == State.WaitingForInput); } _window._adornmentToMinimize = false; @@ -272,7 +289,7 @@ public void ClearView() // Clear the projection and buffers last as this might trigger events that might access other state of the REPL window: RemoveProtection(_window._outputBuffer, _outputProtection); - RemoveProtection(_window._standardInputBuffer, StandardInputProtection); + RemoveProtection(_window._standardInputBuffer, _standardInputProtection); using (var edit = _window._outputBuffer.CreateEdit(EditOptions.None, null, s_suppressPromptInjectionTag)) { @@ -309,11 +326,134 @@ public void ClearView() } } + private static State GetStateBeforeReadingStandardInput(State state) + { + switch (state) + { + case State.WaitingForInputAndReadingStandardInput: + return State.WaitingForInput; + case State.ExecutingInputAndReadingStandardInput: + return State.ExecutingInput; + case State.ResettingAndReadingStandardInput: + return State.Resetting; + default: + throw ExceptionUtilities.UnexpectedValue(state); + } + } + + public async Task ReadStandardInputAsync() + { + try + { + switch (State) + { + case State.Starting: + case State.Initializing: + throw new InvalidOperationException(InteractiveWindowResources.NotInitialized); + + case State.WaitingForInputAndReadingStandardInput: + case State.ExecutingInputAndReadingStandardInput: + case State.ResettingAndReadingStandardInput: + // Guarded by semaphore. + throw ExceptionUtilities.UnexpectedValue(State); + + case State.WaitingForInput: + State = State.WaitingForInputAndReadingStandardInput; + break; + case State.ExecutingInput: + State = State.ExecutingInputAndReadingStandardInput; + break; + case State.Resetting: + State = State.ResettingAndReadingStandardInput; + break; + + default: + throw ExceptionUtilities.UnexpectedValue(State); + } + + Debug.Assert(ReadingStandardInput); + + _window._buffer.Flush(); + + if (State == State.WaitingForInputAndReadingStandardInput) + { + var snapshot = _window._projectionBuffer.CurrentSnapshot; + var spanCount = snapshot.SpanCount; + if (spanCount > 0 && _window.GetSpanKind(snapshot.GetSourceSpan(spanCount - 1)) == ReplSpanKind.Language) + { + // we need to remove our input prompt. + RemoveLastInputPrompt(); + } + } + + AddStandardInputSpan(); + + _window.Caret.EnsureVisible(); + ResetCursor(); + + UncommittedInput = null; + _standardInputStart = _window._standardInputBuffer.CurrentSnapshot.Length; + + var value = await GetStandardInputValue().ConfigureAwait(true); + Debug.Assert(_window.OnUIThread()); // ConfigureAwait should bring us back to the UI thread. + return value.HasValue + ? new StringReader(value.GetValueOrDefault().GetText()) + : null; + } + catch (Exception e) when (_window.ReportAndPropagateException(e)) + { + throw ExceptionUtilities.Unreachable; + } + } + private void CancelStandardInput() + { + SetStandardInputValue(null); + } + + private void SetStandardInputValue(SnapshotSpan? value) + { + _standardInputValue = value; + _standardInputValueGuard.Release(); + } + + private async Task GetStandardInputValue() + { + try + { + await _standardInputValueGuard.WaitAsync().ConfigureAwait(true); + Debug.Assert(_window.OnUIThread()); // ConfigureAwait should bring us back to the UI thread. + return _standardInputValue; + } + catch (Exception e) when (_window.ReportAndPropagateException(e)) + { + throw ExceptionUtilities.Unreachable; + } + } + + private Span GetStandardInputSpan() + { + return Span.FromBounds(_standardInputStart, _window._standardInputBuffer.CurrentSnapshot.Length); + } + + private void MakeStandardInputReadonly() { AppendLineNoPromptInjection(_window._standardInputBuffer); - _window._inputValue = null; - _window._inputEvent.Set(); + + // We can also have an interleaving output span, so we'll search back for the last input span. + var sourceSpans = _window._projectionBuffer.CurrentSnapshot.GetSourceSpans(); + + int index = IndexOfLastStandardInputSpan(sourceSpans); + Debug.Assert(index >= 0); + + RemoveProtection(_window._standardInputBuffer, _standardInputProtection); + + // replace previous span w/ a span that won't grow... + var oldSpan = sourceSpans[index]; + var newSpan = new CustomTrackingSpan(oldSpan.Snapshot, oldSpan.Span, PointTrackingMode.Negative, PointTrackingMode.Negative); + + ReplaceProjectionSpan(index, newSpan); + ApplyProtection(_window._standardInputBuffer, _standardInputProtection, allowAppend: true); } private void AppendLineNoPromptInjection(ITextBuffer buffer) @@ -327,7 +467,7 @@ private void AppendLineNoPromptInjection(ITextBuffer buffer) public void InsertCode(string text) { - if (_window._stdInputStart != null) + if (ReadingStandardInput) { return; } @@ -349,7 +489,7 @@ public void InsertCode(string text) public void Submit(PendingSubmission[] pendingSubmissions) { - if (_window._stdInputStart == null) + if (!ReadingStandardInput) { if (State == State.WaitingForInput && _window._currentLanguageBuffer != null) { @@ -484,7 +624,7 @@ private void MoveCaretToClosestEditableBuffer() /// private SnapshotPoint GetClosestEditablePoint(SnapshotPoint projectionPoint) { - ITextBuffer editableBuffer = (_window._stdInputStart != null) ? _window._standardInputBuffer : _window._currentLanguageBuffer; + ITextBuffer editableBuffer = (ReadingStandardInput) ? _window._standardInputBuffer : _window._currentLanguageBuffer; if (editableBuffer == null) { @@ -903,6 +1043,7 @@ public async Task ExecuteInputAsync() } await SubmitAsync().ConfigureAwait(true); + Debug.Assert(_window.OnUIThread()); // ConfigureAwait should bring us back to the UI thread. } else { @@ -1116,9 +1257,9 @@ public void Cancel() private void ClearInput() { - if (_window._stdInputStart != null) + if (ReadingStandardInput) { - _window._standardInputBuffer.Delete(Span.FromBounds(_window._stdInputStart.Value, _window._standardInputBuffer.CurrentSnapshot.Length)); + _window._standardInputBuffer.Delete(GetStandardInputSpan()); } else { @@ -1620,7 +1761,7 @@ private void CutOrDelete(IEnumerable projectionSpans, bool isCut) StringBuilder deletedText = null; // split into multiple deletes that only affect the language/input buffer: - ITextBuffer affectedBuffer = (_window._stdInputStart != null) ? _window._standardInputBuffer : _window._currentLanguageBuffer; + ITextBuffer affectedBuffer = (ReadingStandardInput) ? _window._standardInputBuffer : _window._currentLanguageBuffer; using (var edit = affectedBuffer.CreateEdit()) { foreach (var projectionSpan in projectionSpans) @@ -1746,7 +1887,7 @@ private StringBuilder GetTextWithoutPrompts(StringBuilder builder, SnapshotSpan } public bool Backspace() - { + { bool handled = false; if (!_window._textView.Selection.IsEmpty) { @@ -1815,7 +1956,7 @@ private void DeletePreviousCharacter() if (_window._standardInputBuffer != null) { - result = _window.GetPositionInStdInputBuffer(projectionPoint); + result = _window.GetPositionInStandardInputBuffer(projectionPoint); } return result; @@ -1824,7 +1965,7 @@ private void DeletePreviousCharacter() public bool TrySubmitStandardInput() { _historySearch = null; - if (_window._stdInputStart != null) + if (ReadingStandardInput) { if (InStandardInputRegion(_window._textView.Caret.Position.BufferPosition)) { @@ -1840,19 +1981,38 @@ public bool TrySubmitStandardInput() private void SubmitStandardInput() { AppendLineNoPromptInjection(_window._standardInputBuffer); - _window._inputValue = new SnapshotSpan(_window._standardInputBuffer.CurrentSnapshot, Span.FromBounds(_window._stdInputStart.Value, _window._standardInputBuffer.CurrentSnapshot.Length)); - _window._inputEvent.Set(); + var inputSpan = new SnapshotSpan(_window._standardInputBuffer.CurrentSnapshot, GetStandardInputSpan()); + History.Add(inputSpan); + SetStandardInputValue(inputSpan); + + MakeStandardInputReadonly(); + + // Subsequent input should appear after the input span we just finished. + NewOutputBuffer(); + + if (State == State.WaitingForInputAndReadingStandardInput) + { + PrepareForInput(); // Will update State. + } + else + { + State = GetStateBeforeReadingStandardInput(State); + } } private bool InStandardInputRegion(SnapshotPoint point) { - if (_window._stdInputStart == null) + if (!ReadingStandardInput) { return false; } - var stdInputPoint = _window.GetPositionInStdInputBuffer(point); - return stdInputPoint != null && stdInputPoint.Value.Position >= _window._stdInputStart.Value; + var standardInputPoint = _window.GetPositionInStandardInputBuffer(point); + if (!standardInputPoint.HasValue) return false; + + var standardInputPosition = standardInputPoint.GetValueOrDefault().Position; + var standardInputSpan = GetStandardInputSpan(); + return standardInputSpan.Contains(standardInputPosition) || standardInputSpan.End == standardInputPosition; } /// @@ -1982,32 +2142,42 @@ internal enum State /// /// In the process of calling . /// Transition to when finished (in ). - /// Note: Should not see calls while in this state. + /// Transition to when is called /// Resetting, /// /// Prompt has been displayed - waiting for the user to make the next submission. /// Transition to when is called. /// Transition to when is called. + /// Transition to when is called /// WaitingForInput, /// /// Executing the user's submission. /// Transition to when finished (in ). /// Transition to when is called. + /// Transition to when is called /// ExecutingInput, /// - /// In the process of calling . - /// Return to preceding state when finished. + /// In the process of calling (within ). + /// Transition to when , + /// , or + /// is called. + /// + ResettingAndReadingStandardInput, + /// + /// In the process of calling (while prompt has been displayed). + /// Transition to when or is called. + /// Transition to when is called. + /// + WaitingForInputAndReadingStandardInput, + /// + /// In the process of calling (while executing the user's submission). + /// Transition to when or is called. /// Transition to when is called. /// - /// - /// TODO: When we clean up (https://github.com/dotnet/roslyn/issues/3984) - /// we should try to eliminate the "preceding state", since it substantially - /// increases the complexity of the state machine. - /// - ReadingStandardInput, + ExecutingInputAndReadingStandardInput, } } } diff --git a/src/InteractiveWindow/EditorTest/InteractiveWindowTests.cs b/src/InteractiveWindow/EditorTest/InteractiveWindowTests.cs index 8d486f403a95666a38232d4e6c681123d8f0aba5..370a4973af08dd7175eea189686d17da3f16c269 100644 --- a/src/InteractiveWindow/EditorTest/InteractiveWindowTests.cs +++ b/src/InteractiveWindow/EditorTest/InteractiveWindowTests.cs @@ -247,7 +247,6 @@ public void CallCloseOnNonUIThread() [Fact] public void CallInsertCodeOnNonUIThread() { - // TODO (https://github.com/dotnet/roslyn/issues/3984): InsertCode is a no-op unless standard input is being collected. Task.Run(() => Window.InsertCode("1")).PumpingWait(); }