Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove lock of ASTextNodeRendererKey #1454

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Source/ASDisplayNode.mm
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ void StubImplementationWithTwoInterfaceStates(id receiver, SEL _cmd, ASInterface
* @remarks The instance value is used only if we suspect the class may be dynamic (because it overloads
* +respondsToSelector: or -respondsToSelector.) In that case we use our "slow path", calling this
* method on each -init and passing the instance value. While this may seem like an unlikely scenario,
* it turns our our own internal tests use a dynamic class, so it's worth capturing this edge case.
* it turns out our own internal tests use a dynamic class, so it's worth capturing this edge case.
*
* @return ASDisplayNode flags.
*/
Expand Down
29 changes: 16 additions & 13 deletions Source/ASTextNode.mm
Original file line number Diff line number Diff line change
Expand Up @@ -58,17 +58,25 @@
#pragma mark - ASTextKitRenderer

@interface ASTextNodeRendererKey : NSObject
@property (nonatomic) ASTextKitAttributes attributes;
@property (nonatomic) CGSize constrainedSize;
- (instancetype)initWithTextKitAttributes:(const ASTextKitAttributes &)attributes constrainedSize:(const CGSize)constrainedSize;
@end

@implementation ASTextNodeRendererKey {
std::mutex _m;
ASTextKitAttributes _attributes;
CGSize _constrainedSize;
}

- (instancetype)initWithTextKitAttributes:(const ASTextKitAttributes &)attributes constrainedSize:(const CGSize)constrainedSize
{
if (self = [super init]) {
_attributes = attributes;
_constrainedSize = constrainedSize;
}
return self;
}

- (NSUInteger)hash
{
std::lock_guard<std::mutex> _l(_m);
#pragma clang diagnostic push
#pragma clang diagnostic warning "-Wpadded"
struct {
Expand All @@ -87,13 +95,10 @@ - (BOOL)isEqual:(ASTextNodeRendererKey *)object
if (self == object) {
return YES;
}

if (!object) {
return NO;
}
// NOTE: Skip the class check for this specialized, internal Key object.

// Lock both objects, avoiding deadlock.
std::lock(_m, object->_m);
std::lock_guard<std::mutex> lk1(_m, std::adopt_lock);
std::lock_guard<std::mutex> lk2(object->_m, std::adopt_lock);

return _attributes == object->_attributes && CGSizeEqualToSize(_constrainedSize, object->_constrainedSize);
}
Expand Down Expand Up @@ -121,9 +126,7 @@ - (BOOL)isEqual:(ASTextNodeRendererKey *)object
{
NSCache *cache = sharedRendererCache();

ASTextNodeRendererKey *key = [[ASTextNodeRendererKey alloc] init];
key.attributes = attributes;
key.constrainedSize = constrainedSize;
ASTextNodeRendererKey *key = [[ASTextNodeRendererKey alloc] initWithTextKitAttributes:attributes constrainedSize:constrainedSize];

ASTextKitRenderer *renderer = [cache objectForKey:key];
if (renderer == nil) {
Expand Down
2 changes: 1 addition & 1 deletion Source/ASVisibilityProtocols.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ AS_EXTERN ASLayoutRangeMode ASLayoutRangeModeForVisibilityDepth(NSUInteger visib
* @discussion Represents the number of user actions necessary to reach the view controller. An increased visibility
* depth indicates a higher number of user interactions for the view controller to be visible again. For example,
* an onscreen navigation controller's top view controller should have a visibility depth of 0. The view controller
* one from the top should have a visibility deptch of 1 as should the root view controller in the stack (because
* one from the top should have a visibility depth of 1 as should the root view controller in the stack (because
* the user can hold the back button to pop to the root view controller).
*
* Visibility depth is used to automatically adjust ranges on range controllers (and thus free up memory) and can
Expand Down