From d210f733f80731487f8f83caf621e04506f50bb4 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Wed, 23 Oct 2024 13:47:44 -0700 Subject: [PATCH] std.Progress: fix data race In end(), the freelist pointer is owned so the bare store would be ok. However, there is a load in start() that can happen at the same time, if another start() and end() pair grabs that same index. I don't think this fixes #21663 actually because even if the data race corrupts the value for `next`, the cmpxchg protects the value from being stored there. --- lib/std/Progress.zig | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/std/Progress.zig b/lib/std/Progress.zig index 7e24f25c73..513a939367 100644 --- a/lib/std/Progress.zig +++ b/lib/std/Progress.zig @@ -188,7 +188,8 @@ pub const Node = struct { var opt_free_index = @atomicLoad(Node.OptionalIndex, freelist_head, .seq_cst); while (opt_free_index.unwrap()) |free_index| { const freelist_ptr = freelistByIndex(free_index); - opt_free_index = @cmpxchgWeak(Node.OptionalIndex, freelist_head, opt_free_index, freelist_ptr.*, .seq_cst, .seq_cst) orelse { + const next = @atomicLoad(Node.OptionalIndex, freelist_ptr, .seq_cst); + opt_free_index = @cmpxchgWeak(Node.OptionalIndex, freelist_head, opt_free_index, next, .seq_cst, .seq_cst) orelse { // We won the allocation race. return init(free_index, parent, name, estimated_total_items); }; @@ -249,7 +250,7 @@ pub const Node = struct { const freelist_head = &global_progress.node_freelist_first; var first = @atomicLoad(Node.OptionalIndex, freelist_head, .seq_cst); while (true) { - freelistByIndex(index).* = first; + @atomicStore(Node.OptionalIndex, freelistByIndex(index), first, .seq_cst); first = @cmpxchgWeak(Node.OptionalIndex, freelist_head, first, index.toOptional(), .seq_cst, .seq_cst) orelse break; } } else {