Skip to content

Commit a9d7613

Browse files
Applied review comments
1 parent a81d563 commit a9d7613

File tree

1 file changed

+9
-6
lines changed

1 file changed

+9
-6
lines changed

stdlib/public/Concurrency/Actor.cpp

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1032,16 +1032,18 @@ class DefaultActorImpl : public HeapObject {
10321032

10331033
struct Padding {
10341034
enum {
1035-
// Available space
1036-
x1 = (int)(sizeof(void *) * NumWords_DefaultActor),
1035+
// Max object size
1036+
x1 = (int)(sizeof(HeapObject) + sizeof(void *) * NumWords_DefaultActor),
10371037
// ... subtracting size of footer
10381038
x2 = x1 - (int)sizeof(Footer),
10391039
// ... ensuring footer is aligned
10401040
x3 = x2 - (int)(x2 % alignof(Footer)),
10411041
// ... subtracting size of header
10421042
x4 = x3 - (int)sizeof(Header),
10431043
// ... subtracting padding before header
1044-
size = x4 - (int)((alignof(Header) - sizeof(HeapObject) % alignof(Header)) % alignof(Header))
1044+
x5 = x4 - (int)((alignof(Header) - sizeof(HeapObject) % alignof(Header)) % alignof(Header)),
1045+
// ... subtracting size of HeapObject
1046+
size = x5 - (int)sizeof(HeapObject),
10451047
};
10461048
};
10471049
char padding[Padding::size];
@@ -1579,17 +1581,18 @@ void DefaultActorImpl::destroy() {
15791581
#if SWIFT_CONCURRENCY_ACTORS_AS_LOCKS
15801582
// TODO (rokhinip): Do something to assert that the lock is unowned
15811583
#else
1582-
auto oldState = _status().load(std::memory_order_relaxed);
1584+
auto oldState = _status().load(std::memory_order_acquire);
15831585
// Tasks on an actor are supposed to keep the actor alive until they start
15841586
// running and we can only get here if ref count of the object = 0 which means
15851587
// there should be no more tasks enqueued on the actor.
1586-
// TODO: It is not safe to access prioritisedJobs here, right?
15871588
assert(!oldState.getFirstUnprioritisedJob() && "actor has queued jobs at destruction");
15881589

15891590
if (oldState.isIdle()) {
1591+
assert(_prioritisedJobs().empty() && "actor has queued jobs at destruction");
15901592
return;
15911593
}
15921594
assert(oldState.isRunning() && "actor scheduled but not running at destruction");
1595+
// In running state we cannot safely access prioritisedJobs to assert that it is empty.
15931596
#endif
15941597
}
15951598

@@ -1684,7 +1687,7 @@ retry:;
16841687
assert(oldState.getMaxPriority() == JobPriority::Unspecified);
16851688
assert(!oldState.getFirstUnprioritisedJob());
16861689
// We cannot assert here that prioritisedJobs is empty,
1687-
// because lock is not held yet. Raise a flag to after getting the lock.
1690+
// because lock is not held yet. Raise a flag to assert after getting the lock.
16881691
assertNoJobs = true;
16891692
}
16901693

0 commit comments

Comments
 (0)