Handle process outputs correctly. Fixes #165

Changelog.md

@@ -3,7 +3,11 @@ This project uses [semantic versioning](http://semver.org/spec/v2.0.0.html). Ref
 *[Semantic Versioning in Practice](https://www.jering.tech/articles/semantic-versioning-in-practice)*
 for an overview of semantic versioning.
 
-## [Unreleased](https://github.com/JeringTech/Javascript.NodeJS/compare/7.0.0-beta.3...HEAD)
+## [Unreleased](https://github.com/JeringTech/Javascript.NodeJS/compare/7.0.0-beta.4...HEAD)
+
+## [7.0.0-beta.4](https://github.com/JeringTech/Javascript.NodeJS/compare/7.0.0-beta.3...7.0.0-beta.4) - Apr 18, 2023
+### Fixes
+- Handle process outputs correctly. ([#166](https://github.com/JeringTech/Javascript.NodeJS/pull/166)).
 
 ## [7.0.0-beta.3](https://github.com/JeringTech/Javascript.NodeJS/compare/7.0.0-beta.2...7.0.0-beta.3) - Feb 14, 2023
 ### Additions

src/NodeJS/NodeJSServiceImplementations/OutOfProcess/NodeJSProcess.cs

@@ -106,13 +106,14 @@ private void OutputThreadStart(object? arg)
             try
             {
                 string? data;
-                while ((data = streamReader.ReadLine()) != null)
+                do
                 {
+                    data = streamReader.ReadLine();
                     if (TryCreateMessage(_outputDataStringBuilder, data, out string? message))
                     {
                         _outputReceivedHandler!(message);
                     }
-                }
+                } while (data != null);
             }
             catch (IOException)
             {
@@ -131,13 +132,14 @@ private void ErrorThreadStart(object? arg)
             try
             {
                 string? data;
-                while ((data = streamReader.ReadLine()) != null)
+                do
                 {
+                    data = streamReader.ReadLine();
                     if (TryCreateMessage(_errorDataStringBuilder, data, out string? message))
                     {
                         _errorReceivedHandler!(message);
                     }
-                }
+                } while (data != null);
             }
             catch (IOException)
             {
@@ -387,13 +389,22 @@ protected virtual async ValueTask DisposeAsyncCore()
                     try
                     {
                         _process.Kill();
-                        await _process.WaitForExitAsync().ConfigureAwait(false);
                     }
                     catch
                     {
                         // Throws if process is already dead, note that process could die between HasExited check and Kill
                     }
                 }
+
+                // Wait for exit
+                //
+                // Even after HasExited is true, there may still be output to be read. See https://github.com/dotnet/runtime/blob/91b93eb22bc7d9029a38469e55aa72d52c087834/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs#L1475.
+                //
+                // Should not throw. Process has started so Process._haveProcessID and Process._haveProcessHandle are true. They are only set to false
+                // by Process.Close, which is only called by Process.Dispose, called below.
+                await _process.WaitForExitAsync().ConfigureAwait(false);
+
+                // Dispose
                 _process.Dispose();
 
                 _disposed = true;
@@ -445,13 +456,17 @@ protected virtual void Dispose(bool disposing)
                         try
                         {
                             _process?.Kill();
-                            _process?.WaitForExit(); // Blocks
                         }
                         catch
                         {
                             // Throws if process is already dead, note that process could die between HasExited check and Kill
                         }
                     }
+
+                    // Wait for exit (see DisposeAsync for why we call this outside of the HasExited == false block)
+                    _process?.WaitForExit(); // Blocks
+
+                    // Dispose
                     _process?.Dispose();
                 }