Skip to content
This repository has been archived by the owner on Feb 2, 2023. It is now read-only.

[ASTextNode, ASImageNode, ASVideoNode] Use ASDisplayNode base class lock for subclass property synchronization #1877

Merged
merged 11 commits into from
Jul 10, 2016
Merged

[ASTextNode, ASImageNode, ASVideoNode] Use ASDisplayNode base class lock for subclass property synchronization #1877

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
2fabd64
Merge remote-tracking branch 'facebook/master'
Jul 7, 2016
224823b
Merge remote-tracking branch 'facebook/master'
Jul 7, 2016
4e265d9
Merge branch 'master' of https://github.com/facebook/AsyncDisplayKit
Jul 8, 2016
28f497c
Merge branch 'master' of https://github.com/facebook/AsyncDisplayKit
Jul 10, 2016
198cd6e
[ASTextNode, ASVideoNode] Use ASDisplayNode base class lock for subcl…
Jul 10, 2016
3ab7aa0
fix headers to match master
Jul 10, 2016
79e3d63
address @appleguy, @maicki comments
Jul 10, 2016
878e9bf
import header
Jul 10, 2016
358cde4
Swap lock in ASNetworkImageNode as well
Jul 10, 2016
6f0a934
remove invalid comment
Jul 10, 2016
69a8048
more cleanup of locks
Jul 10, 2016
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
41 changes: 18 additions & 23 deletions AsyncDisplayKit/ASImageNode.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ @implementation ASImageNode
UIImage *_image;

void (^_displayCompletionBlock)(BOOL canceled);
ASDN::RecursiveMutex _imageLock;

// Drawing
ASImageNodeDrawParameters _drawParameter;
Expand Down Expand Up @@ -120,7 +119,7 @@ - (void)dealloc

- (CGSize)calculateSizeThatFits:(CGSize)constrainedSize
{
ASDN::MutexLocker l(_imageLock);
ASDN::MutexLocker l(_propertyLock);
// if a preferredFrameSize is set, call the superclass to return that instead of using the image size.
if (CGSizeEqualToSize(self.preferredFrameSize, CGSizeZero) == NO)
return [super calculateSizeThatFits:constrainedSize];
Expand All @@ -134,11 +133,9 @@ - (CGSize)calculateSizeThatFits:(CGSize)constrainedSize

- (void)setImage:(UIImage *)image
{
_imageLock.lock();
ASDN::MutexLocker l(_propertyLock);
if (!ASObjectIsEqual(_image, image)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I actually remember now why this method had to use .lock() and .unlock() manually. It was to try to avoid interleaving with the superclass _propertyLock, which was grabbed inside invalidateCalculatedLayout and other methods below.

I think now that this is addressed, we should make this case a simple MutexLocker. The else case that unlocks can be deleted.

_image = image;

_imageLock.unlock();

[self invalidateCalculatedLayout];
if (image) {
Expand All @@ -154,14 +151,12 @@ - (void)setImage:(UIImage *)image
} else {
self.contents = nil;
}
} else {
_imageLock.unlock(); // We avoid using MutexUnlocker as it needlessly re-locks at the end of the scope.
}
}

- (UIImage *)image
{
ASDN::MutexLocker l(_imageLock);
ASDN::MutexLocker l(_propertyLock);
return _image;
}

Expand All @@ -177,7 +172,7 @@ - (void)setPlaceholderColor:(UIColor *)placeholderColor

- (NSObject *)drawParametersForAsyncLayer:(_ASDisplayLayer *)layer
{
ASDN::MutexLocker l(_imageLock);
ASDN::MutexLocker l(_propertyLock);

_drawParameter = {
.bounds = self.bounds,
Expand Down Expand Up @@ -222,7 +217,7 @@ - (UIImage *)displayWithParameters:(id<NSObject> *)parameter isCancelled:(asdisp
asimagenode_modification_block_t imageModificationBlock;

{
ASDN::MutexLocker l(_imageLock);
ASDN::MutexLocker l(_propertyLock);
ASImageNodeDrawParameters drawParameter = _drawParameter;

drawParameterBounds = drawParameter.bounds;
Expand Down Expand Up @@ -357,19 +352,19 @@ - (void)displayDidFinish
{
[super displayDidFinish];

_imageLock.lock();
_propertyLock.lock();
void (^displayCompletionBlock)(BOOL canceled) = _displayCompletionBlock;
UIImage *image = _image;
_imageLock.unlock();
_propertyLock.unlock();

// If we've got a block to perform after displaying, do it.
if (image && displayCompletionBlock) {

displayCompletionBlock(NO);

_imageLock.lock();
_propertyLock.lock();
_displayCompletionBlock = nil;
_imageLock.unlock();
_propertyLock.unlock();
}
}

Expand All @@ -382,7 +377,7 @@ - (void)setNeedsDisplayWithCompletion:(void (^ _Nullable)(BOOL canceled))display
}

// Stash the block and call-site queue. We'll invoke it in -displayDidFinish.
ASDN::MutexLocker l(_imageLock);
ASDN::MutexLocker l(_propertyLock);
if (_displayCompletionBlock != displayCompletionBlock) {
_displayCompletionBlock = [displayCompletionBlock copy];
}
Expand All @@ -394,7 +389,7 @@ - (void)setNeedsDisplayWithCompletion:(void (^ _Nullable)(BOOL canceled))display

- (BOOL)isCropEnabled
{
ASDN::MutexLocker l(_imageLock);
ASDN::MutexLocker l(_propertyLock);
return _cropEnabled;
}

Expand All @@ -405,7 +400,7 @@ - (void)setCropEnabled:(BOOL)cropEnabled

- (void)setCropEnabled:(BOOL)cropEnabled recropImmediately:(BOOL)recropImmediately inBounds:(CGRect)cropBounds
{
ASDN::MutexLocker l(_imageLock);
ASDN::MutexLocker l(_propertyLock);
if (_cropEnabled == cropEnabled)
return;

Expand All @@ -426,13 +421,13 @@ - (void)setCropEnabled:(BOOL)cropEnabled recropImmediately:(BOOL)recropImmediate

- (CGRect)cropRect
{
ASDN::MutexLocker l(_imageLock);
ASDN::MutexLocker l(_propertyLock);
return _cropRect;
}

- (void)setCropRect:(CGRect)cropRect
{
ASDN::MutexLocker l(_imageLock);
ASDN::MutexLocker l(_propertyLock);
if (CGRectEqualToRect(_cropRect, cropRect))
return;

Expand All @@ -453,25 +448,25 @@ - (void)setCropRect:(CGRect)cropRect

- (BOOL)forceUpscaling
{
ASDN::MutexLocker l(_imageLock);
ASDN::MutexLocker l(_propertyLock);
return _forceUpscaling;
}

- (void)setForceUpscaling:(BOOL)forceUpscaling
{
ASDN::MutexLocker l(_imageLock);
ASDN::MutexLocker l(_propertyLock);
_forceUpscaling = forceUpscaling;
}

- (asimagenode_modification_block_t)imageModificationBlock
{
ASDN::MutexLocker l(_imageLock);
ASDN::MutexLocker l(_propertyLock);
return _imageModificationBlock;
}

- (void)setImageModificationBlock:(asimagenode_modification_block_t)imageModificationBlock
{
ASDN::MutexLocker l(_imageLock);
ASDN::MutexLocker l(_propertyLock);
_imageModificationBlock = imageModificationBlock;
}

Expand Down
Loading