Merge remote-tracking branch 'origin/dev15.8-preview2' into merges/dev15.8-preview2-to-master

src/Interactive/EditorFeatures/Core/Extensibility/Interactive/InteractiveEvaluator.cs

@@ -137,8 +137,8 @@ public IInteractiveWindow CurrentWindow
                 _currentWindow = value;
                 _workspace.Window = value;
 
-                _interactiveHost.SetOutput( _currentWindow.OutputWriter);
-                _interactiveHost.SetErrorOutput(_currentWindow.ErrorOutputWriter);
+                _interactiveHost.Output = _currentWindow.OutputWriter;
+                _interactiveHost.ErrorOutput = _currentWindow.ErrorOutputWriter;
 
                 _currentWindow.SubmissionBufferAdded += SubmissionBufferAdded;
                 _interactiveCommands = _commandsFactory.CreateInteractiveCommands(_currentWindow, CommandPrefix, _commands);
@@ -171,8 +171,8 @@ private IInteractiveWindow GetCurrentWindowOrThrow()
         public Task<ExecutionResult> InitializeAsync()
         {
             var window = GetCurrentWindowOrThrow();
-            _interactiveHost.SetOutput(window.OutputWriter);
-            _interactiveHost.SetErrorOutput(window.ErrorOutputWriter);
+            _interactiveHost.Output = window.OutputWriter;
+            _interactiveHost.ErrorOutput = window.ErrorOutputWriter;
             return ResetAsyncWorker();
         }
 

src/Interactive/Features/Interactive/Core/InteractiveHost.LazyRemoteService.cs

@@ -81,8 +81,8 @@ private async Task<InitializedRemoteService> TryStartAndInitializeProcessAsync(C
 
                     if (!initializationResult.Success)
                     {
-                        Host.ReportProcessExited(remoteService.Process);
                         remoteService.Dispose(joinThreads: false);
+                        Host.ReportProcessExited(remoteService.Process);
 
                         return default(InitializedRemoteService);
                     }

src/Interactive/Features/Interactive/Core/InteractiveHost.RemoteService.cs

@@ -74,14 +74,9 @@ private async void ProcessExitedHandler(object _, EventArgs __)
                             _processExitHandlerStatus = ProcessExitHandlerStatus.Handled;
                             // Should set _processExitHandlerStatus before calling OnProcessExited to avoid deadlocks.
                             // Calling the host should be within the lock to prevent its disposing during the execution.
+                            await _host.OnProcessExited(Process).ConfigureAwait(false);
                         }
                     }
-
-                    var host = _host;
-                    if (host != null)
-                    {
-                        await host.OnProcessExited(Process).ConfigureAwait(false);
-                    }
                 }
                 catch (Exception e) when (FatalError.Report(e))
                 {
@@ -119,17 +114,19 @@ private void ReadOutput(bool error)
                 }
             }
 
-            // Dispose may called anytime.
             internal void Dispose(bool joinThreads)
             {
                 // There can be a call from host initiated from OnProcessExit. 
-                // We should not proceed with disposing if _disposeSemaphore is locked.
-                using (_disposeSemaphore.DisposableWait())
+                // This check on the beginning helps to avoid a reentrancy.
+                if (_processExitHandlerStatus == ProcessExitHandlerStatus.Hooked)
                 {
-                    if (_processExitHandlerStatus == ProcessExitHandlerStatus.Hooked)
+                    using (_disposeSemaphore.DisposableWait())
                     {
-                       Process.Exited -= ProcessExitedHandler;
-                       _processExitHandlerStatus = ProcessExitHandlerStatus.Handled;
+                        if (_processExitHandlerStatus == ProcessExitHandlerStatus.Hooked)
+                        {
+                            Process.Exited -= ProcessExitedHandler;
+                            _processExitHandlerStatus = ProcessExitHandlerStatus.Handled;
+                        }
                     }
                 }
 

src/Interactive/Features/Interactive/Core/InteractiveHost.cs

@@ -40,8 +40,6 @@ internal sealed partial class InteractiveHost : MarshalByRefObject
 
         private TextWriter _output;
         private TextWriter _errorOutput;
-        private readonly object _outputGuard;
-        private readonly object _errorOutputGuard;
 
         internal event Action<bool> ProcessStarting;
 
@@ -57,8 +55,6 @@ internal sealed partial class InteractiveHost : MarshalByRefObject
             _replServiceProviderType = replServiceProviderType;
             _hostPath = hostPath;
             _initialWorkingDirectory = workingDirectory;
-            _outputGuard = new object();
-            _errorOutputGuard = new object();
 
             var serverProvider = new BinaryServerFormatterSinkProvider { TypeFilterLevel = TypeFilterLevel.Full };
             _serverChannel = new IpcServerChannel(GenerateUniqueChannelLocalName(), "ReplChannel-" + Guid.NewGuid(), serverProvider);
@@ -73,9 +69,9 @@ internal sealed partial class InteractiveHost : MarshalByRefObject
         internal Process TryGetProcess()
         {
             InitializedRemoteService initializedService;
-            var lazyRemoteService = _lazyRemoteService;
-            return (lazyRemoteService?.InitializedService != null &&
-                    lazyRemoteService.InitializedService.TryGetValue(out initializedService)) ? initializedService.ServiceOpt.Process : null;
+
+            return (_lazyRemoteService?.InitializedService != null &&
+                    _lazyRemoteService.InitializedService.TryGetValue(out initializedService)) ? initializedService.ServiceOpt.Process : null;
         }
 
         internal Service TryGetService()
@@ -171,11 +167,7 @@ private RemoteService TryStartProcess(CultureInfo culture, CancellationToken can
                         return null;
                     }
 
-                    lock (_outputGuard)
-                    {
-                        _output.WriteLine(FeaturesResources.Attempt_to_connect_to_process_Sharp_0_failed_retrying, newProcessId);
-                    }
-
+                    _output.WriteLine(FeaturesResources.Attempt_to_connect_to_process_Sharp_0_failed_retrying, newProcessId);
                     cancellationToken.ThrowIfCancellationRequested();
                 }
 
@@ -220,12 +212,8 @@ private bool CheckAlive(Process process)
             bool alive = process.IsAlive();
             if (!alive)
             {
-                string errorString = process.StandardError.ReadToEnd();
-                lock (_errorOutputGuard)
-                {
-                    _errorOutput.WriteLine(FeaturesResources.Failed_to_launch_0_process_exit_code_colon_1_with_output_colon, _hostPath, process.ExitCode);
-                    _errorOutput.WriteLine(errorString);
-                }
+                _errorOutput.WriteLine(FeaturesResources.Failed_to_launch_0_process_exit_code_colon_1_with_output_colon, _hostPath, process.ExitCode);
+                _errorOutput.WriteLine(process.StandardError.ReadToEnd());
             }
 
             return alive;
@@ -236,12 +224,9 @@ private bool CheckAlive(Process process)
             DisposeRemoteService(disposing: false);
         }
 
-        // Dispose may be called anytime.
         public void Dispose()
         {
             DisposeChannel();
-            SetOutput(TextWriter.Null);
-            SetErrorOutput(TextWriter.Null);
             DisposeRemoteService(disposing: true);
             GC.SuppressFinalize(this);
         }
@@ -264,31 +249,41 @@ private void DisposeChannel()
             }
         }
 
-        public void SetOutput(TextWriter value)
+        public TextWriter Output
         {
-            if (value == null)
+            get
             {
-                throw new ArgumentNullException(nameof(value));
+                return _output;
             }
 
-            lock(_outputGuard)
+            set
             {
-                _output.Flush();
-                _output = value;
+                if (value == null)
+                {
+                    throw new ArgumentNullException(nameof(value));
+                }
+
+                var oldOutput = Interlocked.Exchange(ref _output, value);
+                oldOutput.Flush();
             }
         }
 
-        public void SetErrorOutput(TextWriter value)
+        public TextWriter ErrorOutput
         {
-            if (value == null)
+            get
             {
-                throw new ArgumentNullException(nameof(value));
+                return _errorOutput;
             }
 
-            lock(_errorOutputGuard)
+            set
             {
-                _errorOutput.Flush();
-                _errorOutput = value;
+                if (value == null)
+                {
+                    throw new ArgumentNullException(nameof(value));
+                }
+
+                var oldOutput = Interlocked.Exchange(ref _errorOutput, value);
+                oldOutput.Flush();
             }
         }
 
@@ -296,12 +291,8 @@ internal void OnOutputReceived(bool error, char[] buffer, int count)
         {
             (error ? ErrorOutputReceived : OutputReceived)?.Invoke(buffer, count);
 
-            var writer = error ? _errorOutput : _output;
-            var guard = error ? _errorOutputGuard : _outputGuard;
-            lock (guard)
-            {
-                writer.Write(buffer, 0, count);
-            }
+            var writer = error ? ErrorOutput : Output;
+            writer.Write(buffer, 0, count);
         }
 
         private LazyRemoteService CreateRemoteService(InteractiveHostOptions options, bool skipInitialization)
@@ -329,10 +320,7 @@ private void ReportProcessExited(Process process)
 
             if (exitCode.HasValue)
             {
-                lock (_errorOutputGuard)
-                {
-                    _errorOutput.WriteLine(FeaturesResources.Hosting_process_exited_with_exit_code_0, exitCode.Value);
-                }
+                _errorOutput.WriteLine(FeaturesResources.Hosting_process_exited_with_exit_code_0, exitCode.Value);
             }
         }
 
@@ -342,14 +330,11 @@ private async Task<InitializedRemoteService> TryGetOrCreateRemoteServiceAsync(bo
             {
                 LazyRemoteService currentRemoteService = _lazyRemoteService;
 
+                // disposed or not reset:
+                Debug.Assert(currentRemoteService != null);
+
                 for (int attempt = 0; attempt < MaxAttemptsToCreateProcess; attempt++)
                 {
-                    // Remote service may be disposed anytime.
-                    if (currentRemoteService == null)
-                    {
-                        return default;
-                    }
-
                     var initializedService = await currentRemoteService.InitializedService.GetValueAsync(currentRemoteService.CancellationSource.Token).ConfigureAwait(false);
                     if (initializedService.ServiceOpt != null && initializedService.ServiceOpt.Process.IsAlive())
                     {
@@ -374,10 +359,7 @@ private async Task<InitializedRemoteService> TryGetOrCreateRemoteServiceAsync(bo
                     }
                 }
 
-                lock (_errorOutputGuard)
-                {
-                    _errorOutput.WriteLine(FeaturesResources.Unable_to_create_hosting_process);
-                }
+                _errorOutput.WriteLine(FeaturesResources.Unable_to_create_hosting_process);
             }
             catch (OperationCanceledException)
             {

src/Interactive/HostTest/InteractiveHostTests.cs

@@ -92,8 +92,8 @@ private void RedirectOutput()
             _synchronizedOutput = new SynchronizedStringWriter();
             _synchronizedErrorOutput = new SynchronizedStringWriter();
             ClearOutput();
-            _host.SetOutput(_synchronizedOutput);
-            _host.SetErrorOutput(_synchronizedErrorOutput);
+            _host.Output = _synchronizedOutput;
+            _host.ErrorOutput = _synchronizedErrorOutput;
         }
 
         private bool LoadReference(string reference)