Make Process.{Safe}Handle be a waitable event handle on Unix

src/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs

@@ -12,29 +12,37 @@
 ===========================================================*/
 
 using System;
-using System.Runtime.InteropServices;
-using System.Security;
 
 namespace Microsoft.Win32.SafeHandles
 {
     public sealed partial class SafeProcessHandle : SafeHandleZeroOrMinusOneIsInvalid
     {
-        private const int DefaultInvalidHandleValue = -1;
+        // On Windows, SafeProcessHandle represents the actual OS handle for the process.
+        // On Unix, there's no such concept.  Instead, the implementation manufactures
+        // a WaitHandle that it manually sets when the process completes; SafeProcessHandle
+        // then just wraps that same WaitHandle instance.  This allows consumers that use
+        // Process.{Safe}Handle to initalize and use a WaitHandle to successfull use it on
+        // Unix as well to wait for the process to complete.
 
-        internal SafeProcessHandle(int processId) : 
-            this((IntPtr)processId)
-        {
-        }
+        private readonly SafeWaitHandle _handle;
+        private readonly bool _releaseRef;
 
-        public override bool IsInvalid
+        internal SafeProcessHandle(int processId, SafeWaitHandle handle) : 
+            this(handle.DangerousGetHandle(), ownsHandle: false)
         {
-            get { return ((int)handle) < 0; } // Unix processes have non-negative ID values
+            ProcessId = processId;
+            _handle = handle;
+            handle.DangerousAddRef(ref _releaseRef);
         }
 
+        internal int ProcessId { get; }
+
         protected override bool ReleaseHandle()
         {
-            // Nop.  We don't actually hold handles to a process, as there's no equivalent
-            // concept on Unix.  SafeProcessHandle justs wrap a process ID.
+            if (_releaseRef)
+            {
+                _handle.DangerousRelease();
+            }
             return true;
         }
     }

src/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs

@@ -19,8 +19,6 @@ namespace Microsoft.Win32.SafeHandles
 {
     public sealed partial class SafeProcessHandle : SafeHandleZeroOrMinusOneIsInvalid
     {
-        private const int DefaultInvalidHandleValue = 0;
-
         protected override bool ReleaseHandle()
         {
             return Interop.Kernel32.CloseHandle(handle);

src/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs

@@ -13,8 +13,6 @@
 
 using System;
 using System.Diagnostics;
-using System.Runtime.InteropServices;
-using System.Security;
 
 namespace Microsoft.Win32.SafeHandles
 {
@@ -23,7 +21,7 @@ public sealed partial class SafeProcessHandle : SafeHandleZeroOrMinusOneIsInvali
         internal static readonly SafeProcessHandle InvalidHandle = new SafeProcessHandle();
 
         internal SafeProcessHandle()
-            : this(new IntPtr(DefaultInvalidHandleValue))
+            : this(IntPtr.Zero)
         {
         }
 

src/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs

@@ -159,7 +159,8 @@ private void EnsureWatchingForExit()
                 {
                     if (!_watchingForExit)
                     {
-                        Debug.Assert(_haveProcessHandle, "Process.EnsureWatchingForExit called with no process handle");
+                        Debug.Assert(_waitHandle == null);
+                        Debug.Assert(_registeredWaitHandle == null);
                         Debug.Assert(Associated, "Process.EnsureWatchingForExit called with no associated process");
                         _watchingForExit = true;
                         try
@@ -334,7 +335,8 @@ private SafeProcessHandle GetProcessHandle()
             }
 
             EnsureState(State.HaveNonExitedId | State.IsLocal);
-            return new SafeProcessHandle(_processId);
+            EnsureWatchingForExit();
+            return new SafeProcessHandle(_processId, _waitHandle.SafeWaitHandle);
         }
 
         /// <summary>
@@ -491,7 +493,7 @@ private bool ForkAndExecProcess(
                     // Store the child's information into this Process object.
                     Debug.Assert(childPid >= 0);
                     SetProcessId(childPid);
-                    SetProcessHandle(new SafeProcessHandle(childPid));
+                    SetProcessHandle(GetProcessHandle());
 
                     return true;
                 }

src/System.Diagnostics.Process/src/System/Diagnostics/Process.cs

@@ -119,7 +119,7 @@ public SafeProcessHandle SafeHandle
             get
             {
                 EnsureState(State.Associated);
-                return OpenProcessHandle();
+                return GetOrOpenProcessHandle();
             }
         }
 
@@ -657,7 +657,7 @@ public bool EnableRaisingEvents
                     {
                         if (value)
                         {
-                            OpenProcessHandle();
+                            GetOrOpenProcessHandle();
                             EnsureWatchingForExit();
                         }
                         else
@@ -1130,7 +1130,7 @@ public void Refresh()
         /// Opens a long-term handle to the process, with all access.  If a handle exists,
         /// then it is reused.  If the process has exited, it throws an exception.
         /// </summary>
-        private SafeProcessHandle OpenProcessHandle()
+        private SafeProcessHandle GetOrOpenProcessHandle()
         {
             if (!_haveProcessHandle)
             {

src/System.Diagnostics.Process/src/System/Diagnostics/ProcessManager.Unix.cs

@@ -56,7 +56,7 @@ public static int[] GetProcessIds(string machineName)
         /// <returns>The process ID.</returns>
         public static int GetProcessIdFromHandle(SafeProcessHandle processHandle)
         {
-            return (int)processHandle.DangerousGetHandle(); // not actually dangerous; just wraps a process ID
+            return processHandle.ProcessId;
         }
 
         private static bool IsRemoteMachineCore(string machineName)

src/System.Diagnostics.Process/tests/ProcessTests.cs

@@ -12,6 +12,7 @@
 using System.Security;
 using System.Text;
 using System.Threading;
+using Microsoft.Win32.SafeHandles;
 using Xunit;
 using Xunit.Sdk;
 
@@ -946,6 +947,24 @@ public void TestSafeHandle()
             Assert.False(_process.SafeHandle.IsInvalid);
         }
 
+        [Theory]
+        [InlineData(false)]
+        [InlineData(true)]
+        public void Handle_CreateEvent_BlocksUntilProcessCompleted(bool useSafeHandle)
+        {
+            using (RemoteInvokeHandle h = RemoteInvoke(() => Console.ReadLine(), new RemoteInvokeOptions { StartInfo = new ProcessStartInfo() { RedirectStandardInput = true } }))
+            using (var mre = new ManualResetEvent(false))
+            {
+                mre.SetSafeWaitHandle(new SafeWaitHandle(useSafeHandle ? h.Process.SafeHandle.DangerousGetHandle() : h.Process.Handle, ownsHandle: false));
+
+                Assert.False(mre.WaitOne(millisecondsTimeout: 0), "Event should not yet have been set.");
+
+                h.Process.StandardInput.WriteLine(); // allow child to complete
+
+                Assert.True(mre.WaitOne(FailWaitTimeoutMilliseconds), "Event should have been set.");
+            }
+        }
+
         [Fact]
         public void SafeHandle_GetNotStarted_ThrowsInvalidOperationException()
         {

src/System.Runtime/tests/System/ExitCodeTests.Unix.cs

@@ -49,7 +49,7 @@ public void SigTermExitCode(int? exitCodeOnSigterm)
                 Assert.Equal("Application started", processOutput);
 
                 // Send SIGTERM
-                int rv = kill(process.Handle.ToInt32(), SIGTERM);
+                int rv = kill(process.Id, SIGTERM);
                 Assert.Equal(0, rv);
 
                 // Process exits in a timely manner
@@ -61,4 +61,4 @@ public void SigTermExitCode(int? exitCodeOnSigterm)
             }
         }
     }
-}
+}