Skip to content

Commit 8921285

Browse files
Applied review comments
1 parent df48aee commit 8921285

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
@@ -881,16 +881,18 @@ class DefaultActorImpl : public HeapObject {
881881

882882
struct Padding {
883883
enum {
884-
// Available space
885-
x1 = (int)(sizeof(void *) * NumWords_DefaultActor),
884+
// Max object size
885+
x1 = (int)(sizeof(HeapObject) + sizeof(void *) * NumWords_DefaultActor),
886886
// ... subtracting size of footer
887887
x2 = x1 - (int)sizeof(Footer),
888888
// ... ensuring footer is aligned
889889
x3 = x2 - (int)(x2 % alignof(Footer)),
890890
// ... subtracting size of header
891891
x4 = x3 - (int)sizeof(Header),
892892
// ... subtracting padding before header
893-
size = x4 - (int)((alignof(Header) - sizeof(HeapObject) % alignof(Header)) % alignof(Header))
893+
x5 = x4 - (int)((alignof(Header) - sizeof(HeapObject) % alignof(Header)) % alignof(Header)),
894+
// ... subtracting size of HeapObject
895+
size = x5 - (int)sizeof(HeapObject),
894896
};
895897
};
896898
char padding[Padding::size];
@@ -1428,17 +1430,18 @@ void DefaultActorImpl::destroy() {
14281430
#if SWIFT_CONCURRENCY_ACTORS_AS_LOCKS
14291431
// TODO (rokhinip): Do something to assert that the lock is unowned
14301432
#else
1431-
auto oldState = _status().load(std::memory_order_relaxed);
1433+
auto oldState = _status().load(std::memory_order_acquire);
14321434
// Tasks on an actor are supposed to keep the actor alive until they start
14331435
// running and we can only get here if ref count of the object = 0 which means
14341436
// there should be no more tasks enqueued on the actor.
1435-
// TODO: It is not safe to access prioritisedJobs here, right?
14361437
assert(!oldState.getFirstUnprioritisedJob() && "actor has queued jobs at destruction");
14371438

14381439
if (oldState.isIdle()) {
1440+
assert(_prioritisedJobs().empty() && "actor has queued jobs at destruction");
14391441
return;
14401442
}
14411443
assert(oldState.isRunning() && "actor scheduled but not running at destruction");
1444+
// In running state we cannot safely access prioritisedJobs to assert that it is empty.
14421445
#endif
14431446
}
14441447

@@ -1533,7 +1536,7 @@ retry:;
15331536
assert(oldState.getMaxPriority() == JobPriority::Unspecified);
15341537
assert(!oldState.getFirstUnprioritisedJob());
15351538
// We cannot assert here that prioritisedJobs is empty,
1536-
// because lock is not held yet. Raise a flag to after getting the lock.
1539+
// because lock is not held yet. Raise a flag to assert after getting the lock.
15371540
assertNoJobs = true;
15381541
}
15391542

0 commit comments

Comments
 (0)