Skip to content

Commit 7e7f0b3

Browse files
committed
win: almost fix race detecting ESRCH in uv_kill
libuv/libuv#4341
1 parent d802308 commit 7e7f0b3

File tree

4 files changed

+73
-2
lines changed

4 files changed

+73
-2
lines changed

patches/node/.patches

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,3 +41,4 @@ cli_remove_deprecated_v8_flag.patch
4141
build_restore_clang_as_default_compiler_on_macos.patch
4242
fix_-wextra-semi_errors_in_nghttp2_helper_h.patch
4343
fix_remove_harmony-import-assertions_from_node_cc.patch
44+
win_almost_fix_race_detecting_esrch_in_uv_kill.patch

patches/node/build_add_gn_build_files.patch

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1187,7 +1187,7 @@ index 65d0e1be42f0a85418491ebb548278cf431aa6a0..d4a31342f1c6107b029394c6e1d00a1d
11871187
except Exception as e:
11881188
print(str(e))
11891189
diff --git a/unofficial.gni b/unofficial.gni
1190-
index c3b311e4a7f5444b07d4d7028d4621806959804e..c238b6853e6f96ac7d0133b55d6c7d3d1cc2ad3d 100644
1190+
index c3b311e4a7f5444b07d4d7028d4621806959804e..de6ff5548ca5282199b7d85c11941c1fa351a9d9 100644
11911191
--- a/unofficial.gni
11921192
+++ b/unofficial.gni
11931193
@@ -139,6 +139,7 @@ template("node_gn_build") {

patches/node/fix_handle_boringssl_and_openssl_incompatibilities.patch

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -586,7 +586,7 @@ index 9e656a2815045aa5da7eb267708c03058be9f362..600e0850f01e01024414d42b25605f25
586586
#endif
587587

588588
diff --git a/unofficial.gni b/unofficial.gni
589-
index c238b6853e6f96ac7d0133b55d6c7d3d1cc2ad3d..f449fe43ff25f05f69e8d16c70001cac93d9716c 100644
589+
index de6ff5548ca5282199b7d85c11941c1fa351a9d9..3d8b7957e791ce2fa2a8d0937a87b6010087803d 100644
590590
--- a/unofficial.gni
591591
+++ b/unofficial.gni
592592
@@ -145,7 +145,6 @@ template("node_gn_build") {
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
2+
From: Santiago Gimeno <[email protected]>
3+
Date: Tue, 5 Mar 2024 14:54:59 +0100
4+
Subject: win: almost fix race detecting ESRCH in uv_kill
5+
6+
It might happen that only using `WaitForSingleObject()` with timeout 0
7+
could return WAIT_TIMEOUT as the process might not have been signaled
8+
yet. To improve things, first use `GetExitCodeProcess()` and check
9+
that `status` is not `STILL_ACTIVE`. Then, to cover for the case that the exit
10+
code was actually `STILL_ACTIVE` use `WaitForSingleObject()`. This could
11+
still be prone to the race condition but only for that case.
12+
13+
diff --git a/deps/uv/src/win/process.c b/deps/uv/src/win/process.c
14+
index 4e94dee90e13eede63d8e97ddc9992726f874ea9..f46f34289e8e7d3a2af914d89e6164b751a3e47d 100644
15+
--- a/deps/uv/src/win/process.c
16+
+++ b/deps/uv/src/win/process.c
17+
@@ -1308,16 +1308,34 @@ static int uv__kill(HANDLE process_handle, int signum) {
18+
/* Unconditionally terminate the process. On Windows, killed processes
19+
* normally return 1. */
20+
int err;
21+
+ DWORD status;
22+
23+
if (TerminateProcess(process_handle, 1))
24+
return 0;
25+
26+
- /* If the process already exited before TerminateProcess was called,.
27+
+ /* If the process already exited before TerminateProcess was called,
28+
* TerminateProcess will fail with ERROR_ACCESS_DENIED. */
29+
err = GetLastError();
30+
- if (err == ERROR_ACCESS_DENIED &&
31+
- WaitForSingleObject(process_handle, 0) == WAIT_OBJECT_0) {
32+
- return UV_ESRCH;
33+
+ if (err == ERROR_ACCESS_DENIED) {
34+
+ /* First check using GetExitCodeProcess() with status different from
35+
+ * STILL_ACTIVE (259). This check can be set incorrectly by the process,
36+
+ * though that is uncommon. */
37+
+ if (GetExitCodeProcess(process_handle, &status) &&
38+
+ status != STILL_ACTIVE) {
39+
+ return UV_ESRCH;
40+
+ }
41+
+
42+
+ /* But the process could have exited with code == STILL_ACTIVE, use then
43+
+ * WaitForSingleObject with timeout zero. This is prone to a race
44+
+ * condition as it could return WAIT_TIMEOUT because the handle might
45+
+ * not have been signaled yet.That would result in returning the wrong
46+
+ * error code here (UV_EACCES instead of UV_ESRCH), but we cannot fix
47+
+ * the kernel synchronization issue that TerminateProcess is
48+
+ * inconsistent with WaitForSingleObject with just the APIs available to
49+
+ * us in user space. */
50+
+ if (WaitForSingleObject(process_handle, 0) == WAIT_OBJECT_0) {
51+
+ return UV_ESRCH;
52+
+ }
53+
}
54+
55+
return uv_translate_sys_error(err);
56+
@@ -1325,6 +1343,14 @@ static int uv__kill(HANDLE process_handle, int signum) {
57+
58+
case 0: {
59+
/* Health check: is the process still alive? */
60+
+ DWORD status;
61+
+
62+
+ if (!GetExitCodeProcess(process_handle, &status))
63+
+ return uv_translate_sys_error(GetLastError());
64+
+
65+
+ if (status != STILL_ACTIVE)
66+
+ return UV_ESRCH;
67+
+
68+
switch (WaitForSingleObject(process_handle, 0)) {
69+
case WAIT_OBJECT_0:
70+
return UV_ESRCH;

0 commit comments

Comments
 (0)