Merge pull request #27802 from ivanbasov/dev15.7.x

servicing fix for CLR_EXCEPTION_System.NullReferenceException_80004003_Microsoft.VisualStudio.InteractiveWindow.dll into dev15.7

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

@@ -137,8 +137,8 @@ public IInteractiveWindow CurrentWindow
                 _currentWindow = value;
                 _workspace.Window = value;
 
-                _interactiveHost.Output = _currentWindow.OutputWriter;
-                _interactiveHost.ErrorOutput = _currentWindow.ErrorOutputWriter;
+                _interactiveHost.SetOutput( _currentWindow.OutputWriter);
+                _interactiveHost.SetErrorOutput(_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.Output = window.OutputWriter;
-            _interactiveHost.ErrorOutput = window.ErrorOutputWriter;
+            _interactiveHost.SetOutput(window.OutputWriter);
+            _interactiveHost.SetErrorOutput(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)
                     {
-                        remoteService.Dispose(joinThreads: false);
                         Host.ReportProcessExited(remoteService.Process);
+                        remoteService.Dispose(joinThreads: false);
 
                         return default(InitializedRemoteService);
                     }

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

@@ -74,9 +74,14 @@ 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))
                 {
@@ -114,19 +119,17 @@ private void ReadOutput(bool error)
                 }
             }
 
+            // Dispose may called anytime.
             internal void Dispose(bool joinThreads)
             {
                 // There can be a call from host initiated from OnProcessExit. 
-                // This check on the beginning helps to avoid a reentrancy.
-                if (_processExitHandlerStatus == ProcessExitHandlerStatus.Hooked)
+                // We should not proceed with disposing if _disposeSemaphore is locked.
+                using (_disposeSemaphore.DisposableWait())
                 {
-                    using (_disposeSemaphore.DisposableWait())
+                    if (_processExitHandlerStatus == ProcessExitHandlerStatus.Hooked)
                     {
-                        if (_processExitHandlerStatus == ProcessExitHandlerStatus.Hooked)
-                        {
-                            Process.Exited -= ProcessExitedHandler;
-                            _processExitHandlerStatus = ProcessExitHandlerStatus.Handled;
-                        }
+                       Process.Exited -= ProcessExitedHandler;
+                       _processExitHandlerStatus = ProcessExitHandlerStatus.Handled;
                     }
                 }
 

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

@@ -40,6 +40,8 @@ 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;
 
@@ -55,6 +57,8 @@ 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);
@@ -69,9 +73,9 @@ internal sealed partial class InteractiveHost : MarshalByRefObject
         internal Process TryGetProcess()
         {
             InitializedRemoteService initializedService;
-
-            return (_lazyRemoteService?.InitializedService != null &&
-                    _lazyRemoteService.InitializedService.TryGetValue(out initializedService)) ? initializedService.ServiceOpt.Process : null;
+            var lazyRemoteService = _lazyRemoteService;
+            return (lazyRemoteService?.InitializedService != null &&
+                    lazyRemoteService.InitializedService.TryGetValue(out initializedService)) ? initializedService.ServiceOpt.Process : null;
         }
 
         internal Service TryGetService()
@@ -167,7 +171,11 @@ private RemoteService TryStartProcess(CultureInfo culture, CancellationToken can
                         return null;
                     }
 
-                    _output.WriteLine(FeaturesResources.Attempt_to_connect_to_process_Sharp_0_failed_retrying, newProcessId);
+                    lock (_outputGuard)
+                    {
+                        _output.WriteLine(FeaturesResources.Attempt_to_connect_to_process_Sharp_0_failed_retrying, newProcessId);
+                    }
+
                     cancellationToken.ThrowIfCancellationRequested();
                 }
 
@@ -212,8 +220,12 @@ private bool CheckAlive(Process process)
             bool alive = process.IsAlive();
             if (!alive)
             {
-                _errorOutput.WriteLine(FeaturesResources.Failed_to_launch_0_process_exit_code_colon_1_with_output_colon, _hostPath, process.ExitCode);
-                _errorOutput.WriteLine(process.StandardError.ReadToEnd());
+                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);
+                }
             }
 
             return alive;
@@ -224,9 +236,12 @@ 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);
         }
@@ -249,41 +264,31 @@ private void DisposeChannel()
             }
         }
 
-        public TextWriter Output
+        public void SetOutput(TextWriter value)
         {
-            get
+            if (value == null)
             {
-                return _output;
+                throw new ArgumentNullException(nameof(value));
             }
 
-            set
+            lock(_outputGuard)
             {
-                if (value == null)
-                {
-                    throw new ArgumentNullException(nameof(value));
-                }
-
-                var oldOutput = Interlocked.Exchange(ref _output, value);
-                oldOutput.Flush();
+                _output.Flush();
+                _output = value;
             }
         }
 
-        public TextWriter ErrorOutput
+        public void SetErrorOutput(TextWriter value)
         {
-            get
+            if (value == null)
             {
-                return _errorOutput;
+                throw new ArgumentNullException(nameof(value));
             }
 
-            set
+            lock(_errorOutputGuard)
             {
-                if (value == null)
-                {
-                    throw new ArgumentNullException(nameof(value));
-                }
-
-                var oldOutput = Interlocked.Exchange(ref _errorOutput, value);
-                oldOutput.Flush();
+                _errorOutput.Flush();
+                _errorOutput = value;
             }
         }
 
@@ -291,8 +296,12 @@ internal void OnOutputReceived(bool error, char[] buffer, int count)
         {
             (error ? ErrorOutputReceived : OutputReceived)?.Invoke(buffer, count);
 
-            var writer = error ? ErrorOutput : Output;
-            writer.Write(buffer, 0, count);
+            var writer = error ? _errorOutput : _output;
+            var guard = error ? _errorOutputGuard : _outputGuard;
+            lock (guard)
+            {
+                writer.Write(buffer, 0, count);
+            }
         }
 
         private LazyRemoteService CreateRemoteService(InteractiveHostOptions options, bool skipInitialization)
@@ -320,7 +329,10 @@ private void ReportProcessExited(Process process)
 
             if (exitCode.HasValue)
             {
-                _errorOutput.WriteLine(FeaturesResources.Hosting_process_exited_with_exit_code_0, exitCode.Value);
+                lock (_errorOutputGuard)
+                {
+                    _errorOutput.WriteLine(FeaturesResources.Hosting_process_exited_with_exit_code_0, exitCode.Value);
+                }
             }
         }
 
@@ -330,11 +342,14 @@ 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())
                     {
@@ -359,7 +374,10 @@ private async Task<InitializedRemoteService> TryGetOrCreateRemoteServiceAsync(bo
                     }
                 }
 
-                _errorOutput.WriteLine(FeaturesResources.Unable_to_create_hosting_process);
+                lock (_errorOutputGuard)
+                {
+                    _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.Output = _synchronizedOutput;
-            _host.ErrorOutput = _synchronizedErrorOutput;
+            _host.SetOutput(_synchronizedOutput);
+            _host.SetErrorOutput(_synchronizedErrorOutput);
         }
 
         private bool LoadReference(string reference)