Bug 51742 - Process.WaitForExit for non-child processes does not work correctly
Summary: Process.WaitForExit for non-child processes does not work correctly
Alias: None
Product: Runtime
Classification: Mono
Component: io-layer ()
Version: 4.8.0 (C9)
Hardware: PC Linux
: --- normal
Target Milestone: ---
Assignee: Ludovic Henry
Depends on:
Reported: 2017-01-24 20:44 UTC by mgi
Modified: 2017-02-21 18:51 UTC (History)
4 users (show)

Is this bug a regression?: ---
Last known good build:

test-process.cs (2.21 KB, text/plain)
2017-02-16 23:42 UTC, Ludovic Henry
test-process.cs (2.32 KB, text/plain)
2017-02-21 14:12 UTC, Ludovic Henry

Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.

Please create a new report on GitHub or Developer Community with your current version information, steps to reproduce, and relevant error messages or log files if you are hitting an issue that looks similar to this resolved bug and you do not yet see a matching new report.

Related Links:

Description mgi 2017-01-24 20:44:17 UTC
On a Process not started from mono, WaitForExit does not work and returns immediately

var proc = Process.GetProcessById(1)

WaitForExit(timeout) returns false immediately
WaitForExit returns immediately

using Process.Exited event also causes the event to be fired immediately, without the process having exited

HasExited works correctly

Worked correctly in 4.6.2 and older

Possibly related to this:
Comment 1 mgi 2017-01-24 21:10:44 UTC
Update: it seems its also broken in 4.6.2
Comment 2 Ludovic Henry 2017-02-16 00:09:47 UTC
This is because there is no way to wait for a non-child process on Unix. We can only approximate if a PID is still alive by using `kill`, but if a PID is reused, then HasExited will still return true, even if the original process has exited. So WaitForExit returning immediately false is the expected behaviour.

I will still investigate the Exited event being fired.
Comment 3 Ludovic Henry 2017-02-16 00:10:14 UTC
HasExited will still return false *
Comment 4 Ludovic Henry 2017-02-16 01:49:28 UTC
I added a test case at https://github.com/ludovic-henry/mono/blob/4b1207020c536a91708f9d66033cc2de32ccd773/mcs/class/System/Test/System.Diagnostics/ProcessTest.cs#L1135-L1180 and cannot reproduce the behaviour you are describing, even on mono-4.8.0-branch or mono/master. Can you check if the test case reproduce on your machine, or provide a repro? Thank you.
Comment 5 mg 2017-02-16 17:57:19 UTC
I think Process.EnableRaisingEvents() needs to be called otherwise the event is never fired (after subscribing, calling it before event subscription races with the subscription because its fired (almost?) immediately)
Comment 6 Ludovic Henry 2017-02-16 23:42:25 UTC
You are right, EnableRaisingEvents is needed. But even with it, the bug doesn't reproduce with the above testcase on mono/mono-4.8.0-branch and mono/master.

Can you run the attached script on your machine, and possibly modify to reproduce your bug? I ran it with mono-4.8.0-branch/8aed84d and cannot reproduce. Thank you!
Comment 7 Ludovic Henry 2017-02-16 23:42:44 UTC
Created attachment 19855 [details]
Comment 8 mg 2017-02-20 15:50:01 UTC
I can't reproduce it with your code, but if you add a Thread.Sleep(10) after 
EnableRaisingEvents I can repro it reliably (on a running non-child process)

The problem is that the assertions run too fast and after exiting the using() the event had no chance to fire
Comment 9 mg 2017-02-20 16:12:05 UTC
Also, it seems that Exceptions thrown inside the Exited handler are just ignored, I had to add a Console.WriteLine to test it

It seems that RegisterWaitForSingleObject() which is used by Process to call the Exited handler silently swallows all exceptions (in .net on Windows it reports an unhandled exception)
Comment 10 Ludovic Henry 2017-02-21 14:12:55 UTC
Created attachment 19896 [details]
Comment 11 Ludovic Henry 2017-02-21 14:23:45 UTC
I updated the attached test case to reflect your comment. I can now reproduce your bug on 4.8, but not on master. I cannot guarantee the fix is going to make it into 4.8, but to mitigate the bug, you can simply call `if (!process.HasExited) return;` at the beginning of the `Exited` callback.

You were right in both the race and the swallowed exception. I will investigate the behaviour on .NET for the excepted unhandled exception, and fix Mono accordingly. I will open up a bug for that so we can track this different issues separately.

Thank you!
Comment 12 Ludovic Henry 2017-02-21 18:51:37 UTC
The updated test on master: https://github.com/mono/mono/pull/4409
The fix for the unhandled exception: https://github.com/mono/mono/pull/4408