From 1b52538dfff68ed07f23cb609a11c9ee7f2656d3 Mon Sep 17 00:00:00 2001 From: Felipe Pena Date: Mon, 24 Mar 2025 17:04:10 -0300 Subject: [PATCH] sync: fix a helgrind false positive, for a data race, on PoolProcessor (#24023) --- vlib/sync/pool/pool.c.v | 22 ++++++++++++++++------ vlib/v/gen/c/assign.v | 3 +++ 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/vlib/sync/pool/pool.c.v b/vlib/sync/pool/pool.c.v index 0d4562fd26..21f6fcff0d 100644 --- a/vlib/sync/pool/pool.c.v +++ b/vlib/sync/pool/pool.c.v @@ -13,7 +13,7 @@ pub struct PoolProcessor { mut: njobs int items []voidptr - results []voidptr + results shared []voidptr ntask u32 // reading/writing to this should be atomic waitgroup sync.WaitGroup shared_context voidptr @@ -88,7 +88,9 @@ pub fn (mut pool PoolProcessor) work_on_pointers(items []voidptr) { } unsafe { pool.thread_contexts = []voidptr{len: items.len} - pool.results = []voidptr{len: items.len} + lock pool.results { + pool.results = []voidptr{len: items.len} + } pool.items = []voidptr{cap: items.len} pool.items << items pool.waitgroup.add(njobs) @@ -115,7 +117,9 @@ fn process_in_thread(mut pool PoolProcessor, task_id int) { if idx >= ilen { break } - pool.results[idx] = cb(mut pool, idx, task_id) + lock pool.results { + pool.results[idx] = cb(mut pool, idx, task_id) + } } pool.waitgroup.done() } @@ -129,14 +133,18 @@ pub fn (pool &PoolProcessor) get_item[T](idx int) T { // get_result - called by the main thread to get a specific result. // Retrieves a type safe instance of the produced result. pub fn (pool &PoolProcessor) get_result[T](idx int) T { - return unsafe { *(&T(pool.results[idx])) } + rlock pool.results { + return unsafe { *(&T(pool.results[idx])) } + } } // get_results - get a list of type safe results in the main thread. pub fn (pool &PoolProcessor) get_results[T]() []T { mut res := []T{cap: pool.results.len} for i in 0 .. pool.results.len { - res << unsafe { *(&T(pool.results[i])) } + rlock pool.results { + res << unsafe { *(&T(pool.results[i])) } + } } return res } @@ -145,7 +153,9 @@ pub fn (pool &PoolProcessor) get_results[T]() []T { pub fn (pool &PoolProcessor) get_results_ref[T]() []&T { mut res := []&T{cap: pool.results.len} for i in 0 .. pool.results.len { - res << unsafe { &T(pool.results[i]) } + rlock pool.results { + res << unsafe { &T(pool.results[i]) } + } } return res } diff --git a/vlib/v/gen/c/assign.v b/vlib/v/gen/c/assign.v index 99479c45e1..ff6616f1fc 100644 --- a/vlib/v/gen/c/assign.v +++ b/vlib/v/gen/c/assign.v @@ -234,6 +234,9 @@ fn (mut g Gen) assign_stmt(node_ ast.AssignStmt) { // we can't just do `.str` since we need the extra data from the string struct // doing `&string` is also not an option since the stack memory with the data will be overwritten g.expr(left0) // node.left[0]) + if first_left_type.has_flag(.shared_f) { + g.write('->val') + } g.writeln('); // free ${type_to_free} on re-assignment2') defer { if af {