From e745aded7dd01603f15860ea14c6b4f23635446c Mon Sep 17 00:00:00 2001 From: Huy Nguyen Date: Sun, 4 Nov 2018 16:28:48 -0800 Subject: [PATCH] [ASImageNode] Fix a threading issue which can cause a display completion block to never be executed (#1148) - Clear _displayCompletionBlock while we still have the node's instance lock. Because it may not be the same block by the time the lock is reacquired. In other words, it can happen that another thread sets a new display block after this thread releases the lock but before it reacquires it. And we don't want to clear out the new block. - Reduce a lock/unlock pair which should help perf a tiny bit. --- Source/ASImageNode.mm | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/Source/ASImageNode.mm b/Source/ASImageNode.mm index d5739f1a49..1ace8c74b1 100644 --- a/Source/ASImageNode.mm +++ b/Source/ASImageNode.mm @@ -533,8 +533,15 @@ static ASDN::StaticMutex& cacheLock = *new ASDN::StaticMutex; [super displayDidFinish]; __instanceLock__.lock(); - void (^displayCompletionBlock)(BOOL canceled) = _displayCompletionBlock; UIImage *image = _image; + void (^displayCompletionBlock)(BOOL canceled) = _displayCompletionBlock; + BOOL shouldPerformDisplayCompletionBlock = (image && displayCompletionBlock); + + // Clear the ivar now. The block is retained and will be executed shortly. + if (shouldPerformDisplayCompletionBlock) { + _displayCompletionBlock = nil; + } + BOOL hasDebugLabel = (_debugLabelNode != nil); __instanceLock__.unlock(); @@ -556,13 +563,8 @@ static ASDN::StaticMutex& cacheLock = *new ASDN::StaticMutex; } // If we've got a block to perform after displaying, do it. - if (image && displayCompletionBlock) { - + if (shouldPerformDisplayCompletionBlock) { displayCompletionBlock(NO); - - __instanceLock__.lock(); - _displayCompletionBlock = nil; - __instanceLock__.unlock(); } }