🍒[5.10][Distributed] Destroy decoded parameters after executeDistributedTarget #70808
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@swift-ci please test |
ktoso
changed the title
We recently fixed a similar issue where the objects created from decodeNextArgument in a distributed thunk were not properly destroyed. The fix was in swiftlang#65952 In that fix, we missed handling of Indirect_In_Guaranteed which also need to be destroyed, this patch corrects that omission. Resolves swiftlang#70004 Resolves rdar://119943672 Resolves rdar://119446012
ktoso
force-pushed
the
pick-fix-dist-param-leak
branch
from
d9055bf to
899c89e
Compare
|
@swift-ci please test |
airspeedswift
approved these changes
Jan 10, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description: We recently fixed a similar issue where the objects created from decodeNextArgument in a distributed thunk were not properly destroyed. The fix was in #65952
In that fix, we missed handling of
Indirect_In_Guaranteedwhich also need to be destroyed, this patch corrects that omission.Risk: Low, impacts only distributed methods, and only struct arguments. The specific shape of fix was previously done already, but was missing for one of the cases of parameter ownership kind.
Impact: The problem manifests in deinitialization not running as expected for decoded arguments. The memory for those parameters freed but destroy logic is not run on those. This results in memory leak detection triggering for distributed methods. All distributed actor adopters are impacted.
Review by: @jckarter @xedin
Testing: CI testing, added test coverage for remaining cases; Confirmed two separate reproducers are fixed.
Original PR: #70806
Radar: rdar://119943672 rdar://119446012
Resolves #70004