Skip to content

Commit

Permalink
Try to patch node instead of skipping the test
Browse files Browse the repository at this point in the history
  • Loading branch information
nirinchev committed Nov 12, 2024
1 parent cb278bc commit e68db81
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 7 deletions.
4 changes: 4 additions & 0 deletions .evergreen/install-node-source.sh
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ pushd "${NODE_JS_SOURCE_PATH}"
# patch directory
patch -p1 < "${ROOT_DIR}/scripts/nodejs-patches/001-configure-bz2.patch"

if [[ "$NODE_JS_VERSION" == "22"* ]]; then
patch -p1 < "${ROOT_DIR}/scripts/nodejs-patches/005-win-almost-fix-race-detecting-esrch-in-uv-kill.patch"
fi

./configure --prefix "${NODE_JS_INSTALL_DIR}"

ncpu=$(expr $(nproc 2> /dev/null || echo 5) - 1)
Expand Down
7 changes: 0 additions & 7 deletions packages/e2e-tests/test/e2e.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1059,13 +1059,6 @@ describe('e2e', function () {
});

it('reads and runs the vscode extension example playground', async function () {
if (
process.platform === 'win32' &&
process.versions.node.startsWith('22')
) {
// This test fails on Windows with node 22 likely due to https://github.com/nodejs/node/issues/51766
return this.skip();
}
createReadStream(
path.resolve(__dirname, 'fixtures', 'exampleplayground.js')
).pipe(shell.process.stdin);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
diff --git a/deps/uv/src/win/process.c b/deps/uv/src/win/process.c
index 4e94dee90e13eede63d8e97ddc9992726f874ea9..f46f34289e8e7d3a2af914d89e6164b751a3e47d 100644
--- a/deps/uv/src/win/process.c
+++ b/deps/uv/src/win/process.c
@@ -1308,16 +1308,34 @@ static int uv__kill(HANDLE process_handle, int signum) {
/* Unconditionally terminate the process. On Windows, killed processes
* normally return 1. */
int err;
+ DWORD status;

if (TerminateProcess(process_handle, 1))
return 0;

- /* If the process already exited before TerminateProcess was called,.
+ /* If the process already exited before TerminateProcess was called,
* TerminateProcess will fail with ERROR_ACCESS_DENIED. */
err = GetLastError();
- if (err == ERROR_ACCESS_DENIED &&
- WaitForSingleObject(process_handle, 0) == WAIT_OBJECT_0) {
- return UV_ESRCH;
+ if (err == ERROR_ACCESS_DENIED) {
+ /* First check using GetExitCodeProcess() with status different from
+ * STILL_ACTIVE (259). This check can be set incorrectly by the process,
+ * though that is uncommon. */
+ if (GetExitCodeProcess(process_handle, &status) &&
+ status != STILL_ACTIVE) {
+ return UV_ESRCH;
+ }
+
+ /* But the process could have exited with code == STILL_ACTIVE, use then
+ * WaitForSingleObject with timeout zero. This is prone to a race
+ * condition as it could return WAIT_TIMEOUT because the handle might
+ * not have been signaled yet.That would result in returning the wrong
+ * error code here (UV_EACCES instead of UV_ESRCH), but we cannot fix
+ * the kernel synchronization issue that TerminateProcess is
+ * inconsistent with WaitForSingleObject with just the APIs available to
+ * us in user space. */
+ if (WaitForSingleObject(process_handle, 0) == WAIT_OBJECT_0) {
+ return UV_ESRCH;
+ }
}

return uv_translate_sys_error(err);
@@ -1325,6 +1343,14 @@ static int uv__kill(HANDLE process_handle, int signum) {

case 0: {
/* Health check: is the process still alive? */
+ DWORD status;
+
+ if (!GetExitCodeProcess(process_handle, &status))
+ return uv_translate_sys_error(GetLastError());
+
+ if (status != STILL_ACTIVE)
+ return UV_ESRCH;
+
switch (WaitForSingleObject(process_handle, 0)) {
case WAIT_OBJECT_0:
return UV_ESRCH;

0 comments on commit e68db81

Please sign in to comment.