Skip to content

Commit d93edad

Browse files
authored
Merge pull request #13993 from squeek502/windows-childprocess-perf
`spawnWindows`: Improve worst-case performance considerably + tests
2 parents 6ed0910 + 0cbc59f commit d93edad

File tree

8 files changed

+589
-95
lines changed

8 files changed

+589
-95
lines changed

lib/std/child_process.zig

Lines changed: 365 additions & 82 deletions
Large diffs are not rendered by default.

lib/std/os.zig

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1944,19 +1944,7 @@ pub fn getenvW(key: [*:0]const u16) ?[:0]const u16 {
19441944
while (ptr[i] != 0) : (i += 1) {}
19451945
const this_value = ptr[value_start..i :0];
19461946

1947-
const key_string_bytes = @intCast(u16, key_slice.len * 2);
1948-
const key_string = windows.UNICODE_STRING{
1949-
.Length = key_string_bytes,
1950-
.MaximumLength = key_string_bytes,
1951-
.Buffer = @intToPtr([*]u16, @ptrToInt(key)),
1952-
};
1953-
const this_key_string_bytes = @intCast(u16, this_key.len * 2);
1954-
const this_key_string = windows.UNICODE_STRING{
1955-
.Length = this_key_string_bytes,
1956-
.MaximumLength = this_key_string_bytes,
1957-
.Buffer = this_key.ptr,
1958-
};
1959-
if (windows.ntdll.RtlEqualUnicodeString(&key_string, &this_key_string, windows.TRUE) == windows.TRUE) {
1947+
if (windows.eqlIgnoreCaseWTF16(key_slice, this_key)) {
19601948
return this_value;
19611949
}
19621950

lib/std/os/windows.zig

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1624,6 +1624,9 @@ pub fn CreateProcessW(
16241624
.RING2SEG_MUST_BE_MOVABLE,
16251625
.RELOC_CHAIN_XEEDS_SEGLIM,
16261626
.INFLOOP_IN_RELOC_CHAIN, // MAX_EXEC_ERROR in errno.cpp
1627+
// This one is not mapped to ENOEXEC but it is possible, for example
1628+
// when calling CreateProcessW on a plain text file with a .exe extension
1629+
.EXE_MACHINE_TYPE_MISMATCH,
16271630
=> return error.InvalidExe,
16281631
else => |err| return unexpectedError(err),
16291632
}
@@ -1824,6 +1827,23 @@ pub fn nanoSecondsToFileTime(ns: i128) FILETIME {
18241827
};
18251828
}
18261829

1830+
/// Compares two WTF16 strings using RtlEqualUnicodeString
1831+
pub fn eqlIgnoreCaseWTF16(a: []const u16, b: []const u16) bool {
1832+
const a_bytes = @intCast(u16, a.len * 2);
1833+
const a_string = UNICODE_STRING{
1834+
.Length = a_bytes,
1835+
.MaximumLength = a_bytes,
1836+
.Buffer = @intToPtr([*]u16, @ptrToInt(a.ptr)),
1837+
};
1838+
const b_bytes = @intCast(u16, b.len * 2);
1839+
const b_string = UNICODE_STRING{
1840+
.Length = b_bytes,
1841+
.MaximumLength = b_bytes,
1842+
.Buffer = @intToPtr([*]u16, @ptrToInt(b.ptr)),
1843+
};
1844+
return ntdll.RtlEqualUnicodeString(&a_string, &b_string, TRUE) == TRUE;
1845+
}
1846+
18271847
pub const PathSpace = struct {
18281848
data: [PATH_MAX_WIDE:0]u16,
18291849
len: usize,
@@ -3682,6 +3702,20 @@ pub const RTL_DRIVE_LETTER_CURDIR = extern struct {
36823702

36833703
pub const PPS_POST_PROCESS_INIT_ROUTINE = ?*const fn () callconv(.C) void;
36843704

3705+
pub const FILE_DIRECTORY_INFORMATION = extern struct {
3706+
NextEntryOffset: ULONG,
3707+
FileIndex: ULONG,
3708+
CreationTime: LARGE_INTEGER,
3709+
LastAccessTime: LARGE_INTEGER,
3710+
LastWriteTime: LARGE_INTEGER,
3711+
ChangeTime: LARGE_INTEGER,
3712+
EndOfFile: LARGE_INTEGER,
3713+
AllocationSize: LARGE_INTEGER,
3714+
FileAttributes: ULONG,
3715+
FileNameLength: ULONG,
3716+
FileName: [1]WCHAR,
3717+
};
3718+
36853719
pub const FILE_BOTH_DIR_INFORMATION = extern struct {
36863720
NextEntryOffset: ULONG,
36873721
FileIndex: ULONG,

lib/std/os/windows/kernel32.zig

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,8 @@ pub extern "kernel32" fn GetEnvironmentStringsW() callconv(WINAPI) ?[*:0]u16;
177177

178178
pub extern "kernel32" fn GetEnvironmentVariableW(lpName: LPWSTR, lpBuffer: [*]u16, nSize: DWORD) callconv(WINAPI) DWORD;
179179

180+
pub extern "kernel32" fn SetEnvironmentVariableW(lpName: LPCWSTR, lpValue: ?LPCWSTR) callconv(WINAPI) BOOL;
181+
180182
pub extern "kernel32" fn GetExitCodeProcess(hProcess: HANDLE, lpExitCode: *DWORD) callconv(WINAPI) BOOL;
181183

182184
pub extern "kernel32" fn GetFileSizeEx(hFile: HANDLE, lpFileSize: *LARGE_INTEGER) callconv(WINAPI) BOOL;

test/standalone.zig

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,10 @@ pub fn addCases(cases: *tests.StandaloneContext) void {
6363
cases.addBuildFile("test/standalone/load_dynamic_library/build.zig", .{});
6464
}
6565

66+
if (builtin.os.tag == .windows) {
67+
cases.addBuildFile("test/standalone/windows_spawn/build.zig", .{});
68+
}
69+
6670
cases.addBuildFile("test/standalone/c_compiler/build.zig", .{
6771
.build_modes = true,
6872
.cross_targets = true,
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
const Builder = @import("std").build.Builder;
2+
3+
pub fn build(b: *Builder) void {
4+
const mode = b.standardReleaseOptions();
5+
6+
const hello = b.addExecutable("hello", "hello.zig");
7+
hello.setBuildMode(mode);
8+
9+
const main = b.addExecutable("main", "main.zig");
10+
main.setBuildMode(mode);
11+
const run = main.run();
12+
run.addArtifactArg(hello);
13+
14+
const test_step = b.step("test", "Test it");
15+
test_step.dependOn(&run.step);
16+
}
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
const std = @import("std");
2+
3+
pub fn main() !void {
4+
const stdout = std.io.getStdOut().writer();
5+
try stdout.writeAll("hello from exe\n");
6+
}
Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
const std = @import("std");
2+
const windows = std.os.windows;
3+
const utf16Literal = std.unicode.utf8ToUtf16LeStringLiteral;
4+
5+
pub fn main() anyerror!void {
6+
var gpa = std.heap.GeneralPurposeAllocator(.{}){};
7+
defer if (gpa.deinit()) @panic("found memory leaks");
8+
const allocator = gpa.allocator();
9+
10+
var it = try std.process.argsWithAllocator(allocator);
11+
defer it.deinit();
12+
_ = it.next() orelse unreachable; // skip binary name
13+
const hello_exe_cache_path = it.next() orelse unreachable;
14+
15+
var tmp = std.testing.tmpDir(.{});
16+
defer tmp.cleanup();
17+
18+
const tmp_absolute_path = try tmp.dir.realpathAlloc(allocator, ".");
19+
defer allocator.free(tmp_absolute_path);
20+
const tmp_absolute_path_w = try std.unicode.utf8ToUtf16LeWithNull(allocator, tmp_absolute_path);
21+
defer allocator.free(tmp_absolute_path_w);
22+
const cwd_absolute_path = try std.fs.cwd().realpathAlloc(allocator, ".");
23+
defer allocator.free(cwd_absolute_path);
24+
const tmp_relative_path = try std.fs.path.relative(allocator, cwd_absolute_path, tmp_absolute_path);
25+
defer allocator.free(tmp_relative_path);
26+
27+
// Clear PATH
28+
std.debug.assert(std.os.windows.kernel32.SetEnvironmentVariableW(
29+
utf16Literal("PATH"),
30+
null,
31+
) == windows.TRUE);
32+
33+
// Set PATHEXT to something predictable
34+
std.debug.assert(std.os.windows.kernel32.SetEnvironmentVariableW(
35+
utf16Literal("PATHEXT"),
36+
utf16Literal(".COM;.EXE;.BAT;.CMD;.JS"),
37+
) == windows.TRUE);
38+
39+
// No PATH, so it should fail to find anything not in the cwd
40+
try testExecError(error.FileNotFound, allocator, "something_missing");
41+
42+
std.debug.assert(std.os.windows.kernel32.SetEnvironmentVariableW(
43+
utf16Literal("PATH"),
44+
tmp_absolute_path_w,
45+
) == windows.TRUE);
46+
47+
// Move hello.exe into the tmp dir which is now added to the path
48+
try std.fs.cwd().copyFile(hello_exe_cache_path, tmp.dir, "hello.exe", .{});
49+
50+
// with extension should find the .exe (case insensitive)
51+
try testExec(allocator, "HeLLo.exe", "hello from exe\n");
52+
// without extension should find the .exe (case insensitive)
53+
try testExec(allocator, "heLLo", "hello from exe\n");
54+
55+
// now add a .bat
56+
try tmp.dir.writeFile("hello.bat", "@echo hello from bat");
57+
// and a .cmd
58+
try tmp.dir.writeFile("hello.cmd", "@echo hello from cmd");
59+
60+
// with extension should find the .bat (case insensitive)
61+
try testExec(allocator, "heLLo.bat", "hello from bat\r\n");
62+
// with extension should find the .cmd (case insensitive)
63+
try testExec(allocator, "heLLo.cmd", "hello from cmd\r\n");
64+
// without extension should find the .exe (since its first in PATHEXT)
65+
try testExec(allocator, "heLLo", "hello from exe\n");
66+
67+
// now rename the exe to not have an extension
68+
try tmp.dir.rename("hello.exe", "hello");
69+
70+
// with extension should now fail
71+
try testExecError(error.FileNotFound, allocator, "hello.exe");
72+
// without extension should succeed (case insensitive)
73+
try testExec(allocator, "heLLo", "hello from exe\n");
74+
75+
try tmp.dir.makeDir("something");
76+
try tmp.dir.rename("hello", "something/hello.exe");
77+
78+
const relative_path_no_ext = try std.fs.path.join(allocator, &.{ tmp_relative_path, "something/hello" });
79+
defer allocator.free(relative_path_no_ext);
80+
81+
// Giving a full relative path to something/hello should work
82+
try testExec(allocator, relative_path_no_ext, "hello from exe\n");
83+
// But commands with path separators get excluded from PATH searching, so this will fail
84+
try testExecError(error.FileNotFound, allocator, "something/hello");
85+
86+
// Now that .BAT is the first PATHEXT that should be found, this should succeed
87+
try testExec(allocator, "heLLo", "hello from bat\r\n");
88+
89+
// Add a hello.exe that is not a valid executable
90+
try tmp.dir.writeFile("hello.exe", "invalid");
91+
92+
// Trying to execute it with extension will give InvalidExe. This is a special
93+
// case for .EXE extensions, where if they ever try to get executed but they are
94+
// invalid, that gets treated as a fatal error wherever they are found and InvalidExe
95+
// is returned immediately.
96+
try testExecError(error.InvalidExe, allocator, "hello.exe");
97+
// Same thing applies to the command with no extension--even though there is a
98+
// hello.bat that could be executed, it should stop after it tries executing
99+
// hello.exe and getting InvalidExe.
100+
try testExecError(error.InvalidExe, allocator, "hello");
101+
102+
// If we now rename hello.exe to have no extension, it will behave differently
103+
try tmp.dir.rename("hello.exe", "hello");
104+
105+
// Now, trying to execute it without an extension should treat InvalidExe as recoverable
106+
// and skip over it and find hello.bat and execute that
107+
try testExec(allocator, "hello", "hello from bat\r\n");
108+
109+
// If we rename the invalid exe to something else
110+
try tmp.dir.rename("hello", "goodbye");
111+
// Then we should now get FileNotFound when trying to execute 'goodbye',
112+
// since that is what the original error will be after searching for 'goodbye'
113+
// in the cwd. It will try to execute 'goodbye' from the PATH but the InvalidExe error
114+
// should be ignored in this case.
115+
try testExecError(error.FileNotFound, allocator, "goodbye");
116+
117+
// Now let's set the tmp dir as the cwd and set the path only include the "something" sub dir
118+
try tmp.dir.setAsCwd();
119+
const something_subdir_abs_path = try std.mem.concatWithSentinel(allocator, u16, &.{ tmp_absolute_path_w, utf16Literal("\\something") }, 0);
120+
defer allocator.free(something_subdir_abs_path);
121+
122+
std.debug.assert(std.os.windows.kernel32.SetEnvironmentVariableW(
123+
utf16Literal("PATH"),
124+
something_subdir_abs_path,
125+
) == windows.TRUE);
126+
127+
// Now trying to execute goodbye should give error.InvalidExe since it's the original
128+
// error that we got when trying within the cwd
129+
try testExecError(error.InvalidExe, allocator, "goodbye");
130+
131+
// hello should still find the .bat
132+
try testExec(allocator, "hello", "hello from bat\r\n");
133+
134+
// If we rename something/hello.exe to something/goodbye.exe
135+
try tmp.dir.rename("something/hello.exe", "something/goodbye.exe");
136+
// And try to execute goodbye, then the one in something should be found
137+
// since the one in cwd is an invalid executable
138+
try testExec(allocator, "goodbye", "hello from exe\n");
139+
140+
// If we use an absolute path to execute the invalid goodbye
141+
const goodbye_abs_path = try std.mem.join(allocator, "\\", &.{ tmp_absolute_path, "goodbye" });
142+
defer allocator.free(goodbye_abs_path);
143+
// then the PATH should not be searched and we should get InvalidExe
144+
try testExecError(error.InvalidExe, allocator, goodbye_abs_path);
145+
}
146+
147+
fn testExecError(err: anyerror, allocator: std.mem.Allocator, command: []const u8) !void {
148+
return std.testing.expectError(err, testExec(allocator, command, ""));
149+
}
150+
151+
fn testExec(allocator: std.mem.Allocator, command: []const u8, expected_stdout: []const u8) !void {
152+
var result = try std.ChildProcess.exec(.{
153+
.allocator = allocator,
154+
.argv = &[_][]const u8{command},
155+
});
156+
defer allocator.free(result.stdout);
157+
defer allocator.free(result.stderr);
158+
159+
try std.testing.expectEqualStrings("", result.stderr);
160+
try std.testing.expectEqualStrings(expected_stdout, result.stdout);
161+
}

0 commit comments

Comments
 (0)