Skip to content

Commit cb5e337

Browse files
authored
fix: lean_task_imp.m_canceled needs to be accessed using atomics (#14127)
This PR fixes a theoretical but not practical race condition on `lean_task_imp.m_canceled` by making it atomic. The reason that this works out with the ordering as intended are similar to #14108
1 parent 74d7aef commit cb5e337

2 files changed

Lines changed: 7 additions & 6 deletions

File tree

src/include/lean/lean.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,8 @@ typedef struct {
236236
struct lean_task * m_head_dep;
237237
struct lean_task * m_next_dep;
238238
unsigned m_prio;
239-
uint8_t m_canceled;
239+
// This field is atomic as we access it both with and without holding the task_manager mutex.
240+
_Atomic(uint8_t) m_canceled;
240241
// If true, task will not be freed until finished
241242
uint8_t m_keep_alive;
242243
uint8_t m_deleted;

src/runtime/object.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -785,8 +785,8 @@ class task_manager {
785785
lean_task_object * it = imp->m_head_dep;
786786
imp->m_closure = nullptr;
787787
imp->m_head_dep = nullptr;
788-
imp->m_canceled = true;
789788
imp->m_deleted = true;
789+
imp->m_canceled.store(true, std::memory_order_relaxed);
790790
lock.unlock();
791791
while (it) {
792792
lean_task_imp* imp = it->m_imp.load(std::memory_order_relaxed);
@@ -911,8 +911,8 @@ class task_manager {
911911
imp->m_head_dep = nullptr;
912912
while (it) {
913913
lean_task_imp* it_imp = it->m_imp.load(std::memory_order_relaxed);
914-
if (imp->m_canceled)
915-
it_imp->m_canceled = true;
914+
if (imp->m_canceled.load(std::memory_order_relaxed))
915+
it_imp->m_canceled.store(true, std::memory_order_relaxed);
916916
lean_task_object * next_it = it_imp->m_next_dep;
917917
it_imp->m_next_dep = nullptr;
918918
if (it_imp->m_deleted) {
@@ -1046,7 +1046,7 @@ class task_manager {
10461046
unique_lock<mutex> lock(m_mutex);
10471047
lean_task_imp* imp = t->m_imp.load(std::memory_order_relaxed);
10481048
if (imp)
1049-
imp->m_canceled = true;
1049+
imp->m_canceled.store(true, std::memory_order_relaxed);
10501050
}
10511051

10521052
bool shutting_down() const {
@@ -1256,7 +1256,7 @@ extern "C" LEAN_EXPORT bool lean_io_check_canceled_core() {
12561256
if (lean_task_object * t = g_current_task_object) {
12571257
lean_task_imp* imp = t->m_imp.load(std::memory_order_relaxed);
12581258
lean_assert(imp); // task is being executed
1259-
return imp->m_canceled || g_task_manager->shutting_down();
1259+
return imp->m_canceled.load(std::memory_order_relaxed) || g_task_manager->shutting_down();
12601260
}
12611261
return false;
12621262
}

0 commit comments

Comments
 (0)