diff --git a/src/System.Diagnostics.Process/src/FxCopBaseline.cs b/src/System.Diagnostics.Process/src/FxCopBaseline.cs
index 2e370b51d3c9..1d3b6208e1bb 100644
--- a/src/System.Diagnostics.Process/src/FxCopBaseline.cs
+++ b/src/System.Diagnostics.Process/src/FxCopBaseline.cs
@@ -7,3 +7,5 @@
[assembly: SuppressMessage("Microsoft.Reliability", "CA2002:DoNotLockOnObjectsWithWeakIdentity", Scope = "member", Target = "System.Diagnostics.Process.#EnsureWatchingForExit()")]
[assembly: SuppressMessage("Microsoft.Reliability", "CA2002:DoNotLockOnObjectsWithWeakIdentity", Scope = "member", Target = "System.Diagnostics.Process.#RaiseOnExited()")]
[assembly: SuppressMessage("Microsoft.Reliability", "CA2002:DoNotLockOnObjectsWithWeakIdentity", Scope = "member", Target = "System.Diagnostics.Process.#StopWatchingForExit()")]
+[assembly: SuppressMessage("Microsoft.Reliability", "CA2002:DoNotLockOnObjectsWithWeakIdentity", Scope = "member", Target = "System.Diagnostics.Process.#CompletionCallback(System.Object,System.Boolean)")]
+[assembly: SuppressMessage("Microsoft.Reliability", "CA2002:DoNotLockOnObjectsWithWeakIdentity", Scope = "member", Target = "System.Diagnostics.Process.Close()")]
\ No newline at end of file
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 63db0a871fdd..5692e471e2a2 100644
--- a/src/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs
+++ b/src/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs
@@ -84,7 +84,8 @@ partial void ConfigureAfterProcessIdSet()
{
// Make sure that we configure the wait state holder for this process object, which we can only do once we have a process ID.
Debug.Assert(_haveProcessId, $"{nameof(ConfigureAfterProcessIdSet)} should only be called once a process ID is set");
- GetWaitState(); // lazily initializes the wait state
+ // Initialize WaitStateHolder for non-child processes
+ GetWaitState();
}
///
@@ -104,9 +105,9 @@ private void EnsureWatchingForExit()
_watchingForExit = true;
try
{
- _waitHandle = new ProcessWaitHandle(_processHandle);
+ _waitHandle = new ProcessWaitHandle(GetWaitState());
_registeredWaitHandle = ThreadPool.RegisterWaitForSingleObject(_waitHandle,
- new WaitOrTimerCallback(CompletionCallback), null, -1, true);
+ new WaitOrTimerCallback(CompletionCallback), _waitHandle, -1, true);
}
catch
{
diff --git a/src/System.Diagnostics.Process/src/System/Diagnostics/Process.Windows.cs b/src/System.Diagnostics.Process/src/System/Diagnostics/Process.Windows.cs
index 1b70bc734741..c1fb4fc7d1d3 100644
--- a/src/System.Diagnostics.Process/src/System/Diagnostics/Process.Windows.cs
+++ b/src/System.Diagnostics.Process/src/System/Diagnostics/Process.Windows.cs
@@ -126,7 +126,7 @@ private void EnsureWatchingForExit()
{
_waitHandle = new Interop.Kernel32.ProcessWaitHandle(_processHandle);
_registeredWaitHandle = ThreadPool.RegisterWaitForSingleObject(_waitHandle,
- new WaitOrTimerCallback(CompletionCallback), null, -1, true);
+ new WaitOrTimerCallback(CompletionCallback), _waitHandle, -1, true);
}
catch
{
diff --git a/src/System.Diagnostics.Process/src/System/Diagnostics/Process.cs b/src/System.Diagnostics.Process/src/System/Diagnostics/Process.cs
index 1509b480032f..addc1bd72a82 100644
--- a/src/System.Diagnostics.Process/src/System/Diagnostics/Process.cs
+++ b/src/System.Diagnostics.Process/src/System/Diagnostics/Process.cs
@@ -783,10 +783,20 @@ public event EventHandler Exited
/// This is called from the threadpool when a process exits.
///
///
- private void CompletionCallback(object context, bool wasSignaled)
+ private void CompletionCallback(object waitHandleContext, bool wasSignaled)
{
- StopWatchingForExit();
- RaiseOnExited();
+ Debug.Assert(waitHandleContext != null, "Process.CompletionCallback called with no waitHandleContext");
+ lock (this)
+ {
+ // Check the exited event that we get from the threadpool
+ // matches the event we are waiting for.
+ if (waitHandleContext != _waitHandle)
+ {
+ return;
+ }
+ StopWatchingForExit();
+ RaiseOnExited();
+ }
}
///
@@ -836,7 +846,14 @@ public void Close()
{
if (_haveProcessHandle)
{
- StopWatchingForExit();
+ // We need to lock to ensure we don't run concurrently with CompletionCallback.
+ // Without this lock we could reset _raisedOnExited which causes CompletionCallback to
+ // raise the Exited event a second time for the same process.
+ lock (this)
+ {
+ // This sets _waitHandle to null which causes CompletionCallback to not emit events.
+ StopWatchingForExit();
+ }
#if FEATURE_TRACESWITCH
Debug.WriteLineIf(_processTracing.TraceVerbose, "Process - CloseHandle(process) in Close()");
#endif
diff --git a/src/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitHandle.Unix.cs b/src/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitHandle.Unix.cs
index 2f9270e1754f..8ce139ca5b59 100644
--- a/src/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitHandle.Unix.cs
+++ b/src/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitHandle.Unix.cs
@@ -9,26 +9,12 @@ namespace System.Diagnostics
{
internal sealed class ProcessWaitHandle : WaitHandle
{
- ///
- /// Holds a wait state object associated with this handle.
- ///
- private ProcessWaitState.Holder _waitStateHolder;
-
- internal ProcessWaitHandle(SafeProcessHandle processHandle)
+ internal ProcessWaitHandle(ProcessWaitState processWaitState)
{
- // Get the process ID from the process handle. The handle is just a facade that wraps
- // the process ID, and closing the handle won't affect the process or its ID at all.
- // So we can grab it, and it's not "dangerous".
- int processId = (int)processHandle.DangerousGetHandle();
-
- // Create a wait state holder for this process ID. This gives us access to the shared
- // wait state associated with this process.
- _waitStateHolder = new ProcessWaitState.Holder(processId);
-
// Get the wait state's event, and use that event's safe wait handle
// in place of ours. This will let code register for completion notifications
// on this ProcessWaitHandle and be notified when the wait state's handle completes.
- ManualResetEvent mre = _waitStateHolder._state.EnsureExitedEvent();
+ ManualResetEvent mre = processWaitState.EnsureExitedEvent();
this.SetSafeWaitHandle(mre.GetSafeWaitHandle());
}
@@ -36,14 +22,6 @@ protected override void Dispose(bool explicitDisposing)
{
// ProcessWaitState will dispose the handle
this.SafeWaitHandle = null;
- if (explicitDisposing)
- {
- if (_waitStateHolder != null)
- {
- _waitStateHolder.Dispose();
- _waitStateHolder = null;
- }
- }
base.Dispose(explicitDisposing);
}
}
diff --git a/src/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs b/src/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs
index b2d689c3004e..f9ab4d64752d 100644
--- a/src/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs
+++ b/src/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs
@@ -114,6 +114,9 @@ internal static ProcessWaitState AddRef(int processId, bool isNewChild)
ProcessWaitState pws;
if (isNewChild)
{
+ // When the PID is recycled for a new child, we remove the old child.
+ s_childProcessWaitStates.Remove(processId);
+
pws = new ProcessWaitState(processId, isChild: true);
s_childProcessWaitStates.Add(processId, pws);
pws._outstandingRefCount++; // For Holder
@@ -142,22 +145,25 @@ internal static ProcessWaitState AddRef(int processId, bool isNewChild)
/// Decrements the ref count on the wait state object, and if it's the last one,
/// removes it from the table.
///
- internal bool ReleaseRef()
+ internal void ReleaseRef()
{
ProcessWaitState pws;
Dictionary waitStates = _isChild ? s_childProcessWaitStates : s_processWaitStates;
- bool removed = false;
lock (waitStates)
{
bool foundState = waitStates.TryGetValue(_processId, out pws);
Debug.Assert(foundState);
if (foundState)
{
- --pws._outstandingRefCount;
- if (pws._outstandingRefCount == 0)
+ --_outstandingRefCount;
+ if (_outstandingRefCount == 0)
{
- waitStates.Remove(_processId);
- removed = true;
+ // The dictionary may contain a different ProcessWaitState if the pid was recycled.
+ if (pws == this)
+ {
+ waitStates.Remove(_processId);
+ }
+ pws = this;
}
else
{
@@ -166,7 +172,6 @@ internal bool ReleaseRef()
}
}
pws?.Dispose();
- return removed;
}
///
diff --git a/src/System.Diagnostics.Process/tests/ProcessTests.Unix.cs b/src/System.Diagnostics.Process/tests/ProcessTests.Unix.cs
index 617d1b7cf699..cab7f8e8734b 100644
--- a/src/System.Diagnostics.Process/tests/ProcessTests.Unix.cs
+++ b/src/System.Diagnostics.Process/tests/ProcessTests.Unix.cs
@@ -515,6 +515,39 @@ public void TestProcessWaitStateReferenceCount()
}
}
+ ///
+ /// Verifies a new Process instance can refer to a process with a recycled pid
+ /// for which there is still an existing Process instance.
+ ///
+ [ConditionalFact(typeof(TestEnvironment), nameof(TestEnvironment.IsStressModeEnabled))]
+ public void TestProcessRecycledPid()
+ {
+ const int LinuxPidMaxDefault = 32768;
+ var processes = new Dictionary(LinuxPidMaxDefault);
+ bool foundRecycled = false;
+ for (int i = 0; i < int.MaxValue; i++)
+ {
+ var process = CreateProcessLong();
+ process.Start();
+
+ foundRecycled = processes.ContainsKey(process.Id);
+
+ process.Kill();
+ process.WaitForExit();
+
+ if (foundRecycled)
+ {
+ break;
+ }
+ else
+ {
+ processes.Add(process.Id, process);
+ }
+ }
+
+ Assert.True(foundRecycled);
+ }
+
private static IDictionary GetWaitStateDictionary(bool childDictionary)
{
Assembly assembly = typeof(Process).Assembly;