Avoid self if be nil in block. #3826
Conversation
WalkthroughThe changes introduce explicit nil checks for self after weak-to-strong reference promotion within completion or handler blocks across several files. This ensures that if self has been deallocated, the block exits early, preventing operations on a nil object and avoiding potential crashes or undefined behavior. Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
SDWebImage/Core/UIButton+WebCache.m (1)
143-144: Inconsistent nil check pattern.This
@strongify(self)doesn't have the same nil check pattern that was applied to the image callback at lines 76-78. For consistency, consider adding the same defensive check:@strongify(self); +if (!self) { + return; +} [self setBackgroundImage:image forState:state];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
SDWebImage/Core/SDAnimatedImageView.m(1 hunks)SDWebImage/Core/SDImageCachesManager.m(5 hunks)SDWebImage/Core/UIButton+WebCache.m(1 hunks)SDWebImage/Core/UIImageView+HighlightedWebCache.m(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Unit Test (macOS)
- GitHub Check: Cocoapods Lint
- GitHub Check: Unit Test (visionOS)
- GitHub Check: Unit Test (tvOS)
- GitHub Check: Cocoapods Demo
🔇 Additional comments (9)
SDWebImage/Core/UIButton+WebCache.m (1)
76-78: Good defensive programming practice.The nil check after
@strongify(self)prevents potential crashes if the object has been deallocated while the block was waiting to execute.SDWebImage/Core/SDAnimatedImageView.m (2)
245-247: Excellent defensive programming.The nil check prevents frame updates on a deallocated object, which is crucial for animation handlers that may execute asynchronously.
254-256: Proper safety check for animation loop handling.The nil check ensures that loop count updates and progressive animation logic only execute when the object is still valid.
SDWebImage/Core/UIImageView+HighlightedWebCache.m (1)
63-65: Good defensive programming practice.The nil check prevents setting
highlightedImageon a deallocated object, which is important for asynchronous image loading scenarios.SDWebImage/Core/SDImageCachesManager.m (5)
447-449: Essential safety check for serial cache operations.The nil check prevents recursive calls to
serialQueryImageForKeyon a deallocated manager, which is crucial for the operation chain integrity.
485-487: Proper defensive programming for cache store operations.The nil check ensures that serial store operations don't continue processing on a deallocated manager.
506-508: Good safety practice for cache removal operations.The nil check prevents continuing the serial removal chain on a deallocated manager.
529-531: Appropriate safety check for cache query operations.The nil check prevents recursive calls to
serialContainsImageForKeyon a deallocated manager.
567-569: Essential protection for cache clear operations.The nil check prevents continuing the serial clear chain on a deallocated manager.
|
Seems I made some mistake. Your original commit is correct. You can just rebase and keep only that fdea700 |
hawk0620
force-pushed
the
opt/avoid_self_be_nil_in_block
branch
from
7731fbd to
fdea700
Compare
|
Is this for fixing a crash? Or to improve performance? |
3 participants
New Pull Request Checklist
I have read and understood the CONTRIBUTING guide
I have read the Documentation
I have searched for a similar pull request in the project and found none
I have updated this branch with the latest master to avoid conflicts (via merge from master or rebase)
I have added the required tests to prove the fix/feature I am adding
I have updated the documentation (if necessary)
I have run the tests and they pass
I have run the lint and it passes (
pod lib lint)This merge request fixes / refers to the following issues: ...
Pull Request Description
Add condition statement
if (!self) { return; }to avoid self being nil in block and ensure the logic executes only when self is valid.Summary by CodeRabbit