Skip to content

Commit

Permalink
Add support for acquiring multiple locks at once (#958)
Browse files Browse the repository at this point in the history
* Add ASLocking which supports -tryLock and taking multiple locks safely

* Better multi locking

* Assert about lock set capacity
  • Loading branch information
Adlai-Holler authored Jun 5, 2018
1 parent 9214e3c commit 35d59ac
Show file tree
Hide file tree
Showing 13 changed files with 209 additions and 46 deletions.
4 changes: 4 additions & 0 deletions AsyncDisplayKit.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,7 @@
CCEDDDD1200C488000FFCD0A /* ASConfiguration.m in Sources */ = {isa = PBXBuildFile; fileRef = CCEDDDD0200C488000FFCD0A /* ASConfiguration.m */; };
CCEDDDD9200C518800FFCD0A /* ASConfigurationTests.m in Sources */ = {isa = PBXBuildFile; fileRef = CCEDDDD8200C518800FFCD0A /* ASConfigurationTests.m */; };
CCF18FF41D2575E300DF5895 /* NSIndexSet+ASHelpers.h in Headers */ = {isa = PBXBuildFile; fileRef = CC4981BA1D1C7F65004E13CC /* NSIndexSet+ASHelpers.h */; settings = {ATTRIBUTES = (Private, ); }; };
CCF1FF5E20C4785000AAD8FC /* ASLocking.h in Headers */ = {isa = PBXBuildFile; fileRef = CCF1FF5D20C4785000AAD8FC /* ASLocking.h */; settings = {ATTRIBUTES = (Public, ); }; };
DB55C2671C641AE4004EDCF5 /* ASContextTransitioning.h in Headers */ = {isa = PBXBuildFile; fileRef = DB55C2651C641AE4004EDCF5 /* ASContextTransitioning.h */; settings = {ATTRIBUTES = (Public, ); }; };
DB7121BCD50849C498C886FB /* libPods-AsyncDisplayKitTests.a in Frameworks */ = {isa = PBXBuildFile; fileRef = EFA731F0396842FF8AB635EE /* libPods-AsyncDisplayKitTests.a */; };
DB78412E1C6BCE1600A9E2B4 /* _ASTransitionContext.m in Sources */ = {isa = PBXBuildFile; fileRef = DB55C2601C6408D6004EDCF5 /* _ASTransitionContext.m */; };
Expand Down Expand Up @@ -947,6 +948,7 @@
CCEDDDCE200C42A200FFCD0A /* ASConfigurationDelegate.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = ASConfigurationDelegate.h; sourceTree = "<group>"; };
CCEDDDD0200C488000FFCD0A /* ASConfiguration.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = ASConfiguration.m; sourceTree = "<group>"; };
CCEDDDD8200C518800FFCD0A /* ASConfigurationTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = ASConfigurationTests.m; sourceTree = "<group>"; };
CCF1FF5D20C4785000AAD8FC /* ASLocking.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = ASLocking.h; sourceTree = "<group>"; };
D3779BCFF841AD3EB56537ED /* Pods-AsyncDisplayKitTests.release.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-AsyncDisplayKitTests.release.xcconfig"; path = "Pods/Target Support Files/Pods-AsyncDisplayKitTests/Pods-AsyncDisplayKitTests.release.xcconfig"; sourceTree = "<group>"; };
D785F6601A74327E00291744 /* ASScrollNode.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ASScrollNode.h; sourceTree = "<group>"; };
D785F6611A74327E00291744 /* ASScrollNode.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = ASScrollNode.mm; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1162,6 +1164,7 @@
058D09DD195D050800B7D73C /* ASImageNode.h */,
058D09DE195D050800B7D73C /* ASImageNode.mm */,
68355B2E1CB5799E001D4E68 /* ASImageNode+AnimatedImage.mm */,
CCF1FF5D20C4785000AAD8FC /* ASLocking.h */,
92DD2FE11BF4B97E0074C9DD /* ASMapNode.h */,
92DD2FE21BF4B97E0074C9DD /* ASMapNode.mm */,
0516FA3E1A1563D200B4EBED /* ASMultiplexImageNode.h */,
Expand Down Expand Up @@ -1871,6 +1874,7 @@
CC56013B1F06E9A700DC4FBE /* ASIntegerMap.h in Headers */,
B35061FA1B010EFD0018CF92 /* ASControlNode+Subclasses.h in Headers */,
E54E81FC1EB357BD00FFE8E1 /* ASPageTable.h in Headers */,
CCF1FF5E20C4785000AAD8FC /* ASLocking.h in Headers */,
B35061F81B010EFD0018CF92 /* ASControlNode.h in Headers */,
B35062171B010EFD0018CF92 /* ASDataController.h in Headers */,
34EFC75B1B701BAF00AD841F /* ASDimension.h in Headers */,
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
- Make `ASPerformMainThreadDeallocation` visible in C. [Adlai Holler](https://github.com/Adlai-Holler)
- Add snapshot test for astextnode2. [Max Wang](https://github.com/wsdwsd0829) [#935](https://github.com/TextureGroup/Texture/pull/935)
- Internal housekeeping on the async transaction (rendering) system. [Adlai Holler](https://github.com/Adlai-Holler)
- Add new protocol `ASLocking` that extends `NSLocking` with `tryLock`, and allows taking multiple locks safely. [Adlai Holler](https://github.com/Adlai-Holler)

## 2.7
- Fix pager node for interface coalescing. [Max Wang](https://github.com/wsdwsd0829) [#877](https://github.com/TextureGroup/Texture/pull/877)
Expand Down
3 changes: 2 additions & 1 deletion Source/ASDisplayNode.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#import <AsyncDisplayKit/ASAsciiArtBoxCreator.h>
#import <AsyncDisplayKit/ASObjectDescriptionHelpers.h>
#import <AsyncDisplayKit/ASLayoutElement.h>
#import <AsyncDisplayKit/ASLocking.h>

NS_ASSUME_NONNULL_BEGIN

Expand Down Expand Up @@ -127,7 +128,7 @@ extern NSInteger const ASDefaultDrawingPriority;
*
*/

@interface ASDisplayNode : NSObject <NSLocking>
@interface ASDisplayNode : NSObject <ASLocking>

/** @name Initializing a node object */

Expand Down
10 changes: 1 addition & 9 deletions Source/ASDisplayNode.mm
Original file line number Diff line number Diff line change
Expand Up @@ -373,15 +373,7 @@ - (instancetype)initWithLayerBlock:(ASDisplayNodeLayerBlock)layerBlock didLoadBl
return self;
}

- (void)lock
{
__instanceLock__.lock();
}

- (void)unlock
{
__instanceLock__.unlock();
}
ASSynthesizeLockingMethodsWithMutex(__instanceLock__);

- (void)setViewBlock:(ASDisplayNodeViewBlock)viewBlock
{
Expand Down
162 changes: 162 additions & 0 deletions Source/ASLocking.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
//
// ASLocking.h
// Texture
//
// Copyright (c) 2018-present, Pinterest, Inc. All rights reserved.
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//

#import <Foundation/Foundation.h>
#import <pthread/sched.h>

#import <AsyncDisplayKit/ASAssert.h>

NS_ASSUME_NONNULL_BEGIN

#define kLockSetCapacity 32

/**
* An extension of NSLocking that supports -tryLock.
*/
@protocol ASLocking <NSLocking>

/// Try to take lock without blocking. Returns whether the lock was taken.
- (BOOL)tryLock;

@end

/**
* A set of locks acquired during ASLockSequence.
*/
typedef struct {
unsigned count;
CFTypeRef _Nullable locks[kLockSetCapacity];
} ASLockSet;

/**
* Declare a lock set that is automatically unlocked at the end of scope.
*
* We use this instead of a scope-locking macro because we want to be able
* to step through the lock sequence block in the debugger.
*/
#define ASScopedLockSet __unused ASLockSet __attribute__((cleanup(ASUnlockSet)))

/**
* A block that attempts to add a lock to a lock sequence.
* Such a block is provided to the caller of ASLockSequence.
*
* Returns whether the lock was added. You should return
* NO from your lock sequence body if it returns NO.
*
* For instance, you might write `return addLock(l1) && addLock(l2)`.
*
* @param lock The lock to attempt to add.
* @return YES if the lock was added, NO otherwise.
*/
typedef BOOL(^ASAddLockBlock)(id<ASLocking> lock);

/**
* A block that attempts to lock multiple locks in sequence.
* Such a block is provided by the caller of ASLockSequence.
*
* The block may be run multiple times, if not all locks are immediately
* available. Therefore the block should be idempotent.
*
* The block should attempt to invoke addLock multiple times with
* different locks. It should return NO as soon as any addLock
* operation fails.
*
* For instance, you might write `return addLock(l1) && addLock(l2)`.
*
* @param addLock A block you can call to attempt to add a lock.
* @return YES if all locks were added, NO otherwise.
*/
typedef BOOL(^ASLockSequenceBlock)(NS_NOESCAPE ASAddLockBlock addLock);

/**
* Unlock and release all of the locks in this lock set.
*/
NS_INLINE void ASUnlockSet(ASLockSet *lockSet) {
for (unsigned i = 0; i < lockSet->count; i++) {
CFTypeRef lock = lockSet->locks[i];
[(__bridge id<ASLocking>)lock unlock];
CFRelease(lock);
}
}

/**
* Take multiple locks "simultaneously," avoiding deadlocks
* caused by lock ordering.
*
* The block you provide should attempt to take a series of locks,
* using the provided `addLock` block. As soon as any addLock fails,
* you should return NO.
*
* For example:
* ASLockSequence(^(ASAddLockBlock addLock) ^{
* return addLock(l0) && addLock(l1);
* });
*
* Note: This function doesn't protect from lock ordering deadlocks if
* one of the locks is already locked (recursive.) Only locks taken
* inside this function are guaranteed not to cause a deadlock.
*/
NS_INLINE ASLockSet ASLockSequence(NS_NOESCAPE ASLockSequenceBlock body)
{
__block ASLockSet locks = (ASLockSet){0};
BOOL (^addLock)(id<ASLocking>) = ^(id<ASLocking> obj) {

// nil lock = ignore.
if (!obj) {
return YES;
}

// If they go over capacity, assert and return YES.
// If we return NO, they will enter an infinite loop.
if (locks.count == kLockSetCapacity) {
ASDisplayNodeCFailAssert(@"Locking more than %d locks at once is not supported.", kLockSetCapacity);
return YES;
}

if ([obj tryLock]) {
locks.locks[locks.count++] = (__bridge_retained CFTypeRef)obj;
return YES;
}
return NO;
};

/**
* Repeatedly try running their block, passing in our `addLock`
* until it succeeds. If it fails, unlock all and yield the thread
* to reduce spinning.
*/
while (true) {
if (body(addLock)) {
// Success
return locks;
} else {
ASUnlockSet(&locks);
locks.count = 0;
sched_yield();
}
}
}

/**
* These Foundation classes already implement -tryLock.
*/

@interface NSLock (ASLocking) <ASLocking>
@end

@interface NSRecursiveLock (ASLocking) <ASLocking>
@end

@interface NSConditionLock (ASLocking) <ASLocking>
@end

NS_ASSUME_NONNULL_END
3 changes: 2 additions & 1 deletion Source/ASRunLoopQueue.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

#import <Foundation/Foundation.h>
#import <AsyncDisplayKit/ASBaseDefines.h>
#import <AsyncDisplayKit/ASLocking.h>

NS_ASSUME_NONNULL_BEGIN

Expand All @@ -28,7 +29,7 @@ NS_ASSUME_NONNULL_BEGIN
@end

AS_SUBCLASSING_RESTRICTED
@interface ASRunLoopQueue<ObjectType> : ASAbstractRunLoopQueue <NSLocking>
@interface ASRunLoopQueue<ObjectType> : ASAbstractRunLoopQueue <ASLocking>

/**
* Create a new queue with the given run loop and handler.
Expand Down
12 changes: 1 addition & 11 deletions Source/ASRunLoopQueue.mm
Original file line number Diff line number Diff line change
Expand Up @@ -541,17 +541,7 @@ - (BOOL)isEmpty
return _internalQueue.count == 0;
}

#pragma mark - NSLocking

- (void)lock
{
_internalQueueLock.lock();
}

- (void)unlock
{
_internalQueueLock.unlock();
}
ASSynthesizeLockingMethodsWithMutex(_internalQueueLock)

@end

Expand Down
1 change: 1 addition & 0 deletions Source/AsyncDisplayKit.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@
#import <AsyncDisplayKit/ASHashing.h>
#import <AsyncDisplayKit/ASHighlightOverlayLayer.h>
#import <AsyncDisplayKit/ASImageContainerProtocolCategories.h>
#import <AsyncDisplayKit/ASLocking.h>
#import <AsyncDisplayKit/ASLog.h>
#import <AsyncDisplayKit/ASMutableAttributedStringBuilder.h>
#import <AsyncDisplayKit/ASRunLoopQueue.h>
Expand Down
29 changes: 29 additions & 0 deletions Source/Details/ASThread.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,16 @@ ASDISPLAYNODE_INLINE void _ASLockScopeUnownedCleanup(id<NSLocking> __unsafe_unre
id<NSLocking> __lockToken __attribute__((cleanup(_ASUnlockScopeCleanup))) NS_VALID_UNTIL_END_OF_SCOPE = nsLocking; \
[__lockToken unlock];

#define ASSynthesizeLockingMethodsWithMutex(mutex) \
- (void)lock { mutex.lock(); } \
- (void)unlock { mutex.unlock(); } \
- (BOOL)tryLock { return mutex.tryLock(); }

#define ASSynthesizeLockingMethodsWithObject(object) \
- (void)lock { [object lock]; } \
- (void)unlock { [object unlock]; } \
- (BOOL)tryLock { return [object tryLock]; }

ASDISPLAYNODE_INLINE void _ASUnlockScopeCleanup(id<NSLocking> __strong *lockPtr) {
[*lockPtr lock];
}
Expand Down Expand Up @@ -265,6 +275,25 @@ namespace ASDN {
Mutex (const Mutex&) = delete;
Mutex &operator=(const Mutex&) = delete;

bool tryLock() {
if (gMutex_unfair) {
if (_recursive) {
return ASRecursiveUnfairLockTryLock(&_runfair);
} else {
return os_unfair_lock_trylock(&_unfair);
}
} else {
auto result = pthread_mutex_trylock(&_m);
if (result == 0) {
return true;
} else if (result == EBUSY) {
return false;
} else {
ASDisplayNodeCFailAssert(@"Locking error: %s", strerror(result));
return true; // if we return false we may enter an infinite loop.
}
}
}
void lock() {
if (gMutex_unfair) {
if (_recursive) {
Expand Down
3 changes: 2 additions & 1 deletion Source/Layout/ASLayoutElement.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#import <AsyncDisplayKit/ASAbsoluteLayoutElement.h>
#import <AsyncDisplayKit/ASTraitCollection.h>
#import <AsyncDisplayKit/ASAsciiArtBoxCreator.h>
#import <AsyncDisplayKit/ASLocking.h>

@class ASLayout;
@class ASLayoutSpec;
Expand Down Expand Up @@ -174,7 +175,7 @@ extern NSString * const ASLayoutElementStyleLayoutPositionProperty;
- (void)style:(__kindof ASLayoutElementStyle *)style propertyDidChange:(NSString *)propertyName;
@end

@interface ASLayoutElementStyle : NSObject <ASStackLayoutElement, ASAbsoluteLayoutElement, ASLayoutElementExtensibility, NSLocking>
@interface ASLayoutElementStyle : NSObject <ASStackLayoutElement, ASAbsoluteLayoutElement, ASLayoutElementExtensibility, ASLocking>

/**
* @abstract Initializes the layoutElement style with a specified delegate
Expand Down
12 changes: 1 addition & 11 deletions Source/Layout/ASLayoutElement.mm
Original file line number Diff line number Diff line change
Expand Up @@ -176,17 +176,7 @@ - (instancetype)init
return self;
}

#pragma mark - NSLocking

- (void)lock
{
__instanceLock__.lock();
}

- (void)unlock
{
__instanceLock__.unlock();
}
ASSynthesizeLockingMethodsWithMutex(__instanceLock__)

#pragma mark - ASLayoutElementStyleSize

Expand Down
3 changes: 2 additions & 1 deletion Source/Layout/ASLayoutSpec.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,15 @@

#import <AsyncDisplayKit/ASLayoutElement.h>
#import <AsyncDisplayKit/ASAsciiArtBoxCreator.h>
#import <AsyncDisplayKit/ASLocking.h>
#import <AsyncDisplayKit/ASObjectDescriptionHelpers.h>

NS_ASSUME_NONNULL_BEGIN

/**
* A layout spec is an immutable object that describes a layout, loosely inspired by React.
*/
@interface ASLayoutSpec : NSObject <ASLayoutElement, ASLayoutElementStylability, NSFastEnumeration, ASDescriptionProvider, NSLocking>
@interface ASLayoutSpec : NSObject <ASLayoutElement, ASLayoutElementStylability, NSFastEnumeration, ASDescriptionProvider, ASLocking>

/**
* Creation of a layout spec should only happen by a user in layoutSpecThatFits:. During that method, a
Expand Down
12 changes: 1 addition & 11 deletions Source/Layout/ASLayoutSpec.mm
Original file line number Diff line number Diff line change
Expand Up @@ -259,17 +259,7 @@ - (NSString *)asciiArtName
return result;
}

#pragma mark - NSLocking

- (void)lock
{
__instanceLock__.lock();
}

- (void)unlock
{
__instanceLock__.unlock();
}
ASSynthesizeLockingMethodsWithMutex(__instanceLock__)

@end

Expand Down

0 comments on commit 35d59ac

Please sign in to comment.