Skip to content

Commit 11b5747

Browse files
andrewrkandrewrk
authored
Merge pull request #13983 from squeek502/windows-childprocess-exec-retry
`ChildProcess.spawnWindows`: `PATH` search fixes + optimizations
2 parents 90477e5 + 9e8ac2b commit 11b5747

File tree

2 files changed

+103
-44
lines changed

2 files changed

+103
-44
lines changed

‎lib/std/child_process.zig

+81-44
Original file line numberDiff line numberDiff line change
@@ -965,56 +965,93 @@ pub const ChildProcess = struct {
965965
const cmd_line_w = try unicode.utf8ToUtf16LeWithNull(self.allocator, cmd_line);
966966
defer self.allocator.free(cmd_line_w);
967967

968-
windowsCreateProcess(app_path_w.ptr, cmd_line_w.ptr, envp_ptr, cwd_w_ptr, &siStartInfo, &piProcInfo) catch |no_path_err| {
969-
if (no_path_err != error.FileNotFound) return no_path_err;
970-
971-
var free_path = true;
972-
const PATH = process.getEnvVarOwned(self.allocator, "PATH") catch |err| switch (err) {
973-
error.EnvironmentVariableNotFound => blk: {
974-
free_path = false;
975-
break :blk "";
976-
},
977-
else => |e| return e,
978-
};
979-
defer if (free_path) self.allocator.free(PATH);
980-
981-
var free_path_ext = true;
982-
const PATHEXT = process.getEnvVarOwned(self.allocator, "PATHEXT") catch |err| switch (err) {
983-
error.EnvironmentVariableNotFound => blk: {
984-
free_path_ext = false;
985-
break :blk "";
986-
},
987-
else => |e| return e,
988-
};
989-
defer if (free_path_ext) self.allocator.free(PATHEXT);
990-
991-
const app_name = self.argv[0];
992-
993-
var it = mem.tokenize(u8, PATH, ";");
994-
retry: while (it.next()) |search_path| {
995-
const path_no_ext = try fs.path.join(self.allocator, &[_][]const u8{ search_path, app_name });
996-
defer self.allocator.free(path_no_ext);
997-
998-
var ext_it = mem.tokenize(u8, PATHEXT, ";");
999-
while (ext_it.next()) |app_ext| {
1000-
const joined_path = try mem.concat(self.allocator, u8, &[_][]const u8{ path_no_ext, app_ext });
1001-
defer self.allocator.free(joined_path);
968+
exec: {
969+
windowsCreateProcess(app_path_w.ptr, cmd_line_w.ptr, envp_ptr, cwd_w_ptr, &siStartInfo, &piProcInfo) catch |no_path_err| {
970+
switch (no_path_err) {
971+
error.FileNotFound, error.InvalidExe => {},
972+
else => |e| return e,
973+
}
1002974

1003-
const joined_path_w = try unicode.utf8ToUtf16LeWithNull(self.allocator, joined_path);
1004-
defer self.allocator.free(joined_path_w);
975+
const PATH: [:0]const u16 = std.os.getenvW(unicode.utf8ToUtf16LeStringLiteral("PATH")) orelse &[_:0]u16{};
976+
const PATHEXT: [:0]const u16 = std.os.getenvW(unicode.utf8ToUtf16LeStringLiteral("PATHEXT")) orelse &[_:0]u16{};
977+
978+
var path_buf = std.ArrayListUnmanaged(u16){};
979+
defer path_buf.deinit(self.allocator);
980+
981+
// Try again with PATHEXT's extensions appended
982+
{
983+
try path_buf.appendSlice(self.allocator, app_path_w);
984+
var ext_it = mem.tokenize(u16, PATHEXT, &[_]u16{';'});
985+
while (ext_it.next()) |ext| {
986+
path_buf.shrinkRetainingCapacity(app_path_w.len);
987+
try path_buf.appendSlice(self.allocator, ext);
988+
try path_buf.append(self.allocator, 0);
989+
const path_with_ext = path_buf.items[0 .. path_buf.items.len - 1 :0];
990+
991+
if (windowsCreateProcess(path_with_ext.ptr, cmd_line_w.ptr, envp_ptr, cwd_w_ptr, &siStartInfo, &piProcInfo)) |_| {
992+
break :exec;
993+
} else |err| switch (err) {
994+
error.FileNotFound, error.AccessDenied, error.InvalidExe => {},
995+
else => return err,
996+
}
997+
}
998+
}
1005999

1006-
if (windowsCreateProcess(joined_path_w.ptr, cmd_line_w.ptr, envp_ptr, cwd_w_ptr, &siStartInfo, &piProcInfo)) |_| {
1007-
break :retry;
1000+
// No need to search the PATH if the app path is absolute
1001+
if (fs.path.isAbsoluteWindowsWTF16(app_path_w)) return no_path_err;
1002+
1003+
// app_path_w has the cwd prepended to it if cwd is non-null, so when
1004+
// searching the PATH we should make sure we use the app_name verbatim.
1005+
var app_name_w_needs_free = false;
1006+
const app_name_w = x: {
1007+
if (self.cwd) |_| {
1008+
app_name_w_needs_free = true;
1009+
break :x try unicode.utf8ToUtf16LeWithNull(self.allocator, self.argv[0]);
1010+
} else {
1011+
break :x app_path_w;
1012+
}
1013+
};
1014+
defer if (app_name_w_needs_free) self.allocator.free(app_name_w);
1015+
1016+
var it = mem.tokenize(u16, PATH, &[_]u16{';'});
1017+
while (it.next()) |search_path| {
1018+
path_buf.clearRetainingCapacity();
1019+
const search_path_trimmed = mem.trimRight(u16, search_path, &[_]u16{ '\\', '/' });
1020+
try path_buf.appendSlice(self.allocator, search_path_trimmed);
1021+
try path_buf.append(self.allocator, fs.path.sep);
1022+
const app_name_trimmed = mem.trimLeft(u16, app_name_w, &[_]u16{ '\\', '/' });
1023+
try path_buf.appendSlice(self.allocator, app_name_trimmed);
1024+
try path_buf.append(self.allocator, 0);
1025+
const path_no_ext = path_buf.items[0 .. path_buf.items.len - 1 :0];
1026+
1027+
if (windowsCreateProcess(path_no_ext.ptr, cmd_line_w.ptr, envp_ptr, cwd_w_ptr, &siStartInfo, &piProcInfo)) |_| {
1028+
break :exec;
10081029
} else |err| switch (err) {
1009-
error.FileNotFound => continue,
1010-
error.AccessDenied => continue,
1030+
error.FileNotFound, error.AccessDenied, error.InvalidExe => {},
10111031
else => return err,
10121032
}
1033+
1034+
var ext_it = mem.tokenize(u16, PATHEXT, &[_]u16{';'});
1035+
while (ext_it.next()) |ext| {
1036+
path_buf.shrinkRetainingCapacity(path_no_ext.len);
1037+
try path_buf.appendSlice(self.allocator, ext);
1038+
try path_buf.append(self.allocator, 0);
1039+
const joined_path = path_buf.items[0 .. path_buf.items.len - 1 :0];
1040+
1041+
if (windowsCreateProcess(joined_path.ptr, cmd_line_w.ptr, envp_ptr, cwd_w_ptr, &siStartInfo, &piProcInfo)) |_| {
1042+
break :exec;
1043+
} else |err| switch (err) {
1044+
error.FileNotFound => continue,
1045+
error.AccessDenied => continue,
1046+
error.InvalidExe => continue,
1047+
else => return err,
1048+
}
1049+
}
1050+
} else {
1051+
return no_path_err; // return the original error
10131052
}
1014-
} else {
1015-
return no_path_err; // return the original error
1016-
}
1017-
};
1053+
};
1054+
}
10181055

10191056
if (g_hChildStd_IN_Wr) |h| {
10201057
self.stdin = File{ .handle = h };

‎lib/std/os/windows.zig

+22
Original file line numberDiff line numberDiff line change
@@ -1569,6 +1569,7 @@ pub const CreateProcessError = error{
15691569
AccessDenied,
15701570
InvalidName,
15711571
NameTooLong,
1572+
InvalidExe,
15721573
Unexpected,
15731574
};
15741575

@@ -1603,6 +1604,27 @@ pub fn CreateProcessW(
16031604
.INVALID_PARAMETER => unreachable,
16041605
.INVALID_NAME => return error.InvalidName,
16051606
.FILENAME_EXCED_RANGE => return error.NameTooLong,
1607+
// These are all the system errors that are mapped to ENOEXEC by
1608+
// the undocumented _dosmaperr (old CRT) or __acrt_errno_map_os_error
1609+
// (newer CRT) functions. Their code can be found in crt/src/dosmap.c (old SDK)
1610+
// or urt/misc/errno.cpp (newer SDK) in the Windows SDK.
1611+
.BAD_FORMAT,
1612+
.INVALID_STARTING_CODESEG, // MIN_EXEC_ERROR in errno.cpp
1613+
.INVALID_STACKSEG,
1614+
.INVALID_MODULETYPE,
1615+
.INVALID_EXE_SIGNATURE,
1616+
.EXE_MARKED_INVALID,
1617+
.BAD_EXE_FORMAT,
1618+
.ITERATED_DATA_EXCEEDS_64k,
1619+
.INVALID_MINALLOCSIZE,
1620+
.DYNLINK_FROM_INVALID_RING,
1621+
.IOPL_NOT_ENABLED,
1622+
.INVALID_SEGDPL,
1623+
.AUTODATASEG_EXCEEDS_64k,
1624+
.RING2SEG_MUST_BE_MOVABLE,
1625+
.RELOC_CHAIN_XEEDS_SEGLIM,
1626+
.INFLOOP_IN_RELOC_CHAIN, // MAX_EXEC_ERROR in errno.cpp
1627+
=> return error.InvalidExe,
16061628
else => |err| return unexpectedError(err),
16071629
}
16081630
}

0 commit comments

Comments
 (0)