Commit d210f733f8

Andrew Kelley <andrew@ziglang.org>
2024-10-23 22:47:44
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.
master
1 parent 6bf52b0
Changed files (1)
lib
std
Progress.zig
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 {