[PATCH v4 0/1] MR3908: wineserver: Report non-zero exit code for abnormal process termination

[Adam Rehn (＠adamrehn)]

**Overview:**

Wine currently reports an exit code of zero for processes that terminate due to either of the following conditions:

- The Linux process that represents the Windows process receives a signal such as `SIGKILL` (e.g. due to a user manually killing the process, or the Linux OOM killer targeting the process due to memory pressure)

- The process fails to start because the image is a 32-bit executable and Wine has only been built with 64-bit support

Both of these scenarios represent failures, and so a non-zero exit code is appropriate. This patch ensures that an exit code of 1 is reported for these abnormal process termination edge cases.

**Underlying cause:**

The following logic flow occurs in the Wine server for typical process termination:

1. When a `process` object is created by the Wine server, its `exit_code` field is [set to an initial value](https://gitlab.winehq.org/wine/wine/-/blob/wine-8.16/server/process.c?ref_ty...) of `STILL_ACTIVE`, and the `exit_code` field in each of its corresponding `thread` objects is [set to a default value of zero](https://gitlab.winehq.org/wine/wine/-/blob/wine-8.16/server/thread.c?ref_typ...).

2. When a client process terminates, it sends a `terminate_process` protocol request to the server, and the handler [passes the specified exit code](https://gitlab.winehq.org/wine/wine/-/blob/wine-8.16/server/process.c?ref_ty...) to the `terminate_process()` function. This function sets the `is_terminating` field of the process object [to a value of 1](https://gitlab.winehq.org/wine/wine/-/blob/wine-8.16/server/process.c?ref_ty...), and if the exit code is non-zero then it also [copies its value to each of the thread objects](https://gitlab.winehq.org/wine/wine/-/blob/wine-8.16/server/process.c?ref_ty...) for the process.

3. The client process closes the pipe that it had been using to communicate with the server.

4. The server [detects the pipe close event](https://gitlab.winehq.org/wine/wine/-/blob/wine-8.16/server/process.c?ref_ty...) and calls the `kill_process()` function, specifying a value of zero for the `violent_death` parameter.

5. The logic in the `kill_process()` function identifies this as a [normal termination on pipe close](https://gitlab.winehq.org/wine/wine/-/blob/wine-8.16/server/process.c?ref_ty...), and then subsequently calls the `kill_thread()` function to [kill each of the threads for the process](https://gitlab.winehq.org/wine/wine/-/blob/wine-8.16/server/process.c?ref_ty...).

6. The `kill_thread()` function [then calls](https://gitlab.winehq.org/wine/wine/-/blob/wine-8.16/server/thread.c?ref_typ...) `remove_process_thread()`, which [copies the exit code from the last thread object](https://gitlab.winehq.org/wine/wine/-/blob/wine-8.16/server/process.c?ref_ty...) to the `exit_code` field of the process object.

7. The final value of the `exit_code` field is [reported when responding](https://gitlab.winehq.org/wine/wine/-/blob/wine-8.16/server/process.c?ref_ty...) to `get_process_info` protocol requests, [as sent by](https://gitlab.winehq.org/wine/wine/-/blob/wine-8.16/dlls/ntdll/unix/process...) `NtQueryInformationProcess()`.

The abnormal process termination scenarios listed earlier will skip the `terminate_process` protocol request and immediately close the pipe. As a result, the `is_terminating` field of the process object will retain its default value of zero, and the `exit_code` field of each thread for the process will also remain at the default value of zero.

Since the logic in `process_poll_event()` always treats a pipe close event as a non-violent termination, `remove_process_thread()` will simply copy the exit code value of zero from the last thread into the process object, and this will then be reported when querying the process exit code.

**Fix details:**

The fix modifies `process_poll_event()` to check the value of the `is_terminating` field of the process object, and to treat a pipe close event as a violent termination if the value of the field is zero. This triggers the alternative code path in `kill_process()` that calls `terminate_process()` and [specifies an exit code value of 1](https://gitlab.winehq.org/wine/wine/-/blob/wine-8.16/server/process.c?ref_ty...), which is copied to the thread objects and subsequently to the process object.

In my testing, only the abnormal process termination scenarios listed above result in the value of the `is_terminating` field being zero when the pipe is closed. In all other scenarios, I have observed the value to be 1 when the pipe close event is detected. Non-abnormal scenarios tested include ordinary process completion with both zero and non-zero exit codes, crashes in Windows processes due to unhandled exceptions, and ending processes via the Wine implementation of Task Manager.

--
  v4: wineserver: Report non-zero exit code for abnormal process termination
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3908

[PATCH v4 1/1] wineserver: Report non-zero exit code for abnormal process termination

[Adam Rehn]

From: Adam Rehn <adam(a)tensorworks.com.au>

Signed-off-by: Adam Rehn <adam(a)tensorworks.com.au>
---
 server/process.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/server/process.c b/server/process.c
index b7c7df38e44..f0dcd2d0e49 100644
--- a/server/process.c
+++ b/server/process.c
@@ -864,7 +864,7 @@ static void process_poll_event( struct fd *fd, int event )
     struct process *process = get_fd_user( fd );
     assert( process->obj.ops == &process_ops );
 
-    if (event & (POLLERR | POLLHUP)) kill_process( process, 0 );
+    if (event & (POLLERR | POLLHUP)) kill_process( process, !process->is_terminating );
     else if (event & POLLIN) receive_fd( process );
 }
 
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/3908

[PATCH v4 1/1] wineserver: Report non-zero exit code for abnormal process termination

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how
to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=144840

Your paranoid android.


=== debian11b (64 bit WoW report) ===

dmime:
dmime.c:1950: Test failed: got mtTime 26, expected 24
dmime.c:1962: Test failed: got mtTime 51, expected 49
dmime.c:1974: Test failed: got mtTime 76, expected 74
dmime.c:1986: Test failed: got mtTime 126, expected 124
dmime.c:1998: Test failed: got mtTime 651, expected 649

Report validation errors:
ntdll:exception is missing some failure messages