Skip to content

Commit 519e60c

Browse files
committed
Allow concurrent modifications to the ActiveTaskStatus while another
thread has the task status record lock. Today, if a thread is holding the StatusRecordLock, then no other modification of the task status is possible - including a thread starting to execute the task or stopping execution of the task. However, the TaskStatusRecordLock is really about protecting the linked list that is maintained in the ActiveTaskStatus. As such, other operations which don't need to look at that linked list of us records really shouldn't have to block on the StatusRecordLock. This change allows for concurrent modification of the veTaskStatus while the TaskStatusRecordLock is held. In particular, a task can cancelled, escalated, start and stop running, all while another ad is holding onto the task's StatusRecordLock. In the event of cancellation and escalation, the task's StatusRecordLock must be n in order to propagate cancellation and escalation to its child tasks is not needed to cancel or escalate the task itself. Radar-Id: rdar://problem/76127624
1 parent ff10907 commit 519e60c

File tree

3 files changed

+109
-151
lines changed

3 files changed

+109
-151
lines changed

include/swift/ABI/Task.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,6 @@ class AsyncTask : public Job {
297297
/// Generally this should be done immediately after updating
298298
/// ActiveTask.
299299
void flagAsRunning();
300-
void flagAsRunning_slow();
301300

302301
/// Flag that this task is now suspended. This can update the
303302
/// priority stored in the job flags if the priority hsa been
@@ -306,7 +305,6 @@ class AsyncTask : public Job {
306305
/// somewhere. TODO: record where the task is enqueued if
307306
/// possible.
308307
void flagAsSuspended();
309-
void flagAsSuspended_slow();
310308

311309
/// Flag that this task is now completed. This normally does not do anything
312310
/// but can be used to locally insert logging.

stdlib/public/Concurrency/TaskPrivate.h

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -353,7 +353,7 @@ class alignas(sizeof(void*) * 2) ActiveTaskStatus {
353353
#endif
354354
}
355355

356-
/// Is there an active lock on the cancellation information?
356+
/// Is there a lock on the linked list of status records?
357357
bool isStatusRecordLocked() const { return Flags & IsStatusRecordLocked; }
358358
ActiveTaskStatus withLockingRecord(TaskStatusRecord *lockRecord) const {
359359
assert(!isStatusRecordLocked());
@@ -365,6 +365,19 @@ class alignas(sizeof(void*) * 2) ActiveTaskStatus {
365365
#endif
366366
}
367367

368+
ActiveTaskStatus withoutLockingRecord() const {
369+
assert(isStatusRecordLocked());
370+
assert(Record->getKind() == TaskStatusRecordKind::Private_RecordLock);
371+
372+
// Remove the lock record, and put the next one as the head
373+
auto newRecord = Record->Parent;
374+
#if SWIFT_CONCURRENCY_ENABLE_PRIORITY_ESCALATION
375+
return ActiveTaskStatus(newRecord, Flags & ~IsStatusRecordLocked, ExecutionLock);
376+
#else
377+
return ActiveTaskStatus(newRecord, Flags & ~IsStatusRecordLocked);
378+
#endif
379+
}
380+
368381
JobPriority getStoredPriority() const {
369382
return JobPriority(Flags & PriorityMask);
370383
}
@@ -543,13 +556,6 @@ inline void AsyncTask::flagAsRunning() {
543556
auto oldStatus = _private().Status.load(std::memory_order_relaxed);
544557
while (true) {
545558
assert(!oldStatus.isRunning());
546-
if (oldStatus.isStatusRecordLocked()) {
547-
flagAsRunning_slow();
548-
adoptTaskVoucher(this);
549-
swift_task_enterThreadLocalContext(
550-
(char *)&_private().ExclusivityAccessSet[0]);
551-
return;
552-
}
553559

554560
auto newStatus = oldStatus.withRunning(true);
555561
if (newStatus.isStoredPriorityEscalated()) {
@@ -559,8 +565,8 @@ inline void AsyncTask::flagAsRunning() {
559565
}
560566

561567
if (_private().Status.compare_exchange_weak(oldStatus, newStatus,
562-
std::memory_order_relaxed,
563-
std::memory_order_relaxed)) {
568+
/* success */ std::memory_order_relaxed,
569+
/* failure */ std::memory_order_relaxed)) {
564570
newStatus.traceStatusChanged(this);
565571
adoptTaskVoucher(this);
566572
swift_task_enterThreadLocalContext(
@@ -575,13 +581,6 @@ inline void AsyncTask::flagAsSuspended() {
575581
auto oldStatus = _private().Status.load(std::memory_order_relaxed);
576582
while (true) {
577583
assert(oldStatus.isRunning());
578-
if (oldStatus.isStatusRecordLocked()) {
579-
flagAsSuspended_slow();
580-
swift_task_exitThreadLocalContext(
581-
(char *)&_private().ExclusivityAccessSet[0]);
582-
restoreTaskVoucher(this);
583-
return;
584-
}
585584

586585
auto newStatus = oldStatus.withRunning(false);
587586
if (newStatus.isStoredPriorityEscalated()) {
@@ -591,15 +590,16 @@ inline void AsyncTask::flagAsSuspended() {
591590
}
592591

593592
if (_private().Status.compare_exchange_weak(oldStatus, newStatus,
594-
std::memory_order_relaxed,
595-
std::memory_order_relaxed)) {
596-
newStatus.traceStatusChanged(this);
597-
swift_task_exitThreadLocalContext(
598-
(char *)&_private().ExclusivityAccessSet[0]);
599-
restoreTaskVoucher(this);
600-
return;
593+
/* success */std::memory_order_relaxed,
594+
/* failure */std::memory_order_relaxed)) {
595+
break;
601596
}
602597
}
598+
599+
newStatus.traceStatusChanged(this);
600+
swift_task_exitThreadLocalContext((char *)&_private().ExclusivityAccessSet[0]);
601+
restoreTaskVoucher(this);
602+
return;
603603
}
604604

605605
// READ ME: This is not a dead function! Do not remove it! This is a function

0 commit comments

Comments
 (0)