Fix group child tasks not being released #39204
Merged
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
The proper handling of task group child tasks is that:
- if it completes a waiting task immediately, we don't need to retain it
- we just move the value to the waiting task and can destroy the task
- if we need to store the ready task and wait for a waiting task (for a
task that hits `await group.next()`) then we need to retain the ready
task.
- as the waiting task arrives, we move the value from the ready task
to the waiting task, and swift_release the ready task -- it will now
be destroyed safely.
(cherry picked from commit d4ebc58)
If we didn't do this (and we didn't), the tasks get released as we perform the next() impl, and move the value from the ready task to the waiting task. Then, the ready task gets destroyed. But as the task group exists, it performs a cancelAll() and that iterates over all records. Those records were not removed previously (!!!) which meant we were pointing at now deallocated tasks. Previously this worked because we didn't deallocate the tasks, so they leaked, but we didn't crash. With the memory leak fixed, this began to crash since we'd attempt to cancel already destroyed tasks. Solution: - Remove task records whenever they complete a waiting task. - This can ONLY be done by the "group owning task" itself, becuause the contract of ONLY this task being allowed to modify records. o It MUST NOT be done by the completing tasks as they complete, as it would race with the owning task modifying this linked list of child tasks in the group record. (cherry picked from commit f336404)
|
@swift-ci please test |
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.
Explanation: Task groups were failing to remove release their child tasks, which led to memory leaks.
Scope: Affects new code using Swift's Concurrency model that uses task groups.
Radar/SR Issue: https://bugs.swift.org/browse/SR-14973 / rdar://81130259
Risk: Low.
Reviewed By: Doug Gregor / nominated on behalf of Konrad Malawski
Testing: PR testing and CI on main, including new tests.
Original PR: #39158