Skip to content

Commit 8baf34b

Browse files
committed
Fixing use after free
- Properly synchronizing `finish` with `run` with task counts - Adding sync_wait relacy test for verification - adapting atomic wrappers to account for missing std::atomic_ref in relacy
1 parent 6c1ed99 commit 8baf34b

File tree

4 files changed

+38
-7
lines changed

4 files changed

+38
-7
lines changed

include/stdexec/__detail/__atomic.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ namespace stdexec::__std {
5858
using std::atomic_thread_fence;
5959
using std::atomic_signal_fence;
6060

61-
# if __cpp_lib_atomic_ref >= 2018'06L
61+
#if __cpp_lib_atomic_ref >= 2018'06L && !defined(STDEXEC_RELACY)
6262
using std::atomic_ref;
6363
# else
6464
inline constexpr int __atomic_flag_map[] = {

include/stdexec/__detail/__run_loop.hpp

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727

2828
#include "__atomic.hpp"
2929
#include "stdexec/__detail/__config.hpp"
30-
#include <atomic>
3130
#include <cstddef>
3231

3332
namespace stdexec {
@@ -56,16 +55,21 @@ namespace stdexec {
5655
STDEXEC_ATTRIBUTE(host, device) void finish() noexcept {
5756
// Increment our task count to avoid lifetime issues. This is preventing
5857
// a use-after-free issue if finish is called from a different thread.
59-
__task_count_.fetch_add(1, __std::memory_order_release);
58+
// We increment the task counter by two to avoid the run loop to exit before
59+
// we scheduled the noop task
60+
__task_count_.fetch_add(2, __std::memory_order_release);
6061
if (!__finishing_.exchange(true, __std::memory_order_acq_rel)) {
6162
// push an empty work item to the queue to wake up the consuming thread
6263
// and let it finish.
6364
// The count will be decremented once the tasks executes.
6465
__queue_.push(&__noop_task);
66+
// If the task got pushed, simply subtract one again, the other increment
67+
// happens when the noop task got executed.
68+
__task_count_.fetch_sub(1, __std::memory_order_release);
6569
return;
6670
}
67-
// We are done finishing. Decrement the count, which signals final completion.
68-
__task_count_.fetch_sub(1, __std::memory_order_release);
71+
// We are done finishing. Decrement the count by two, which signals final completion.
72+
__task_count_.fetch_sub(2, __std::memory_order_release);
6973
}
7074

7175
struct __task : __immovable {

test/rrd/Makefile

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
# User-customizable variables:
22
CXX ?= c++
33
CXX_STD ?= c++20
4-
CXXFLAGS ?= -I relacy -I relacy/relacy/fakestd -O1 -std=$(CXX_STD) -I ../../include -I ../../test -g
4+
CXXFLAGS ?= -DSTDEXEC_RELACY -I relacy -I relacy/relacy/fakestd -O1 -std=$(CXX_STD) -I ../../include -I ../../test -g
55
DEPFLAGS ?= -MD -MF $(@).d -MP -MT $(@)
66
build_dir = build
77

88
.SECONDARY:
99

10-
test_programs = split async_scope
10+
test_programs = split async_scope sync_wait
1111

1212
test_exe_files = $(foreach name,$(test_programs),$(build_dir)/$(name))
1313

test/rrd/sync_wait.cpp

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
#include "../../relacy/relacy_std.hpp"
2+
3+
#include <stdexec/execution.hpp>
4+
#include <exec/static_thread_pool.hpp>
5+
6+
namespace ex = stdexec;
7+
8+
struct sync_wait_bg_thread : rl::test_suite<sync_wait_bg_thread, 1> {
9+
static size_t const dynamic_thread_count = 1;
10+
11+
void thread(unsigned) {
12+
exec::static_thread_pool pool{1};
13+
auto sender = ex::schedule(pool.get_scheduler()) | ex::then([] { return 42; });
14+
15+
auto [val] = ex::sync_wait(sender).value();
16+
RL_ASSERT(val == 42);
17+
}
18+
};
19+
20+
auto main() -> int {
21+
rl::test_params p;
22+
p.iteration_count = 50000;
23+
p.execution_depth_limit = 10000;
24+
p.search_type = rl::random_scheduler_type;
25+
rl::simulate<sync_wait_bg_thread>(p);
26+
return 0;
27+
}

0 commit comments

Comments
 (0)