Skip to content

Commit 00c8113

Browse files
committed
stage2: improve handling of the generated file builtin.zig
All Zig code is eligible to `@import("builtin")` which is mapped to a generated file, build.zig, based on the target and other settings. Zig invocations which share the same target settings will generate the same builtin.zig file and thus the path to builtin.zig is in a shared cache folder, and different projects can sometimes use the same file. Before this commit, this led to race conditions where multiple invocations of `zig` would race to write this file. If one process wanted to *read* the file while the other process *wrote* the file, the reading process could observe a truncated or partially written builtin.zig file. This commit makes the following improvements: - limitations: - avoid clobbering the inode, mtime in the hot path - avoid creating a partially written file - builtin.zig needs to be on disk for debug info / stack trace purposes - don't mark the task as complete until the file is finished being populated (possibly by an external process) - strategy: - create the `@import("builtin")` `Module.File` during the AstGen work, based on generating the contents in memory rather than loading from disk. - write builtin.zig in a separate task that doesn't have to complete until the end of the AstGen work queue so that it can be done in parallel with everything else. - when writing the file, first stat the file path. If it exists, we are done. - otherwise, write the file to a temp file in the same directory and atomically rename it into place (clobbering the inode, mtime in the cold path). - summary: - all limitations respected - hot path: one stat() syscall that happens in a worker thread This required adding a missing function to the standard library: `std.fs.Dir.statFile`. In this commit, it does open() and then fstat() which is two syscalls. It should be improved in a future commit to only make one. Fixes #9439.
1 parent a3d9cd1 commit 00c8113

File tree

3 files changed

+124
-27
lines changed

3 files changed

+124
-27
lines changed

lib/std/fs.zig

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2170,6 +2170,16 @@ pub const Dir = struct {
21702170
return file.stat();
21712171
}
21722172

2173+
pub const StatFileError = File.OpenError || StatError;
2174+
2175+
// TODO: improve this to use the fstatat syscall instead of making 2 syscalls here.
2176+
pub fn statFile(self: Dir, sub_path: []const u8) StatFileError!File.Stat {
2177+
var file = try self.openFile(sub_path, .{});
2178+
defer file.close();
2179+
2180+
return file.stat();
2181+
}
2182+
21732183
pub const ChmodError = File.ChmodError;
21742184

21752185
/// Changes the mode of the directory.

src/Compilation.zig

Lines changed: 42 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -2152,15 +2152,6 @@ pub fn performAllTheWork(self: *Compilation) error{ TimerUnsupported, OutOfMemor
21522152
defer main_progress_node.end();
21532153
if (self.color == .off) progress.terminal = null;
21542154

2155-
// If we need to write out builtin.zig, it needs to be done before starting
2156-
// the AstGen tasks.
2157-
if (self.bin_file.options.module) |mod| {
2158-
if (mod.job_queued_update_builtin_zig) {
2159-
mod.job_queued_update_builtin_zig = false;
2160-
try self.updateBuiltinZigFile(mod);
2161-
}
2162-
}
2163-
21642155
// Here we queue up all the AstGen tasks first, followed by C object compilation.
21652156
// We wait until the AstGen tasks are all completed before proceeding to the
21662157
// (at least for now) single-threaded main work queue. However, C object compilation
@@ -2185,6 +2176,21 @@ pub fn performAllTheWork(self: *Compilation) error{ TimerUnsupported, OutOfMemor
21852176
self.astgen_wait_group.reset();
21862177
defer self.astgen_wait_group.wait();
21872178

2179+
// builtin.zig is handled specially for two reasons:
2180+
// 1. to avoid race condition of zig processes truncating each other's builtin.zig files
2181+
// 2. optimization; in the hot path it only incurs a stat() syscall, which happens
2182+
// in the `astgen_wait_group`.
2183+
if (self.bin_file.options.module) |mod| {
2184+
if (mod.job_queued_update_builtin_zig) {
2185+
mod.job_queued_update_builtin_zig = false;
2186+
2187+
self.astgen_wait_group.start();
2188+
try self.thread_pool.spawn(workerUpdateBuiltinZigFile, .{
2189+
self, mod, &self.astgen_wait_group,
2190+
});
2191+
}
2192+
}
2193+
21882194
while (self.astgen_work_queue.readItem()) |file| {
21892195
self.astgen_wait_group.start();
21902196
try self.thread_pool.spawn(workerAstGenFile, .{
@@ -2748,6 +2754,8 @@ fn workerAstGenFile(
27482754
extra_index = item.end;
27492755

27502756
const import_path = file.zir.nullTerminatedString(item.data.name);
2757+
// `@import("builtin")` is handled specially.
2758+
if (mem.eql(u8, import_path, "builtin")) continue;
27512759

27522760
const import_result = blk: {
27532761
comp.mutex.lock();
@@ -2775,6 +2783,29 @@ fn workerAstGenFile(
27752783
}
27762784
}
27772785

2786+
fn workerUpdateBuiltinZigFile(
2787+
comp: *Compilation,
2788+
mod: *Module,
2789+
wg: *WaitGroup,
2790+
) void {
2791+
defer wg.finish();
2792+
2793+
mod.populateBuiltinFile() catch |err| {
2794+
const dir_path: []const u8 = mod.zig_cache_artifact_directory.path orelse ".";
2795+
2796+
comp.mutex.lock();
2797+
defer comp.mutex.unlock();
2798+
2799+
comp.setMiscFailure(.write_builtin_zig, "unable to write builtin.zig to {s}: {s}", .{
2800+
dir_path, @errorName(err),
2801+
}) catch |oom| switch (oom) {
2802+
error.OutOfMemory => log.err("unable to write builtin.zig to {s}: {s}", .{
2803+
dir_path, @errorName(err),
2804+
}),
2805+
};
2806+
};
2807+
}
2808+
27782809
fn workerCheckEmbedFile(
27792810
comp: *Compilation,
27802811
embed_file: *Module.EmbedFile,
@@ -4046,22 +4077,6 @@ fn wantBuildLibUnwindFromSource(comp: *Compilation) bool {
40464077
comp.bin_file.options.object_format != .c;
40474078
}
40484079

4049-
fn updateBuiltinZigFile(comp: *Compilation, mod: *Module) Allocator.Error!void {
4050-
const tracy_trace = trace(@src());
4051-
defer tracy_trace.end();
4052-
4053-
const source = try comp.generateBuiltinZigSource(comp.gpa);
4054-
defer comp.gpa.free(source);
4055-
4056-
mod.zig_cache_artifact_directory.handle.writeFile("builtin.zig", source) catch |err| {
4057-
const dir_path: []const u8 = mod.zig_cache_artifact_directory.path orelse ".";
4058-
try comp.setMiscFailure(.write_builtin_zig, "unable to write builtin.zig to {s}: {s}", .{
4059-
dir_path,
4060-
@errorName(err),
4061-
});
4062-
};
4063-
}
4064-
40654080
fn setMiscFailure(
40664081
comp: *Compilation,
40674082
tag: MiscTask,
@@ -4084,7 +4099,7 @@ pub fn dump_argv(argv: []const []const u8) void {
40844099
std.debug.print("{s}\n", .{argv[argv.len - 1]});
40854100
}
40864101

4087-
pub fn generateBuiltinZigSource(comp: *Compilation, allocator: Allocator) Allocator.Error![]u8 {
4102+
pub fn generateBuiltinZigSource(comp: *Compilation, allocator: Allocator) Allocator.Error![:0]u8 {
40884103
const tracy_trace = trace(@src());
40894104
defer tracy_trace.end();
40904105

@@ -4290,7 +4305,7 @@ pub fn generateBuiltinZigSource(comp: *Compilation, allocator: Allocator) Alloca
42904305
}
42914306
}
42924307

4293-
return buffer.toOwnedSlice();
4308+
return buffer.toOwnedSliceSentinel(0);
42944309
}
42954310

42964311
pub fn updateSubCompilation(sub_compilation: *Compilation) !void {

src/Module.zig

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2967,6 +2967,78 @@ fn updateZirRefs(gpa: Allocator, file: *File, old_zir: Zir) !void {
29672967
}
29682968
}
29692969

2970+
pub fn populateBuiltinFile(mod: *Module) !void {
2971+
const tracy = trace(@src());
2972+
defer tracy.end();
2973+
2974+
const comp = mod.comp;
2975+
const pkg_and_file = blk: {
2976+
comp.mutex.lock();
2977+
defer comp.mutex.unlock();
2978+
2979+
const builtin_pkg = mod.main_pkg.table.get("builtin").?;
2980+
const result = try mod.importPkg(builtin_pkg);
2981+
break :blk .{
2982+
.file = result.file,
2983+
.pkg = builtin_pkg,
2984+
};
2985+
};
2986+
const file = pkg_and_file.file;
2987+
const builtin_pkg = pkg_and_file.pkg;
2988+
const gpa = mod.gpa;
2989+
file.source = try comp.generateBuiltinZigSource(gpa);
2990+
file.source_loaded = true;
2991+
2992+
if (builtin_pkg.root_src_directory.handle.statFile(builtin_pkg.root_src_path)) |stat| {
2993+
if (stat.size != file.source.len) {
2994+
const full_path = try builtin_pkg.root_src_directory.join(gpa, &.{
2995+
builtin_pkg.root_src_path,
2996+
});
2997+
defer gpa.free(full_path);
2998+
2999+
log.warn(
3000+
"the cached file '{s}' had the wrong size. Expected {d}, found {d}. " ++
3001+
"Overwriting with correct file contents now",
3002+
.{ full_path, file.source.len, stat.size },
3003+
);
3004+
3005+
try writeBuiltinFile(file, builtin_pkg);
3006+
} else {
3007+
file.stat_size = stat.size;
3008+
file.stat_inode = stat.inode;
3009+
file.stat_mtime = stat.mtime;
3010+
}
3011+
} else |err| switch (err) {
3012+
error.BadPathName => unreachable, // it's always "builtin.zig"
3013+
error.NameTooLong => unreachable, // it's always "builtin.zig"
3014+
error.PipeBusy => unreachable, // it's not a pipe
3015+
error.WouldBlock => unreachable, // not asking for non-blocking I/O
3016+
3017+
error.FileNotFound => try writeBuiltinFile(file, builtin_pkg),
3018+
3019+
else => |e| return e,
3020+
}
3021+
3022+
file.tree = try std.zig.parse(gpa, file.source);
3023+
file.tree_loaded = true;
3024+
assert(file.tree.errors.len == 0); // builtin.zig must parse
3025+
3026+
file.zir = try AstGen.generate(gpa, file.tree);
3027+
file.zir_loaded = true;
3028+
file.status = .success_zir;
3029+
}
3030+
3031+
pub fn writeBuiltinFile(file: *File, builtin_pkg: *Package) !void {
3032+
var af = try builtin_pkg.root_src_directory.handle.atomicFile(builtin_pkg.root_src_path, .{});
3033+
defer af.deinit();
3034+
try af.file.writeAll(file.source);
3035+
try af.finish();
3036+
3037+
file.stat_size = file.source.len;
3038+
file.stat_inode = 0; // dummy value
3039+
file.stat_mtime = 0; // dummy value
3040+
}
3041+
29703042
pub fn mapOldZirToNew(
29713043
gpa: Allocator,
29723044
old_zir: Zir,

0 commit comments

Comments
 (0)