Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

std.Progress: fix race assertion failure #20269

Merged
merged 2 commits into from
Jun 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions lib/std/Progress.zig
Original file line number Diff line number Diff line change
Expand Up @@ -734,7 +734,7 @@ const Serialized = struct {
const Buffer = struct {
parents: [node_storage_buffer_len]Node.Parent,
storage: [node_storage_buffer_len]Node.Storage,
map: [node_storage_buffer_len]Node.Index,
map: [node_storage_buffer_len]Node.OptionalIndex,

parents_copy: [node_storage_buffer_len]Node.Parent,
storage_copy: [node_storage_buffer_len]Node.Storage,
Expand All @@ -753,9 +753,11 @@ fn serialize(serialized_buffer: *Serialized.Buffer) Serialized {
// Iterate all of the nodes and construct a serializable copy of the state that can be examined
// without atomics.
const end_index = @atomicLoad(u32, &global_progress.node_end_index, .monotonic);
const node_parents = global_progress.node_parents[0..end_index];
const node_storage = global_progress.node_storage[0..end_index];
for (node_parents, node_storage, 0..) |*parent_ptr, *storage_ptr, i| {
for (
global_progress.node_parents[0..end_index],
global_progress.node_storage[0..end_index],
serialized_buffer.map[0..end_index],
) |*parent_ptr, *storage_ptr, *map| {
var begin_parent = @atomicLoad(Node.Parent, parent_ptr, .acquire);
while (begin_parent != .unused) {
const dest_storage = &serialized_buffer.storage[serialized_len];
Expand All @@ -766,12 +768,19 @@ fn serialize(serialized_buffer: *Serialized.Buffer) Serialized {
if (begin_parent == end_parent) {
any_ipc = any_ipc or (dest_storage.getIpcFd() != null);
serialized_buffer.parents[serialized_len] = begin_parent;
serialized_buffer.map[i] = @enumFromInt(serialized_len);
map.* = @enumFromInt(serialized_len);
serialized_len += 1;
break;
}

begin_parent = end_parent;
} else {
// A node may be freed during the execution of this loop, causing
// there to be a parent reference to a nonexistent node. Without
// this assignment, this would lead to the map entry containing
// stale data. By assigning none, the child node with the bad
// parent pointer will be harmlessly omitted from the tree.
map.* = .none;
}
}

Expand Down
17 changes: 6 additions & 11 deletions tools/update_cpu_features.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1038,8 +1038,7 @@ pub fn main() anyerror!void {
var zig_src_dir = try fs.cwd().openDir(zig_src_root, .{});
defer zig_src_dir.close();

var progress = std.Progress{};
const root_progress = progress.start("", llvm_targets.len);
const root_progress = std.Progress.start(.{ .estimated_total_items = llvm_targets.len });
defer root_progress.end();

if (builtin.single_threaded) {
Expand Down Expand Up @@ -1074,7 +1073,7 @@ const Job = struct {
llvm_tblgen_exe: []const u8,
llvm_src_root: []const u8,
zig_src_dir: std.fs.Dir,
root_progress: *std.Progress.Node,
root_progress: std.Progress.Node,
llvm_target: LlvmTarget,
};

Expand All @@ -1085,12 +1084,10 @@ fn processOneTarget(job: Job) anyerror!void {
defer arena_state.deinit();
const arena = arena_state.allocator();

var progress_node = job.root_progress.start(llvm_target.zig_name, 3);
progress_node.activate();
const progress_node = job.root_progress.start(llvm_target.zig_name, 3);
defer progress_node.end();

var tblgen_progress = progress_node.start("invoke llvm-tblgen", 0);
tblgen_progress.activate();
const tblgen_progress = progress_node.start("invoke llvm-tblgen", 0);

const child_args = [_][]const u8{
job.llvm_tblgen_exe,
Expand Down Expand Up @@ -1127,16 +1124,14 @@ fn processOneTarget(job: Job) anyerror!void {
},
};

var json_parse_progress = progress_node.start("parse JSON", 0);
json_parse_progress.activate();
const json_parse_progress = progress_node.start("parse JSON", 0);

const parsed = try json.parseFromSlice(json.Value, arena, json_text, .{});
defer parsed.deinit();
const root_map = &parsed.value.object;
json_parse_progress.end();

var render_progress = progress_node.start("render zig code", 0);
render_progress.activate();
const render_progress = progress_node.start("render zig code", 0);

var features_table = std.StringHashMap(Feature).init(arena);
var all_features = std.ArrayList(Feature).init(arena);
Expand Down