diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ac85790fa..e3e86c4e14 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ - [ASNetworkImageNode] New delegate callback to tell the consumer whether the image was loaded from cache or download. [Adlai Holler](https://github.com/Adlai-Holler) - [Layout] Fixes a deadlock in layout. [#638](https://github.com/TextureGroup/Texture/pull/638) [Garrett Moon](https://github.com/garrettmoon) - Updated to be backwards compatible with Xcode 8. [Adlai Holler](https://github.com/Adlai-Holler) +- [API CHANGES] `ASPerformMainThreadDeallocation` and `ASPerformBackgroundDeallocation` functions take `id *` instead of `id` and they're now more reliable. Also, in Swift, `ASDeallocQueue.sharedDeallocationQueue() -> ASDeallocQueue.sharedDeallocationQueue`. [Adlai Holler](https://github.com/Adlai-Holler) [#651](https://github.com/TextureGroup/Texture/pull/651) ## 2.6 - [Xcode 9] Updated to require Xcode 9 (to fix warnings) [Garrett Moon](https://github.com/garrettmoon) diff --git a/Source/ASCollectionView.mm b/Source/ASCollectionView.mm index 339728d83a..d3857ae451 100644 --- a/Source/ASCollectionView.mm +++ b/Source/ASCollectionView.mm @@ -332,8 +332,8 @@ static NSString * const kReuseIdentifier = @"_ASCollectionReuseIdentifier"; [self setAsyncDataSource:nil]; // Data controller & range controller may own a ton of nodes, let's deallocate those off-main. - ASPerformBackgroundDeallocation(_dataController); - ASPerformBackgroundDeallocation(_rangeController); + ASPerformBackgroundDeallocation(&_dataController); + ASPerformBackgroundDeallocation(&_rangeController); } #pragma mark - diff --git a/Source/ASDisplayNode.mm b/Source/ASDisplayNode.mm index 3ef5f36294..c5bdd88323 100644 --- a/Source/ASDisplayNode.mm +++ b/Source/ASDisplayNode.mm @@ -440,7 +440,7 @@ static ASDisplayNodeMethodOverrides GetASDisplayNodeMethodOverrides(Class c) // is still deallocated on a background thread object_setIvar(self, ivar, nil); - ASPerformMainThreadDeallocation(value); + ASPerformMainThreadDeallocation(&value); } else { as_log_debug(ASMainThreadDeallocationLog(), "%@: Not trampolining ivar '%s' value %@.", self, ivar_getName(ivar), value); } diff --git a/Source/ASDisplayNodeExtras.h b/Source/ASDisplayNodeExtras.h index 923f8b72fe..ba0a38172c 100644 --- a/Source/ASDisplayNodeExtras.h +++ b/Source/ASDisplayNodeExtras.h @@ -35,7 +35,7 @@ #endif /// For deallocation of objects on the main thread across multiple run loops. -extern void ASPerformMainThreadDeallocation(_Nullable id object); +extern void ASPerformMainThreadDeallocation(id _Nullable __strong * _Nonnull objectPtr); // Because inline methods can't be extern'd and need to be part of the translation unit of code // that compiles with them to actually inline, we both declare and define these in the header. diff --git a/Source/ASDisplayNodeExtras.mm b/Source/ASDisplayNodeExtras.mm index 279687662a..2839e1a6cf 100644 --- a/Source/ASDisplayNodeExtras.mm +++ b/Source/ASDisplayNodeExtras.mm @@ -23,8 +23,7 @@ #import #import -extern void ASPerformMainThreadDeallocation(_Nullable id object) -{ +extern void ASPerformMainThreadDeallocation(id _Nullable __strong * _Nonnull objectPtr) { /** * UIKit components must be deallocated on the main thread. We use this shared * run loop queue to gradually deallocate them across many turns of the main run loop. @@ -35,8 +34,14 @@ extern void ASPerformMainThreadDeallocation(_Nullable id object) queue = [[ASRunLoopQueue alloc] initWithRunLoop:CFRunLoopGetMain() retainObjects:YES handler:nil]; queue.batchSize = 10; }); - if (object != nil) { - [queue enqueue:object]; + + if (objectPtr != NULL && *objectPtr != nil) { + // Lock queue while enqueuing and releasing, so that there's no risk + // that the queue will release before we get a chance to release. + [queue lock]; + [queue enqueue:*objectPtr]; // Retain, +1 + *objectPtr = nil; // Release, +0 + [queue unlock]; // (After queue drains), release, -1 } } diff --git a/Source/ASImageNode.mm b/Source/ASImageNode.mm index df39741b00..5e2383081f 100644 --- a/Source/ASImageNode.mm +++ b/Source/ASImageNode.mm @@ -278,7 +278,7 @@ typedef void (^ASImageNodeDrawParametersBlock)(ASWeakMapEntry *entry); BOOL shouldReleaseImageOnBackgroundThread = oldImageSize.width > kMinReleaseImageOnBackgroundSize.width || oldImageSize.height > kMinReleaseImageOnBackgroundSize.height; if (shouldReleaseImageOnBackgroundThread) { - ASPerformBackgroundDeallocation(oldImage); + ASPerformBackgroundDeallocation(&oldImage); } } diff --git a/Source/ASMultiplexImageNode.mm b/Source/ASMultiplexImageNode.mm index 81f3f43c13..dc689d8299 100644 --- a/Source/ASMultiplexImageNode.mm +++ b/Source/ASMultiplexImageNode.mm @@ -532,10 +532,10 @@ typedef void(^ASMultiplexImageLoadCompletionBlock)(UIImage *image, id imageIdent CGSize imageSize = image.size; BOOL shouldReleaseImageOnBackgroundThread = imageSize.width > kMinReleaseImageOnBackgroundSize.width || imageSize.height > kMinReleaseImageOnBackgroundSize.height; - if (shouldReleaseImageOnBackgroundThread) { - ASPerformBackgroundDeallocation(image); - } [self _setImage:nil]; + if (shouldReleaseImageOnBackgroundThread) { + ASPerformBackgroundDeallocation(&image); + } } #pragma mark - diff --git a/Source/ASRunLoopQueue.h b/Source/ASRunLoopQueue.h index 6b08540ccd..50a0eeb249 100644 --- a/Source/ASRunLoopQueue.h +++ b/Source/ASRunLoopQueue.h @@ -21,7 +21,7 @@ NS_ASSUME_NONNULL_BEGIN AS_SUBCLASSING_RESTRICTED -@interface ASRunLoopQueue : NSObject +@interface ASRunLoopQueue : NSObject /** * Create a new queue with the given run loop and handler. @@ -51,11 +51,11 @@ AS_SUBCLASSING_RESTRICTED AS_SUBCLASSING_RESTRICTED @interface ASDeallocQueue : NSObject -+ (instancetype)sharedDeallocationQueue; +@property (class, atomic, readonly) ASDeallocQueue *sharedDeallocationQueue; - (void)test_drain; -- (void)releaseObjectInBackground:(id)object; +- (void)releaseObjectInBackground:(id __strong _Nullable * _Nonnull)objectPtr; @end diff --git a/Source/ASRunLoopQueue.mm b/Source/ASRunLoopQueue.mm index 1902ad23d0..ced7982529 100644 --- a/Source/ASRunLoopQueue.mm +++ b/Source/ASRunLoopQueue.mm @@ -45,7 +45,7 @@ static void runLoopSourceCallback(void *info) { ASDN::RecursiveMutex _queueLock; } -+ (instancetype)sharedDeallocationQueue ++ (ASDeallocQueue *)sharedDeallocationQueue { static ASDeallocQueue *deallocQueue = nil; static dispatch_once_t onceToken; @@ -55,16 +55,18 @@ static void runLoopSourceCallback(void *info) { return deallocQueue; } -- (void)releaseObjectInBackground:(id)object +- (void)releaseObjectInBackground:(id _Nullable __strong *)objectPtr { // Disable background deallocation on iOS 8 and below to avoid crashes related to UIAXDelegateClearer (#2767). if (!AS_AT_LEAST_IOS9) { return; } - _queueLock.lock(); - _queue.push_back(object); - _queueLock.unlock(); + if (objectPtr != NULL && *objectPtr != nil) { + ASDN::MutexLocker l(_queueLock); + _queue.push_back(*objectPtr); + *objectPtr = nil; + } } - (void)threadMain @@ -454,4 +456,16 @@ typedef enum { return _internalQueue.count == 0; } +#pragma mark - NSLocking + +- (void)lock +{ + _internalQueueLock.lock(); +} + +- (void)unlock +{ + _internalQueueLock.unlock(); +} + @end diff --git a/Source/ASTableView.mm b/Source/ASTableView.mm index 32e4979778..594d6a9966 100644 --- a/Source/ASTableView.mm +++ b/Source/ASTableView.mm @@ -372,8 +372,8 @@ static NSString * const kCellReuseIdentifier = @"_ASTableViewCell"; [self setAsyncDataSource:nil]; // Data controller & range controller may own a ton of nodes, let's deallocate those off-main - ASPerformBackgroundDeallocation(_dataController); - ASPerformBackgroundDeallocation(_rangeController); + ASPerformBackgroundDeallocation(&_dataController); + ASPerformBackgroundDeallocation(&_rangeController); } #pragma mark - diff --git a/Source/ASViewController.mm b/Source/ASViewController.mm index 2826fae9dc..4e1e57c3a6 100644 --- a/Source/ASViewController.mm +++ b/Source/ASViewController.mm @@ -96,7 +96,7 @@ - (void)dealloc { - ASPerformBackgroundDeallocation(_node); + ASPerformBackgroundDeallocation(&_node); } - (void)loadView diff --git a/Source/Private/ASInternalHelpers.h b/Source/Private/ASInternalHelpers.h index 90a525ef83..dc67700e13 100644 --- a/Source/Private/ASInternalHelpers.h +++ b/Source/Private/ASInternalHelpers.h @@ -38,7 +38,7 @@ void ASPerformBlockOnMainThread(void (^block)(void)); void ASPerformBlockOnBackgroundThread(void (^block)(void)); // DISPATCH_QUEUE_PRIORITY_DEFAULT /// For deallocation of objects on a background thread without GCD overhead / thread explosion -void ASPerformBackgroundDeallocation(id object); +void ASPerformBackgroundDeallocation(id __strong _Nullable * _Nonnull object); CGFloat ASScreenScale(void); diff --git a/Source/Private/ASInternalHelpers.m b/Source/Private/ASInternalHelpers.m index 8bc865eb30..ad55f295aa 100644 --- a/Source/Private/ASInternalHelpers.m +++ b/Source/Private/ASInternalHelpers.m @@ -84,7 +84,7 @@ void ASPerformBlockOnBackgroundThread(void (^block)(void)) } } -void ASPerformBackgroundDeallocation(id object) +void ASPerformBackgroundDeallocation(id __strong _Nullable * _Nonnull object) { [[ASDeallocQueue sharedDeallocationQueue] releaseObjectInBackground:object]; }