From a90c721042eab662df30c3ab42f83e99f523bb83 Mon Sep 17 00:00:00 2001 From: Kavon Farvardin Date: Thu, 18 Nov 2021 16:15:47 -0800 Subject: [PATCH] use `compare_exchange_strong` to protect agianst spurious failures A `compare_exchange_weak` can spuriously return false, regardless of whether a concurrent access happened. This was causing a null-pointer dereference in TaskGroupImpl::poll in a narrow circumstance. The dereference failure only appears when using the `arm64` slice of the runtime library, since Clang will use `ldxr/stxr` for synchronization on such targets. The weak form does not retry on a spurious failure, but the strong version will. resolves rdar://84192672 (cherry picked from commit 69e80a1201636e124451de946dfa7fc255bf05ca) --- stdlib/public/Concurrency/TaskGroup.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/stdlib/public/Concurrency/TaskGroup.cpp b/stdlib/public/Concurrency/TaskGroup.cpp index 1aa10f6e1ed56..6ddc66ceb70e7 100644 --- a/stdlib/public/Concurrency/TaskGroup.cpp +++ b/stdlib/public/Concurrency/TaskGroup.cpp @@ -394,14 +394,14 @@ class TaskGroupImpl: public TaskGroupTaskStatusRecord { /// /// This is used to atomically perform a waiting task completion. bool statusCompletePendingReadyWaiting(GroupStatus &old) { - return status.compare_exchange_weak( + return status.compare_exchange_strong( old.status, old.completingPendingReadyWaiting().status, /*success*/ std::memory_order_relaxed, /*failure*/ std::memory_order_relaxed); } bool statusCompletePendingReady(GroupStatus &old) { - return status.compare_exchange_weak( + return status.compare_exchange_strong( old.status, old.completingPendingReady().status, /*success*/ std::memory_order_relaxed, /*failure*/ std::memory_order_relaxed); @@ -588,7 +588,7 @@ void TaskGroupImpl::offer(AsyncTask *completedTask, AsyncContext *context) { assert(assumed.pendingTasks() && "offered to group with no pending tasks!"); // We are the "first" completed task to arrive, // and since there is a task waiting we immediately claim and complete it. - if (waitQueue.compare_exchange_weak( + if (waitQueue.compare_exchange_strong( waitingTask, nullptr, /*success*/ std::memory_order_release, /*failure*/ std::memory_order_acquire) && @@ -755,7 +755,7 @@ PollResult TaskGroupImpl::poll(AsyncTask *waitingTask) { auto assumedStatus = assumed.status; auto newStatus = TaskGroupImpl::GroupStatus{assumedStatus}; - if (status.compare_exchange_weak( + if (status.compare_exchange_strong( assumedStatus, newStatus.completingPendingReadyWaiting().status, /*success*/ std::memory_order_relaxed, /*failure*/ std::memory_order_acquire)) { @@ -821,7 +821,7 @@ PollResult TaskGroupImpl::poll(AsyncTask *waitingTask) { waitingTask->flagAsSuspended(); } // Put the waiting task at the beginning of the wait queue. - if (waitQueue.compare_exchange_weak( + if (waitQueue.compare_exchange_strong( waitHead, waitingTask, /*success*/ std::memory_order_release, /*failure*/ std::memory_order_acquire)) {