From ea64d7d09be74580a27f7f4b2a9e3b358705a71b Mon Sep 17 00:00:00 2001 From: John Engelhart Date: Wed, 22 Jun 2016 16:56:53 -0700 Subject: [PATCH] Pr/fix unit tests memory leaks (#1795) * Fix some concurrency problems detected by Xcode 8's new Thread Sanitizer. Some of these changes are arguably just to silence the warnings from Thread Sanitizer. * Fix several memory leaks in the unit tests. A number of the unit test source files are compield with `-fno-objc-arc`. This was clearly overlooked when writing several of the unit tests. Fixed by (mostly) switching to use of `-autorelease` for the problem code. NOTE: This commit doesn't fix all the memory leaks found. There's still at least one leak in `-[ASDisplayNodeTests testSetNeedsDataFetchImmediateState]`, and several leaks in `ASBasicImageDownloader.mm`. I wasn't able to find a trivial cause to these, unfortunately. --- .../Transactions/_ASAsyncTransaction.mm | 34 +++--- AsyncDisplayKit/Private/_AS-objc-internal.h | 9 +- AsyncDisplayKitTests/ASDisplayLayerTests.m | 11 +- .../ASDisplayNodeAppearanceTests.m | 20 ++-- AsyncDisplayKitTests/ASDisplayNodeTests.m | 101 ++++++++---------- .../ASMultiplexImageNodeTests.m | 18 +++- 6 files changed, 98 insertions(+), 95 deletions(-) diff --git a/AsyncDisplayKit/Details/Transactions/_ASAsyncTransaction.mm b/AsyncDisplayKit/Details/Transactions/_ASAsyncTransaction.mm index 3f02afba4d..5342e34f33 100644 --- a/AsyncDisplayKit/Details/Transactions/_ASAsyncTransaction.mm +++ b/AsyncDisplayKit/Details/Transactions/_ASAsyncTransaction.mm @@ -327,7 +327,7 @@ ASAsyncTransactionQueue & ASAsyncTransactionQueue::instance() _callbackQueue = callbackQueue; _completionBlock = [completionBlock copy]; - _state = ASAsyncTransactionStateOpen; + __atomic_store_n(&_state, ASAsyncTransactionStateOpen, __ATOMIC_SEQ_CST); } return self; } @@ -335,7 +335,7 @@ ASAsyncTransactionQueue & ASAsyncTransactionQueue::instance() - (void)dealloc { // Uncommitted transactions break our guarantees about releasing completion blocks on callbackQueue. - ASDisplayNodeAssert(_state != ASAsyncTransactionStateOpen, @"Uncommitted ASAsyncTransactions are not allowed"); + ASDisplayNodeAssert(__atomic_load_n(&_state, __ATOMIC_SEQ_CST) != ASAsyncTransactionStateOpen, @"Uncommitted ASAsyncTransactions are not allowed"); if (_group) { _group->release(); } @@ -360,7 +360,7 @@ ASAsyncTransactionQueue & ASAsyncTransactionQueue::instance() completion:(asyncdisplaykit_async_transaction_operation_completion_block_t)completion { ASDisplayNodeAssertMainThread(); - ASDisplayNodeAssert(_state == ASAsyncTransactionStateOpen, @"You can only add operations to open transactions"); + ASDisplayNodeAssert(__atomic_load_n(&_state, __ATOMIC_SEQ_CST) == ASAsyncTransactionStateOpen, @"You can only add operations to open transactions"); [self _ensureTransactionData]; @@ -368,7 +368,7 @@ ASAsyncTransactionQueue & ASAsyncTransactionQueue::instance() [_operations addObject:operation]; _group->schedule(priority, queue, ^{ @autoreleasepool { - if (_state != ASAsyncTransactionStateCanceled) { + if (__atomic_load_n(&_state, __ATOMIC_SEQ_CST) != ASAsyncTransactionStateCanceled) { _group->enter(); block(^(id value){ operation.value = value; @@ -395,7 +395,7 @@ ASAsyncTransactionQueue & ASAsyncTransactionQueue::instance() completion:(asyncdisplaykit_async_transaction_operation_completion_block_t)completion { ASDisplayNodeAssertMainThread(); - ASDisplayNodeAssert(_state == ASAsyncTransactionStateOpen, @"You can only add operations to open transactions"); + ASDisplayNodeAssert(__atomic_load_n(&_state, __ATOMIC_SEQ_CST) == ASAsyncTransactionStateOpen, @"You can only add operations to open transactions"); [self _ensureTransactionData]; @@ -403,7 +403,7 @@ ASAsyncTransactionQueue & ASAsyncTransactionQueue::instance() [_operations addObject:operation]; _group->schedule(priority, queue, ^{ @autoreleasepool { - if (_state != ASAsyncTransactionStateCanceled) { + if (__atomic_load_n(&_state, __ATOMIC_SEQ_CST) != ASAsyncTransactionStateCanceled) { operation.value = block(); } } @@ -422,15 +422,15 @@ ASAsyncTransactionQueue & ASAsyncTransactionQueue::instance() - (void)cancel { ASDisplayNodeAssertMainThread(); - ASDisplayNodeAssert(_state != ASAsyncTransactionStateOpen, @"You can only cancel a committed or already-canceled transaction"); - _state = ASAsyncTransactionStateCanceled; + ASDisplayNodeAssert(__atomic_load_n(&_state, __ATOMIC_SEQ_CST) != ASAsyncTransactionStateOpen, @"You can only cancel a committed or already-canceled transaction"); + __atomic_store_n(&_state, ASAsyncTransactionStateCanceled, __ATOMIC_SEQ_CST); } - (void)commit { ASDisplayNodeAssertMainThread(); - ASDisplayNodeAssert(_state == ASAsyncTransactionStateOpen, @"You cannot double-commit a transaction"); - _state = ASAsyncTransactionStateCommitted; + ASDisplayNodeAssert(__atomic_load_n(&_state, __ATOMIC_SEQ_CST) == ASAsyncTransactionStateOpen, @"You cannot double-commit a transaction"); + __atomic_store_n(&_state, ASAsyncTransactionStateCommitted, __ATOMIC_SEQ_CST); if ([_operations count] == 0) { // Fast path: if a transaction was opened, but no operations were added, execute completion block synchronously. @@ -451,8 +451,8 @@ ASAsyncTransactionQueue & ASAsyncTransactionQueue::instance() - (void)completeTransaction { - if (_state != ASAsyncTransactionStateComplete) { - BOOL isCanceled = (_state == ASAsyncTransactionStateCanceled); + if (__atomic_load_n(&_state, __ATOMIC_SEQ_CST) != ASAsyncTransactionStateComplete) { + BOOL isCanceled = (__atomic_load_n(&_state, __ATOMIC_SEQ_CST) == ASAsyncTransactionStateCanceled); for (ASDisplayNodeAsyncTransactionOperation *operation in _operations) { [operation callAndReleaseCompletionBlock:isCanceled]; } @@ -460,7 +460,7 @@ ASAsyncTransactionQueue & ASAsyncTransactionQueue::instance() // Always set _state to Complete, even if we were cancelled, to block any extraneous // calls to this method that may have been scheduled for the next runloop // (e.g. if we needed to force one in this runloop with -waitUntilComplete, but another was already scheduled) - _state = ASAsyncTransactionStateComplete; + __atomic_store_n(&_state, ASAsyncTransactionStateComplete, __ATOMIC_SEQ_CST); if (_completionBlock) { _completionBlock(self, isCanceled); @@ -471,7 +471,7 @@ ASAsyncTransactionQueue & ASAsyncTransactionQueue::instance() - (void)waitUntilComplete { ASDisplayNodeAssertMainThread(); - if (_state != ASAsyncTransactionStateComplete) { + if (__atomic_load_n(&_state, __ATOMIC_SEQ_CST) != ASAsyncTransactionStateComplete) { if (_group) { ASDisplayNodeAssertTrue(_callbackQueue == dispatch_get_main_queue()); _group->wait(); @@ -481,9 +481,9 @@ ASAsyncTransactionQueue & ASAsyncTransactionQueue::instance() // commit ourselves via the group to avoid double-committing the transaction. // This is only necessary when forcing display work to complete before allowing the runloop // to continue, e.g. in the implementation of -[ASDisplayNode recursivelyEnsureDisplay]. - if (_state == ASAsyncTransactionStateOpen) { + if (__atomic_load_n(&_state, __ATOMIC_SEQ_CST) == ASAsyncTransactionStateOpen) { [_ASAsyncTransactionGroup commit]; - ASDisplayNodeAssert(_state != ASAsyncTransactionStateOpen, @"Transaction should not be open after committing group"); + ASDisplayNodeAssert(__atomic_load_n(&_state, __ATOMIC_SEQ_CST) != ASAsyncTransactionStateOpen, @"Transaction should not be open after committing group"); } // If we needed to commit the group above, -completeTransaction may have already been run. // It is designed to accommodate this by checking _state to ensure it is not complete. @@ -508,7 +508,7 @@ ASAsyncTransactionQueue & ASAsyncTransactionQueue::instance() - (NSString *)description { - return [NSString stringWithFormat:@"<_ASAsyncTransaction: %p - _state = %lu, _group = %p, _operations = %@>", self, (unsigned long)_state, _group, _operations]; + return [NSString stringWithFormat:@"<_ASAsyncTransaction: %p - _state = %lu, _group = %p, _operations = %@>", self, (unsigned long)__atomic_load_n(&_state, __ATOMIC_SEQ_CST), _group, _operations]; } @end diff --git a/AsyncDisplayKit/Private/_AS-objc-internal.h b/AsyncDisplayKit/Private/_AS-objc-internal.h index b4635c63f5..196b3c8239 100644 --- a/AsyncDisplayKit/Private/_AS-objc-internal.h +++ b/AsyncDisplayKit/Private/_AS-objc-internal.h @@ -442,7 +442,7 @@ typedef enum { -(BOOL)_tryRetain { \ __typeof__(_rc_ivar) _prev; \ do { \ - _prev = _rc_ivar; \ + _prev = __atomic_load_n(&_rc_ivar, __ATOMIC_SEQ_CST);; \ if (_prev & 1) { \ return 0; \ } else if (_prev == -2) { \ @@ -454,12 +454,13 @@ typedef enum { return 1; \ } \ -(BOOL)_isDeallocating { \ - if (_rc_ivar == -2) { \ + __typeof__(_rc_ivar) _prev = __atomic_load_n(&_rc_ivar, __ATOMIC_SEQ_CST); \ + if (_prev == -2) { \ return 1; \ - } else if (_rc_ivar < -2) { \ + } else if (_prev < -2) { \ __builtin_trap(); /* BUG: over-release elsewhere */ \ } \ - return _rc_ivar & 1; \ + return _prev & 1; \ } #define _OBJC_SUPPORTED_INLINE_REFCNT_LOGIC(_rc_ivar, _dealloc2main) \ diff --git a/AsyncDisplayKitTests/ASDisplayLayerTests.m b/AsyncDisplayKitTests/ASDisplayLayerTests.m index e0d3b653b6..c0c2613d00 100644 --- a/AsyncDisplayKitTests/ASDisplayLayerTests.m +++ b/AsyncDisplayKitTests/ASDisplayLayerTests.m @@ -227,7 +227,12 @@ static _ASDisplayLayerTestDelegateClassModes _class_modes; // DANGER: Don't use the delegate as the parameters in real code; this is not thread-safe and just for accounting in unit tests! + (void)drawRect:(CGRect)bounds withParameters:(_ASDisplayLayerTestDelegate *)delegate isCancelled:(asdisplaynode_iscancelled_block_t)sentinelBlock isRasterizing:(BOOL)isRasterizing { - delegate->_drawRectCount++; + __atomic_add_fetch(&delegate->_drawRectCount, 1, __ATOMIC_SEQ_CST); +} + +- (NSUInteger)drawRectCount +{ + return(__atomic_load_n(&_drawRectCount, __ATOMIC_SEQ_CST)); } - (void)dealloc @@ -267,9 +272,9 @@ static _ASDisplayLayerTestDelegateClassModes _class_modes; // make sure we don't lock up the tests indefinitely; fail after 1 sec by using an async barrier __block BOOL didHitBarrier = NO; dispatch_barrier_async([_ASDisplayLayer displayQueue], ^{ - didHitBarrier = YES; + __atomic_store_n(&didHitBarrier, YES, __ATOMIC_SEQ_CST); }); - XCTAssertTrue(ASDisplayNodeRunRunLoopUntilBlockIsTrue(^BOOL{ return didHitBarrier; })); + XCTAssertTrue(ASDisplayNodeRunRunLoopUntilBlockIsTrue(^BOOL{ return __atomic_load_n(&didHitBarrier, __ATOMIC_SEQ_CST); })); } - (void)waitForLayer:(_ASDisplayLayerTestLayer *)layer asyncDisplayCount:(NSUInteger)count diff --git a/AsyncDisplayKitTests/ASDisplayNodeAppearanceTests.m b/AsyncDisplayKitTests/ASDisplayNodeAppearanceTests.m index b7273893da..f350eb1b93 100644 --- a/AsyncDisplayKitTests/ASDisplayNodeAppearanceTests.m +++ b/AsyncDisplayKitTests/ASDisplayNodeAppearanceTests.m @@ -60,11 +60,11 @@ static dispatch_block_t modifyMethodByAddingPrologueBlockAndReturnCleanupBlock(C @end // Conveniences for making nodes named a certain way -#define DeclareNodeNamed(n) ASDisplayNode *n = [[ASDisplayNode alloc] init]; n.name = @#n +#define DeclareNodeNamed(n) ASDisplayNode *n = [[[ASDisplayNode alloc] init] autorelease]; n.name = @#n #define DeclareViewNamed(v) UIView *v = viewWithName(@#v) static UIView *viewWithName(NSString *name) { - ASDisplayNode *n = [[ASDisplayNode alloc] init]; + ASDisplayNode *n = [[[ASDisplayNode alloc] init] autorelease]; n.name = name; return n.view; } @@ -130,7 +130,7 @@ static UIView *viewWithName(NSString *name) { - (void)checkAppearanceMethodsCalledWithRootNodeInWindowLayerBacked:(BOOL)isLayerBacked { // ASDisplayNode visibility does not change if modifying a hierarchy that is not in a window. So create one and add the superview to it. - UIWindow *window = [[UIWindow alloc] initWithFrame:CGRectZero]; + UIWindow *window = [[[UIWindow alloc] initWithFrame:CGRectZero] autorelease]; DeclareNodeNamed(n); DeclareViewNamed(superview); @@ -162,15 +162,12 @@ static UIView *viewWithName(NSString *name) { XCTAssertEqual([_willEnterHierarchyCounts countForObject:n], 1u, @"willEnterHierarchy not called when node's view added to hierarchy"); XCTAssertEqual([_didExitHierarchyCounts countForObject:n], 1u, @"didExitHierarchy erroneously called"); - - [superview release]; - [window release]; } - (void)checkManualAppearanceViewLoaded:(BOOL)isViewLoaded layerBacked:(BOOL)isLayerBacked { // ASDisplayNode visibility does not change if modifying a hierarchy that is not in a window. So create one and add the superview to it. - UIWindow *window = [[UIWindow alloc] initWithFrame:CGRectZero]; + UIWindow *window = [[[UIWindow alloc] initWithFrame:CGRectZero] autorelease]; DeclareNodeNamed(parent); DeclareNodeNamed(a); @@ -263,13 +260,13 @@ static UIView *viewWithName(NSString *name) { - (void)testSynchronousIntermediaryView { // Parent is a wrapper node for a scrollview - ASDisplayNode *parentSynchronousNode = [[ASDisplayNode alloc] initWithViewClass:[UIScrollView class]]; + ASDisplayNode *parentSynchronousNode = [[[ASDisplayNode alloc] initWithViewClass:[UIScrollView class]] autorelease]; DeclareNodeNamed(layerBackedNode); DeclareNodeNamed(viewBackedNode); layerBackedNode.layerBacked = YES; - UIWindow *window = [[UIWindow alloc] initWithFrame:CGRectZero]; + UIWindow *window = [[[UIWindow alloc] initWithFrame:CGRectZero] autorelease]; [parentSynchronousNode addSubnode:layerBackedNode]; [parentSynchronousNode addSubnode:viewBackedNode]; @@ -303,11 +300,6 @@ static UIView *viewWithName(NSString *name) { XCTAssertFalse(parentSynchronousNode.inHierarchy, @"Should not have changed"); XCTAssertFalse(layerBackedNode.inHierarchy, @"Should have been marked invisible when synchronous superview was removed from the window"); XCTAssertFalse(viewBackedNode.inHierarchy, @"Should have been marked invisible when synchronous superview was removed from the window"); - - [window release]; - [parentSynchronousNode release]; - [layerBackedNode release]; - [viewBackedNode release]; } - (void)checkMoveAcrossHierarchyLayerBacked:(BOOL)isLayerBacked useManualCalls:(BOOL)useManualDisable useNodeAPI:(BOOL)useNodeAPI diff --git a/AsyncDisplayKitTests/ASDisplayNodeTests.m b/AsyncDisplayKitTests/ASDisplayNodeTests.m index c85b7d3180..86529730f3 100644 --- a/AsyncDisplayKitTests/ASDisplayNodeTests.m +++ b/AsyncDisplayKitTests/ASDisplayNodeTests.m @@ -21,18 +21,18 @@ #import "ASCellNode.h" // Conveniences for making nodes named a certain way -#define DeclareNodeNamed(n) ASDisplayNode *n = [[ASDisplayNode alloc] init]; n.name = @#n +#define DeclareNodeNamed(n) ASDisplayNode *n = [[[ASDisplayNode alloc] init] autorelease]; n.name = @#n #define DeclareViewNamed(v) UIView *v = viewWithName(@#v) #define DeclareLayerNamed(l) CALayer *l = layerWithName(@#l) static UIView *viewWithName(NSString *name) { - ASDisplayNode *n = [[ASDisplayNode alloc] init]; + ASDisplayNode *n = [[[ASDisplayNode alloc] init] autorelease]; n.name = name; return n.view; } static CALayer *layerWithName(NSString *name) { - ASDisplayNode *n = [[ASDisplayNode alloc] init]; + ASDisplayNode *n = [[[ASDisplayNode alloc] init] autorelease]; n.layerBacked = YES; n.name = name; return n.layer; @@ -144,6 +144,12 @@ for (ASDisplayNode *n in @[ nodes ]) {\ { if (_willDeallocBlock) { _willDeallocBlock(self); + [_willDeallocBlock release]; + _willDeallocBlock = nil; + } + if (_calculateSizeBlock) { + [_calculateSizeBlock release]; + _calculateSizeBlock = nil; } [super dealloc]; } @@ -214,19 +220,19 @@ for (ASDisplayNode *n in @[ nodes ]) {\ } - (void)testOverriddenFirstResponderBehavior { - ASTestDisplayNode *node = [[ASTestResponderNode alloc] init]; + ASTestDisplayNode *node = [[[ASTestResponderNode alloc] init] autorelease]; XCTAssertTrue([node canBecomeFirstResponder]); XCTAssertTrue([node becomeFirstResponder]); } - (void)testDefaultFirstResponderBehavior { - ASTestDisplayNode *node = [[ASTestDisplayNode alloc] init]; + ASTestDisplayNode *node = [[[ASTestDisplayNode alloc] init] autorelease]; XCTAssertFalse([node canBecomeFirstResponder]); XCTAssertFalse([node becomeFirstResponder]); } - (void)testLayerBackedFirstResponderBehavior { - ASTestDisplayNode *node = [[ASTestResponderNode alloc] init]; + ASTestDisplayNode *node = [[[ASTestResponderNode alloc] init] autorelease]; node.layerBacked = YES; XCTAssertTrue([node canBecomeFirstResponder]); XCTAssertFalse([node becomeFirstResponder]); @@ -250,6 +256,8 @@ for (ASDisplayNode *n in @[ nodes ]) {\ [self executeOffThread:^{ node = [[ASDisplayNode alloc] init]; }]; + // executeOffThread: blocks until the background thread finishes executing. + node = [node autorelease]; // XXX This is very bad style. UIView *view = node.view; XCTAssertNotNil(view, @"Getting node's view on-thread should succeed."); @@ -257,7 +265,7 @@ for (ASDisplayNode *n in @[ nodes ]) {\ - (void)testNodeCreatedOffThreadWithExistingView { - UIView *view = [[UIDisplayNodeTestView alloc] init]; + UIView *view = [[[UIDisplayNodeTestView alloc] init] autorelease]; __block ASDisplayNode *node = nil; [self executeOffThread:^{ @@ -265,6 +273,8 @@ for (ASDisplayNode *n in @[ nodes ]) {\ return view; }]; }]; + // executeOffThread: blocks until the background thread finishes executing. + node = [node autorelease]; // XXX This is very bad style. XCTAssertFalse(node.layerBacked, @"Can't be layer backed"); XCTAssertTrue(node.synchronous, @"Node with plain view should be synchronous"); @@ -283,6 +293,9 @@ for (ASDisplayNode *n in @[ nodes ]) {\ return view; }]; }]; + // executeOffThread: blocks until the background thread finishes executing. + view = [view autorelease]; // XXX This is very bad style. + node = [node autorelease]; // XXX This is very bad style. XCTAssertNil(view, @"View block should not be invoked yet"); [node view]; @@ -293,10 +306,10 @@ for (ASDisplayNode *n in @[ nodes ]) {\ - (void)testNodeCreatedWithLazyAsyncView { - ASDisplayNode *node = [[ASDisplayNode alloc] initWithViewBlock:^UIView *{ + ASDisplayNode *node = [[[ASDisplayNode alloc] initWithViewBlock:^UIView *{ XCTAssertTrue([NSThread isMainThread], @"View block must run on the main queue"); - return [[_ASDisplayView alloc] init]; - }]; + return [[[_ASDisplayView alloc] init] autorelease]; + }] autorelease]; XCTAssertThrows([node view], @"Externally provided views should be synchronous"); XCTAssertTrue(node.synchronous, @"Node with externally provided view should be synchronous"); @@ -631,7 +644,7 @@ for (ASDisplayNode *n in @[ nodes ]) {\ // Perform parallel updates of a standard UIView/CALayer and an ASDisplayNode and ensure they are equivalent. - (void)testDeriveFrameFromBoundsPositionAnchorPoint { - UIView *plainView = [[UIView alloc] initWithFrame:CGRectZero]; + UIView *plainView = [[[UIView alloc] initWithFrame:CGRectZero] autorelease]; plainView.layer.anchorPoint = CGPointMake(0.25f, 0.75f); plainView.layer.position = CGPointMake(10, 20); plainView.layer.bounds = CGRectMake(0, 0, 60, 80); @@ -643,6 +656,8 @@ for (ASDisplayNode *n in @[ nodes ]) {\ node.bounds = CGRectMake(0, 0, 60, 80); node.position = CGPointMake(10, 20); }]; + // executeOffThread: blocks until the background thread finishes executing. + node = [node autorelease]; // XXX This is very bad style. XCTAssertTrue(CGRectEqualToRect(plainView.frame, node.frame), @"Node frame should match UIView frame before realization."); XCTAssertTrue(CGRectEqualToRect(plainView.frame, node.view.frame), @"Realized view frame should match UIView frame."); @@ -651,7 +666,7 @@ for (ASDisplayNode *n in @[ nodes ]) {\ // Perform parallel updates of a standard UIView/CALayer and an ASDisplayNode and ensure they are equivalent. - (void)testSetFrameSetsBoundsPosition { - UIView *plainView = [[UIView alloc] initWithFrame:CGRectZero]; + UIView *plainView = [[[UIView alloc] initWithFrame:CGRectZero] autorelease]; plainView.layer.anchorPoint = CGPointMake(0.25f, 0.75f); plainView.layer.frame = CGRectMake(10, 20, 60, 80); @@ -661,6 +676,8 @@ for (ASDisplayNode *n in @[ nodes ]) {\ node.anchorPoint = CGPointMake(0.25f, 0.75f); node.frame = CGRectMake(10, 20, 60, 80); }]; + // executeOffThread: blocks until the background thread finishes executing. + node = [node autorelease]; // XXX This is very bad style. XCTAssertTrue(CGPointEqualToPoint(plainView.layer.position, node.position), @"Node position should match UIView position before realization."); XCTAssertTrue(CGRectEqualToRect(plainView.layer.bounds, node.bounds), @"Node bounds should match UIView bounds before realization."); @@ -921,7 +938,7 @@ for (ASDisplayNode *n in @[ nodes ]) {\ - (void)testDisplayNodePointConversionOnDeepHierarchies { - ASDisplayNode *node = [[ASDisplayNode alloc] init]; + ASDisplayNode *node = [[[ASDisplayNode alloc] init] autorelease]; // 7 deep (six below root); each one positioned at position = (1, 1) _addTonsOfSubnodes(node, 2, 6, ^(ASDisplayNode *createdNode) { @@ -1108,7 +1125,7 @@ static inline BOOL _CGPointEqualToPointWithEpsilon(CGPoint point1, CGPoint point - (void)testSubnodes { - ASDisplayNode *parent = [[ASDisplayNode alloc] init]; + ASDisplayNode *parent = [[[ASDisplayNode alloc] init] autorelease]; ASDisplayNode *nilNode = nil; XCTAssertNoThrow([parent addSubnode:nilNode], @"Don't try to add nil, but we'll deal."); XCTAssertNoThrow([parent addSubnode:parent], @"Not good, test that we recover"); @@ -1211,11 +1228,6 @@ static inline BOOL _CGPointEqualToPointWithEpsilon(CGPoint point1, CGPoint point XCTAssertNodesHaveParent(nilParent, a,d); //TODO: assert that things deallocate immediately and don't have latent autoreleases in here - [parent release]; - [a release]; - [b release]; - [c release]; - [d release]; } - (void)testInsertSubnodeAtIndexView @@ -1344,17 +1356,12 @@ static inline BOOL _CGPointEqualToPointWithEpsilon(CGPoint point1, CGPoint point XCTAssertNodesHaveParent(nilParent, d); //TODO: assert that things deallocate immediately and don't have latent autoreleases in here - [parent release]; - [a release]; - [b release]; - [c release]; - [d release]; } // This tests our resiliancy to having other views and layers inserted into our view or layer - (void)testInsertSubviewAtIndexWithMeddlingViewsAndLayersViewBacked { - ASDisplayNode *parent = [[ASDisplayNode alloc] init]; + ASDisplayNode *parent = [[[ASDisplayNode alloc] init] autorelease]; DeclareNodeNamed(a); DeclareNodeNamed(b); @@ -1389,12 +1396,6 @@ static inline BOOL _CGPointEqualToPointWithEpsilon(CGPoint point1, CGPoint point XCTAssertEqual(4u, parent.layer.sublayers.count, @"Should have the right sublayer count"); //TODO: assert that things deallocate immediately and don't have latent autoreleases in here - [parent release]; - [a release]; - [b release]; - [c release]; - [d release]; - [e release]; } - (void)testAppleBugInsertSubview @@ -1467,11 +1468,6 @@ static inline BOOL _CGPointEqualToPointWithEpsilon(CGPoint point1, CGPoint point XCTAssertEqual(4u, parent.layer.sublayers.count, @"Should have the right sublayer count"); //TODO: assert that things deallocate immediately and don't have latent autoreleases in here - [parent release]; - [a release]; - [b release]; - [c release]; - [d release]; } @@ -1550,10 +1546,6 @@ static inline BOOL _CGPointEqualToPointWithEpsilon(CGPoint point1, CGPoint point XCTAssertNodesHaveParent(parent, a, c, b); //TODO: assert that things deallocate immediately and don't have latent autoreleases in here - [parent release]; - [a release]; - [b release]; - [c release]; } - (void)testInsertSubnodeAboveWithView @@ -1632,10 +1624,6 @@ static inline BOOL _CGPointEqualToPointWithEpsilon(CGPoint point1, CGPoint point XCTAssertNodesHaveParent(parent, a, c, b); //TODO: assert that things deallocate immediately and don't have latent autoreleases in here - [parent release]; - [a release]; - [b release]; - [c release]; } - (void)testRemoveFromViewBackedLoadedSupernode @@ -1702,6 +1690,9 @@ static inline BOOL _CGPointEqualToPointWithEpsilon(CGPoint point1, CGPoint point child = [[ASDisplayNode alloc] init]; [parent addSubnode:child]; }]; + // executeOffThread: blocks until the background thread finishes executing. + parent = [parent autorelease]; // XXX This is very bad style. + child = [child autorelease]; // XXX This is very bad style. XCTAssertEqual(1, parent.subnodes.count, @"Parent should have 1 subnode"); XCTAssertEqualObjects(parent, child.supernode, @"Child has the wrong parent"); @@ -1714,14 +1705,14 @@ static inline BOOL _CGPointEqualToPointWithEpsilon(CGPoint point1, CGPoint point - (void)testSubnodeAddedAfterLoadingExternalView { - UIView *view = [[UIDisplayNodeTestView alloc] init]; - ASDisplayNode *parent = [[ASDisplayNode alloc] initWithViewBlock:^{ + UIView *view = [[[UIDisplayNodeTestView alloc] init] autorelease]; + ASDisplayNode *parent = [[[ASDisplayNode alloc] initWithViewBlock:^{ return view; - }]; + }] autorelease]; [parent view]; - ASDisplayNode *child = [[ASDisplayNode alloc] init]; + ASDisplayNode *child = [[[ASDisplayNode alloc] init] autorelease]; [parent addSubnode:child]; XCTAssertEqual(1, parent.subnodes.count, @"Parent should have 1 subnode"); @@ -1851,7 +1842,7 @@ static inline BOOL _CGPointEqualToPointWithEpsilon(CGPoint point1, CGPoint point - (void)testInitWithViewClass { - ASDisplayNode *scrollNode = [[ASDisplayNode alloc] initWithViewClass:[UIScrollView class]]; + ASDisplayNode *scrollNode = [[[ASDisplayNode alloc] initWithViewClass:[UIScrollView class]] autorelease]; XCTAssertFalse(scrollNode.isLayerBacked, @"Can't be layer backed"); XCTAssertFalse(scrollNode.nodeLoaded, @"Shouldn't have a view yet"); @@ -1866,7 +1857,7 @@ static inline BOOL _CGPointEqualToPointWithEpsilon(CGPoint point1, CGPoint point - (void)testInitWithLayerClass { - ASDisplayNode *transformNode = [[ASDisplayNode alloc] initWithLayerClass:[CATransformLayer class]]; + ASDisplayNode *transformNode = [[[ASDisplayNode alloc] initWithLayerClass:[CATransformLayer class]] autorelease]; XCTAssertTrue(transformNode.isLayerBacked, @"Created with layer class => should be layer-backed by default"); XCTAssertFalse(transformNode.nodeLoaded, @"Shouldn't have a view yet"); @@ -1949,7 +1940,7 @@ static bool stringContainsPointer(NSString *description, const void *p) { - (void)testBounds { - ASDisplayNode *node = [[ASDisplayNode alloc] init]; + ASDisplayNode *node = [[[ASDisplayNode alloc] init] autorelease]; node.bounds = CGRectMake(1, 2, 3, 4); node.frame = CGRectMake(5, 6, 7, 8); @@ -1961,7 +1952,7 @@ static bool stringContainsPointer(NSString *description, const void *p) { - (void)testDidEnterDisplayIsCalledWhenNodesEnterDisplayRange { - ASTestDisplayNode *node = [[ASTestDisplayNode alloc] init]; + ASTestDisplayNode *node = [[[ASTestDisplayNode alloc] init] autorelease]; [node recursivelySetInterfaceState:ASInterfaceStateDisplay]; @@ -1970,7 +1961,7 @@ static bool stringContainsPointer(NSString *description, const void *p) { - (void)testDidExitDisplayIsCalledWhenNodesExitDisplayRange { - ASTestDisplayNode *node = [[ASTestDisplayNode alloc] init]; + ASTestDisplayNode *node = [[[ASTestDisplayNode alloc] init] autorelease]; [node recursivelySetInterfaceState:ASInterfaceStateDisplay]; [node recursivelySetInterfaceState:ASInterfaceStateFetchData]; @@ -1980,7 +1971,7 @@ static bool stringContainsPointer(NSString *description, const void *p) { - (void)testDidEnterFetchDataIsCalledWhenNodesEnterFetchDataRange { - ASTestDisplayNode *node = [[ASTestDisplayNode alloc] init]; + ASTestDisplayNode *node = [[[ASTestDisplayNode alloc] init] autorelease]; [node recursivelySetInterfaceState:ASInterfaceStateFetchData]; @@ -1989,7 +1980,7 @@ static bool stringContainsPointer(NSString *description, const void *p) { - (void)testDidExitFetchDataIsCalledWhenNodesExitFetchDataRange { - ASTestDisplayNode *node = [[ASTestDisplayNode alloc] init]; + ASTestDisplayNode *node = [[[ASTestDisplayNode alloc] init] autorelease]; [node recursivelySetInterfaceState:ASInterfaceStateFetchData]; [node recursivelySetInterfaceState:ASInterfaceStateDisplay]; diff --git a/AsyncDisplayKitTests/ASMultiplexImageNodeTests.m b/AsyncDisplayKitTests/ASMultiplexImageNodeTests.m index 3da2c43875..9b7b325660 100644 --- a/AsyncDisplayKitTests/ASMultiplexImageNodeTests.m +++ b/AsyncDisplayKitTests/ASMultiplexImageNodeTests.m @@ -90,8 +90,22 @@ static BOOL ASRunRunLoopUntilBlockIsTrue(BOOL (^block)()) { [super setUp]; - _mockCache = [OCMockObject mockForProtocol:@protocol(ASImageCacheProtocol)]; - _mockDownloader = [OCMockObject mockForProtocol:@protocol(ASImageDownloaderProtocol)]; + _mockCache = [[OCMockObject mockForProtocol:@protocol(ASImageCacheProtocol)] retain]; + _mockDownloader = [[OCMockObject mockForProtocol:@protocol(ASImageDownloaderProtocol)] retain]; +} + +- (void)tearDown +{ + if(_mockCache) { + [_mockCache release]; + _mockCache = nil; + } + if(_mockDownloader) { + [_mockDownloader release]; + _mockDownloader = nil; + } + + [super tearDown]; } - (void)testDataSourceImageMethod