diff --git a/src/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs b/src/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs index eff131c23ce2..e25e6d95fb38 100644 --- a/src/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs +++ b/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; } } diff --git a/src/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs b/src/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs index e1b274a60823..e886b245bc63 100644 --- a/src/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs +++ b/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); diff --git a/src/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs b/src/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs index 471ddfd8cd86..b2713b80dff0 100644 --- a/src/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs +++ b/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) { } diff --git a/src/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs b/src/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs index 590a8eced78f..c6cc13236698 100644 --- a/src/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs +++ b/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); } /// @@ -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; } diff --git a/src/System.Diagnostics.Process/src/System/Diagnostics/Process.cs b/src/System.Diagnostics.Process/src/System/Diagnostics/Process.cs index a7b8a24f1836..efb5afa08a75 100644 --- a/src/System.Diagnostics.Process/src/System/Diagnostics/Process.cs +++ b/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. /// - private SafeProcessHandle OpenProcessHandle() + private SafeProcessHandle GetOrOpenProcessHandle() { if (!_haveProcessHandle) { diff --git a/src/System.Diagnostics.Process/src/System/Diagnostics/ProcessManager.Unix.cs b/src/System.Diagnostics.Process/src/System/Diagnostics/ProcessManager.Unix.cs index 6621d9afe749..5209a1bbc53b 100644 --- a/src/System.Diagnostics.Process/src/System/Diagnostics/ProcessManager.Unix.cs +++ b/src/System.Diagnostics.Process/src/System/Diagnostics/ProcessManager.Unix.cs @@ -56,7 +56,7 @@ public static int[] GetProcessIds(string machineName) /// The process ID. 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) diff --git a/src/System.Diagnostics.Process/tests/ProcessTests.cs b/src/System.Diagnostics.Process/tests/ProcessTests.cs index d7994ed87679..290d763048ab 100644 --- a/src/System.Diagnostics.Process/tests/ProcessTests.cs +++ b/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() { diff --git a/src/System.Runtime/tests/System/ExitCodeTests.Unix.cs b/src/System.Runtime/tests/System/ExitCodeTests.Unix.cs index 6a9671dffc12..d96e56592894 100644 --- a/src/System.Runtime/tests/System/ExitCodeTests.Unix.cs +++ b/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) } } } -} \ No newline at end of file +}