Merge pull request #1624 from maicki/FixDelegateProxyCrash

[Accessibility / iOS 8] Fix 8.0 and 8.1-only, VoiceOver crash when clearing delegate / dataSource for Table or Collection

AsyncDisplayKit/ASCollectionView.mm

@@ -343,10 +343,10 @@ - (void)setAsyncDataSource:(id<ASCollectionViewDataSource>)asyncDataSource
 {
   // Note: It's common to check if the value hasn't changed and short-circuit but we aren't doing that here to handle
   // the (common) case of nilling the asyncDataSource in the ViewController's dealloc. In this case our _asyncDataSource
-  // will return as nil (ARC magic) even though the _proxyDataSource still exists. It's really important to nil out
-  // super.dataSource in this case because calls to ASCollectionViewProxy will start failing and cause crashes.
-
-  super.dataSource = nil;
+  // will return as nil (ARC magic) even though the _proxyDataSource still exists. It's really important to hold a strong
+  // reference to the old dataSource in this case because calls to ASCollectionViewProxy will start failing and cause crashes.
+  NS_VALID_UNTIL_END_OF_SCOPE id oldDataSource = super.dataSource;
+
   if (asyncDataSource == nil) {
     _asyncDataSource = nil;
     _proxyDataSource = _isDeallocating ? nil : [[ASCollectionViewProxy alloc] initWithTarget:nil interceptor:self];
@@ -375,13 +375,9 @@ - (void)setAsyncDelegate:(id<ASCollectionViewDelegate>)asyncDelegate
 {
   // Note: It's common to check if the value hasn't changed and short-circuit but we aren't doing that here to handle
   // the (common) case of nilling the asyncDelegate in the ViewController's dealloc. In this case our _asyncDelegate
-  // will return as nil (ARC magic) even though the _proxyDelegate still exists. It's really important to nil out
-  // super.delegate in this case because calls to ASCollectionViewProxy will start failing and cause crashes.
-
-  // Order is important here, the asyncDelegate must be callable while nilling super.delegate to avoid random crashes
-  // in UIScrollViewAccessibility.
-
-  super.delegate = nil;
+  // will return as nil (ARC magic) even though the _proxyDataSource still exists. It's really important to hold a strong
+  // reference to the old delegate in this case because calls to ASCollectionViewProxy will start failing and cause crashes.
+  NS_VALID_UNTIL_END_OF_SCOPE id oldDelegate = super.delegate;
 
   if (asyncDelegate == nil) {
     _asyncDelegate = nil;

AsyncDisplayKit/ASTableView.mm

@@ -268,10 +268,9 @@ - (void)setAsyncDataSource:(id<ASTableViewDataSource>)asyncDataSource
 {
   // Note: It's common to check if the value hasn't changed and short-circuit but we aren't doing that here to handle
   // the (common) case of nilling the asyncDataSource in the ViewController's dealloc. In this case our _asyncDataSource
-  // will return as nil (ARC magic) even though the _proxyDataSource still exists. It's really important to nil out
-  // super.dataSource in this case because calls to ASTableViewProxy will start failing and cause crashes.
-
-  super.dataSource = nil;
+  // will return as nil (ARC magic) even though the _proxyDataSource still exists. It's really important to hold a strong
+  // reference to the old dataSource in this case because calls to ASTableViewProxy will start failing and cause crashes.
+  NS_VALID_UNTIL_END_OF_SCOPE id oldDataSource = self.dataSource;
 
   if (asyncDataSource == nil) {
     _asyncDataSource = nil;
@@ -299,13 +298,9 @@ - (void)setAsyncDelegate:(id<ASTableViewDelegate>)asyncDelegate
 {
   // Note: It's common to check if the value hasn't changed and short-circuit but we aren't doing that here to handle
   // the (common) case of nilling the asyncDelegate in the ViewController's dealloc. In this case our _asyncDelegate
-  // will return as nil (ARC magic) even though the _proxyDelegate still exists. It's really important to nil out
-  // super.delegate in this case because calls to ASTableViewProxy will start failing and cause crashes.
-
-  // Order is important here, the asyncDelegate must be callable while nilling super.delegate to avoid random crashes
-  // in UIScrollViewAccessibility.
-
-  super.delegate = nil;
+  // will return as nil (ARC magic) even though the _proxyDataSource still exists. It's really important to hold a strong
+  // reference to the old delegate in this case because calls to ASTableViewProxy will start failing and cause crashes.
+  NS_VALID_UNTIL_END_OF_SCOPE id oldDelegate = super.delegate;
 
   if (asyncDelegate == nil) {
     _asyncDelegate = nil;

AsyncDisplayKit/Details/ASDelegateProxy.m

@@ -125,7 +125,15 @@ - (id)forwardingTargetForSelector:(SEL)aSelector
     if (_target) {
       return [_target respondsToSelector:aSelector] ? _target : nil;
     } else {
-      [_interceptor proxyTargetHasDeallocated:self];
+      // The _interceptor needs to be nilled out in this scenario. For that a strong reference needs to be created
+      // to be able to nil out the _interceptor but still let it know that the proxy target has deallocated
+      // We have to hold a strong reference to the interceptor as we have to nil it out and call the proxyTargetHasDeallocated
+      // The reason that the interceptor needs to be nilled out is that there maybe a change of a infinite loop, for example
+      // if a method will be called in the proxyTargetHasDeallocated: that again would trigger a whole new forwarding cycle
+      id <ASDelegateProxyInterceptor> interceptor = _interceptor;
+      _interceptor = nil;
+      [interceptor proxyTargetHasDeallocated:self];
+
       return nil;
     }
   }