[Distributed] prevent remote distributed actor's from running deinit bodies #60050
Conversation
ktoso
force-pushed
the
wip-deinit-test-da
branch
from
4182dd9 to
fbfe72f
Compare
| auto managedSelf = ManagedValue::forBorrowedRValue(selfValue); | ||
|
|
||
| auto selfTy = F.mapTypeIntoContext(cd->getDeclaredInterfaceType()); | ||
| emitDistributedIfRemoteBranch( |
There was a problem hiding this comment.
For isolated deinit I need this check to live inside deallocating deinit. Because isolation happens in deallocating deinit, and remote proxies don't need isolation. I would really appreciate if you could place the check already in the deallocating deinit, so that I don't need to move it later.
There was a problem hiding this comment.
As I'd like to cherry-pick this over to 5.7 so it gets has a chance to get into the release still, and we're a bit limited in how big "risk" changes we can take there, I'd I'd like to keep this PR as minimal as possible and only focused on fixing the bug. We can refactor and change things around later though.
I'm happy to do that in a follow up PR later on 👍
ktoso
force-pushed
the
wip-deinit-test-da
branch
from
fbfe72f to
60c015d
Compare
ktoso
force-pushed
the
wip-deinit-test-da
branch
from
60c015d to
027c4a6
Compare
|
@swift-ci please smoke test |
Co-authored-by: Kavon Farvardin <[email protected]>
|
@swift-ci please smoke test and merge |
| @@ -554,20 +557,21 @@ SILGenFunction::emitConditionalResignIdentityCall(SILLocation loc, | |||
|
|
|||
| void SILGenFunction::emitDistributedActorClassMemberDestruction( | |||
There was a problem hiding this comment.
emitDistributedActorClassMemberDestruction() and emitConditionalResignIdentityCall() seem to be dead
| B.createBranch(SILLocation(Loc), finishBB); | ||
| } | ||
| } | ||
|
|
||
| emitProfilerIncrement(dd->getTypecheckedBody()); |
There was a problem hiding this comment.
IIUC, this can be emitted into remoteBB after branch instruction, creating incorrect SIL. Should it be after B.emitBlock(deinitBodyBB)?
Would be nice to have a unit test for deinit and profiling.
Deinit bodies should not run for remote distributed actors since they have no state at all, and any such user defined deinit will be wrong.
Resolves: rdar://96980182
Reported in https://forums.swift.org/t/pitch-distributed-actor-runtime/54045/15