Skip to content

Commit 55b7187

Browse files
committed
link: fix obvious race condition
Did you know that allocators reuse addresses? If not, then don't feel bad, because apparently I don't either! This dumb mistake was probably responsible for the CI failures on `master` yesterday.
1 parent 121d620 commit 55b7187

File tree

2 files changed

+7
-6
lines changed

2 files changed

+7
-6
lines changed

src/Zcu/PerThread.zig

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4395,7 +4395,7 @@ pub fn runCodegen(pt: Zcu.PerThread, func_index: InternPool.Index, air: *Air, ou
43954395
};
43964396
// release `out.value` with this store; synchronizes with acquire loads in `link`
43974397
out.status.store(if (success) .ready else .failed, .release);
4398-
zcu.comp.link_task_queue.mirReady(zcu.comp, out);
4398+
zcu.comp.link_task_queue.mirReady(zcu.comp, func_index, out);
43994399
if (zcu.pending_codegen_jobs.rmw(.Sub, 1, .monotonic) == 1) {
44004400
// Decremented to 0, so all done.
44014401
zcu.codegen_prog_node.end();

src/link/Queue.zig

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ state: union(enum) {
6464
finished,
6565
/// The link thread is not running or queued, because it is waiting for this MIR to be populated.
6666
/// Once codegen completes, it must call `mirReady` which will restart the link thread.
67-
wait_for_mir: *ZcuTask.LinkFunc.SharedMir,
67+
wait_for_mir: InternPool.Index,
6868
},
6969

7070
/// In the worst observed case, MIR is around 50 times as large as AIR. More typically, the ratio is
@@ -113,15 +113,15 @@ pub fn start(q: *Queue, comp: *Compilation) void {
113113

114114
/// Called by codegen workers after they have populated a `ZcuTask.LinkFunc.SharedMir`. If the link
115115
/// thread was waiting for this MIR, it can resume.
116-
pub fn mirReady(q: *Queue, comp: *Compilation, mir: *ZcuTask.LinkFunc.SharedMir) void {
116+
pub fn mirReady(q: *Queue, comp: *Compilation, func_index: InternPool.Index, mir: *ZcuTask.LinkFunc.SharedMir) void {
117117
// We would like to assert that `mir` is not pending, but that would race with a worker thread
118118
// potentially freeing it.
119119
{
120120
q.mutex.lock();
121121
defer q.mutex.unlock();
122122
switch (q.state) {
123123
.finished, .running => return,
124-
.wait_for_mir => |wait_for| if (wait_for != mir) return,
124+
.wait_for_mir => |wait_for| if (wait_for != func_index) return,
125125
}
126126
// We were waiting for `mir`, so we will restart the linker thread.
127127
q.state = .running;
@@ -171,7 +171,7 @@ pub fn enqueueZcu(q: *Queue, comp: *Compilation, task: ZcuTask) Allocator.Error!
171171
}
172172
// Restart the linker thread, unless it would immediately be blocked
173173
if (task == .link_func and task.link_func.mir.status.load(.acquire) == .pending) {
174-
q.state = .{ .wait_for_mir = task.link_func.mir };
174+
q.state = .{ .wait_for_mir = task.link_func.func };
175175
return;
176176
}
177177
q.state = .running;
@@ -248,7 +248,7 @@ fn flushTaskQueue(tid: usize, q: *Queue, comp: *Compilation) void {
248248
defer q.mutex.unlock();
249249
if (status_ptr.load(.acquire) != .pending) break :pending;
250250
// We will stop for now, and get restarted once this MIR is ready.
251-
q.state = .{ .wait_for_mir = task.link_func.mir };
251+
q.state = .{ .wait_for_mir = task.link_func.func };
252252
q.flush_safety.unlock();
253253
return;
254254
}
@@ -273,6 +273,7 @@ const std = @import("std");
273273
const assert = std.debug.assert;
274274
const Allocator = std.mem.Allocator;
275275
const Compilation = @import("../Compilation.zig");
276+
const InternPool = @import("../InternPool.zig");
276277
const link = @import("../link.zig");
277278
const PrelinkTask = link.PrelinkTask;
278279
const ZcuTask = link.ZcuTask;

0 commit comments

Comments
 (0)