Skip to content

fix: resolve retain cycle in GutenbergCoverUploadProcessor#25442

Closed
amanjeetsingh150 wants to merge 1 commit intowordpress-mobile:trunkfrom
amanjeetsingh150:fix/gutenberg-cover-upload-processor-leak
Closed

fix: resolve retain cycle in GutenbergCoverUploadProcessor#25442
amanjeetsingh150 wants to merge 1 commit intowordpress-mobile:trunkfrom
amanjeetsingh150:fix/gutenberg-cover-upload-processor-leak

Conversation

@amanjeetsingh150
Copy link
Copy Markdown
Contributor

@amanjeetsingh150 amanjeetsingh150 commented Mar 24, 2026

Summary

Screenshot 2026-03-24 at 4 46 17 PM

Report:

xctestleaks-artifacts.zip

Fixes #25441

The lazy var coverBlockProcessor in GutenbergCoverUploadProcessor created a GutenbergBlockProcessor with a replacer closure that strongly captured self, while self strongly held coverBlockProcessor, forming a retain cycle that prevented deallocation after every cover block upload.

Cycle:

GutenbergCoverUploadProcessor
  → lazy var coverBlockProcessor (strong)
    → GutenbergBlockProcessor.replacer closure (strong)
      → self (GutenbergCoverUploadProcessor) ← cycle back

Fix: add [weak self] to the replacer closure.

Verification

Detected and verified using XCTestLeaks a tool I am working to get leaks from unit tests.

Test Before After
testCoverBlockProcessor leaks=1 leaks=0 ✅
testCoverBlockProcessorWithOtherAttributes leaks=2 leaks=0 ✅
testDeepNestedCoverBlockProcessor leaks=3 leaks=0 ✅
testImageCoverInVideoCoverBlockProcessor leaks=4 leaks=0 ✅
testMultipleCoverBlocksProcessor leaks=5 leaks=0 ✅
testNestedCoverBlockProcessor leaks=5 leaks=0 ✅
testUpdateOuterCoverBlockProcessor leaks=7 leaks=0 ✅
testVideoCoverBlockProcessor leaks=7 leaks=0 ✅
testVideoCoverInImageCoverBlockProcessor leaks=7 leaks=0 ✅

Test plan

  • All GutenbergCoverUploadProcessorTests pass with leaks=0
  • No functional behaviour changed — [weak self] with guard let self maintains the same logic

@sonarqubecloud
Copy link
Copy Markdown

@kean kean self-requested a review March 24, 2026 14:40
@kean kean added this to the 26.9 milestone Mar 24, 2026
Copy link
Copy Markdown
Contributor

@kean kean left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good find, @amanjeetsingh150. Thank you!

@kean kean enabled auto-merge March 24, 2026 14:41
@amanjeetsingh150
Copy link
Copy Markdown
Contributor Author

Hey @kean, I see the auto merge didn't trigger is it because of the checks are stuck? Is there anything I can do from my side to unblock this.

@kean
Copy link
Copy Markdown
Contributor

kean commented Mar 26, 2026

Hold on. There is some protections in place to allow only certain builds to run. I'm going to verify it and merge locally. Thank you.

@kean
Copy link
Copy Markdown
Contributor

kean commented Mar 27, 2026

Hey, I merged the changes via #25455 (the commit attribution is in tact). Thank you.

@kean kean closed this Mar 27, 2026
@amanjeetsingh150
Copy link
Copy Markdown
Contributor Author

Thanks @kean appreciated! 🙌

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.

Bug: [Memory leak] retain cycle in GutenbergCoverUploadProcessor

2 participants