Skip to content

Commit a90c721

Browse files
kavonDougGregor
authored andcommitted
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 69e80a1)
1 parent c1776ee commit a90c721

File tree

1 file changed

+5
-5
lines changed

1 file changed

+5
-5
lines changed

stdlib/public/Concurrency/TaskGroup.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -394,14 +394,14 @@ class TaskGroupImpl: public TaskGroupTaskStatusRecord {
394394
///
395395
/// This is used to atomically perform a waiting task completion.
396396
bool statusCompletePendingReadyWaiting(GroupStatus &old) {
397-
return status.compare_exchange_weak(
397+
return status.compare_exchange_strong(
398398
old.status, old.completingPendingReadyWaiting().status,
399399
/*success*/ std::memory_order_relaxed,
400400
/*failure*/ std::memory_order_relaxed);
401401
}
402402

403403
bool statusCompletePendingReady(GroupStatus &old) {
404-
return status.compare_exchange_weak(
404+
return status.compare_exchange_strong(
405405
old.status, old.completingPendingReady().status,
406406
/*success*/ std::memory_order_relaxed,
407407
/*failure*/ std::memory_order_relaxed);
@@ -588,7 +588,7 @@ void TaskGroupImpl::offer(AsyncTask *completedTask, AsyncContext *context) {
588588
assert(assumed.pendingTasks() && "offered to group with no pending tasks!");
589589
// We are the "first" completed task to arrive,
590590
// and since there is a task waiting we immediately claim and complete it.
591-
if (waitQueue.compare_exchange_weak(
591+
if (waitQueue.compare_exchange_strong(
592592
waitingTask, nullptr,
593593
/*success*/ std::memory_order_release,
594594
/*failure*/ std::memory_order_acquire) &&
@@ -755,7 +755,7 @@ PollResult TaskGroupImpl::poll(AsyncTask *waitingTask) {
755755

756756
auto assumedStatus = assumed.status;
757757
auto newStatus = TaskGroupImpl::GroupStatus{assumedStatus};
758-
if (status.compare_exchange_weak(
758+
if (status.compare_exchange_strong(
759759
assumedStatus, newStatus.completingPendingReadyWaiting().status,
760760
/*success*/ std::memory_order_relaxed,
761761
/*failure*/ std::memory_order_acquire)) {
@@ -821,7 +821,7 @@ PollResult TaskGroupImpl::poll(AsyncTask *waitingTask) {
821821
waitingTask->flagAsSuspended();
822822
}
823823
// Put the waiting task at the beginning of the wait queue.
824-
if (waitQueue.compare_exchange_weak(
824+
if (waitQueue.compare_exchange_strong(
825825
waitHead, waitingTask,
826826
/*success*/ std::memory_order_release,
827827
/*failure*/ std::memory_order_acquire)) {

0 commit comments

Comments
 (0)