Skip to content

Conversation

@ahoppen
Copy link
Member

@ahoppen ahoppen commented Feb 24, 2025

self.taskTracker.onTaskProgressUpdateText captured self strongly, creating a reference cycle in LLBuildProgressTracker, which caused it to never get deallocated.

@ahoppen
Copy link
Member Author

ahoppen commented Feb 24, 2025

@swift-ci Please test

@ahoppen
Copy link
Member Author

ahoppen commented Feb 24, 2025

@swift-ci Please test

`self.taskTracker.onTaskProgressUpdateText` captured `self` strongly, creating a reference cycle in `LLBuildProgressTracker`, which caused it to never get deallocated.
@ahoppen
Copy link
Member Author

ahoppen commented Feb 26, 2025

@swift-ci Please test

@ahoppen
Copy link
Member Author

ahoppen commented Feb 26, 2025

@swift-ci Please test Windows

@ahoppen ahoppen enabled auto-merge (squash) February 26, 2025 04:25
@ahoppen
Copy link
Member Author

ahoppen commented Feb 26, 2025

@swift-ci Please test macOS

ahoppen added a commit that referenced this pull request Feb 26, 2025
- **Explanation**: `self.taskTracker.onTaskProgressUpdateText` captured
`self` strongly, creating a reference cycle in `LLBuildProgressTracker`,
which caused it to never get deallocated.
- **Scope**: Manifested as a memory leak in SourceKit-LSP
- **Issue**: n/a
- **Original PR**:
#8305
- **Risk**: Low, just captures a delegate weakly
- **Testing**: Verified that this fixes the memory leak we saw
- **Reviewer**: @MaxDesiatov
@ahoppen
Copy link
Member Author

ahoppen commented Feb 27, 2025

@swift-ci Please test macOS

@ahoppen ahoppen merged commit 150b709 into swiftlang:main Feb 27, 2025
5 checks passed
@ahoppen ahoppen deleted the fix-retain-cycle branch February 27, 2025 17:09
bripeticca pushed a commit to bripeticca/swift-package-manager that referenced this pull request Feb 28, 2025
`self.taskTracker.onTaskProgressUpdateText` captured `self` strongly,
creating a reference cycle in `LLBuildProgressTracker`, which caused it
to never get deallocated.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants