Fabric: fix border memory leaks #23815
Conversation
facebook-github-bot
added
the
CLA Signed
There was a problem hiding this comment.
This is 100% valid concern ❤️!
... but probably not the best solution.
I think we should rename the method (or create an additional one) to RCTCGColorRefUnretainedFromSharedColor, annotate it with CF_RETURNS_NOT_RETAINED and don't use Create inside.
RCTBorderColors
|
@shergin I think I addressed your feedback. |
| @@ -396,7 +398,9 @@ - (void)invalidateLayer | |||
| } | |||
|
|
|||
| layer.borderWidth = (CGFloat)borderMetrics.borderWidths.left; | |||
| layer.borderColor = RCTCGColorRefFromSharedColor(borderMetrics.borderColors.left); | |||
| CGColorRef borderColor = RCTCGColorRefFromSharedColor(borderMetrics.borderColors.left) | |||
There was a problem hiding this comment.
I am not sure that this part is correct (or optimal), but I trust you.
facebook-github-bot
added
the
Import Started
There was a problem hiding this comment.
@shergin is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@shergin maybe this fixes the complaints? :) |
semicolon
There was a problem hiding this comment.
@shergin is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
This pull request was successfully merged by @ericlewis in 8e490d4. When will my fix make it into a release? | Upcoming Releases |
react-native-bot
added
Merged
Summary
This fixes a few memory leaks in fabrics handling of colors for borders, when using CGColorRef's we must be diligent about releasing the memory back.
Changelog
[iOS] [Fixed] - Border style memory leaks
Test Plan
Run RNTester, and ensure it is working, notice no memory leaks from CoreGraphics.