babylonhealth / Bento

Swift library for building component-based interfaces on top of UITableView and UICollectionView 🍱
MIT License
374 stars 11 forks source link

[CNSMR-1748] Attempt to fix crash in NSISEngine on deinit #183

Closed AliSoftware closed 5 years ago

AliSoftware commented 5 years ago

Why?

Recent versions of Bento seem to crash in BentoReusableView.deinit, triggering autolayout (NSISEngine) while the view is being removed.

Here's one of the (partially demangled) stacktraces that happens when such a crash happens ``` 0 CoreFoundation __exceptionPreprocess + 331 1 libobjc.A.dylib objc_exception_throw + 48 2 CoreFoundation +[NSException raise:format:arguments:] + 98 3 Foundation -[NSAssertionHandler handleFailureInFunction:file:lineNumber:description:] + 166 4 Foundation -[NSISEngine removeConstraintWithMarker:] + 1651 5 Foundation -[NSLayoutConstraint _removeFromEngine:] + 229 6 UIKitCore __57-[UIView(AdditionalLayoutSupport) _switchToLayoutEngine:]_block_invoke + 217 7 Foundation -[NSISEngine withBehaviors:performModifications:] + 110 8 UIKitCore -[UIView(AdditionalLayoutSupport) _switchToLayoutEngine:] + 217 9 UIKitCore __57-[UIView(AdditionalLayoutSupport) _switchToLayoutEngine:]_block_invoke_2 + 213 10 UIKitCore __57-[UIView(AdditionalLayoutSupport) _switchToLayoutEngine:]_block_invoke + 571 11 Foundation -[NSISEngine withBehaviors:performModifications:] + 110 12 UIKitCore -[UIView(AdditionalLayoutSupport) _switchToLayoutEngine:] + 217 13 UIKitCore __57-[UIView(AdditionalLayoutSupport) _switchToLayoutEngine:]_block_invoke_2 + 213 14 UIKitCore __57-[UIView(AdditionalLayoutSupport) _switchToLayoutEngine:]_block_invoke + 571 15 Foundation -[NSISEngine withBehaviors:performModifications:] + 110 16 UIKitCore -[UIView(AdditionalLayoutSupport) _switchToLayoutEngine:] + 217 17 UIKitCore __57-[UIView(AdditionalLayoutSupport) _switchToLayoutEngine:]_block_invoke_2 + 213 18 UIKitCore __57-[UIView(AdditionalLayoutSupport) _switchToLayoutEngine:]_block_invoke + 571 19 Foundation -[NSISEngine withBehaviors:performModifications:] + 110 20 UIKitCore -[UIView(AdditionalLayoutSupport) _switchToLayoutEngine:] + 217 21 UIKitCore __45-[UIView(Hierarchy) _postMovedFromSuperview:]_block_invoke + 112 22 UIKitCore -[UIView(Hierarchy) _postMovedFromSuperview:] + 795 23 UIKitCore __UIViewWasRemovedFromSuperview + 172 24 UIKitCore -[UIView(Hierarchy) removeFromSuperview] + 493 25 Bento (extension in Bento):Bento.BentoReusableView.containerViewDidChange(from: __C.UIView?, to: __C.UIView?) -> () + 1094 26 Bento Bento.TableViewContainerCell.containedView.didset : __C.UIView? + 189 27 Bento Bento.TableViewContainerCell.containedView.setter : __C.UIView? + 288 28 Bento protocol witness for Bento.BentoReusableView.containedView.setter : __C.UIView? in conformance Bento.TableViewContainerCell : Bento.BentoReusableView in Bento + 9 29 Bento (extension in Bento):Bento.BentoReusableView.(unbindIfNeeded in _C920B861C8A0418300F3C0EC0904FFD1)(removesView: Swift.Bool) -> Bento.AnyRenderable? + 847 30 Bento (extension in Bento):Bento.BentoReusableView.unbindIfNeeded() + 72 31 Bento Bento.TableViewContainerCell.__deallocating_deinit + 79 32 Bento @objc Bento.TableViewContainerCell.__deallocating_deinit + 36 33 CoreFoundation -[__NSArrayM dealloc] + 159 34 libobjc.A.dylib _ZN11objc_object17sidetable_releaseEb + 202 35 UIKitCore -[UITableView .cxx_destruct] + 1561 36 libobjc.A.dylib _ZL27object_cxxDestructFromClassP11objc_objectP10objc_class + 127 37 libobjc.A.dylib objc_destructInstance + 134 38 libobjc.A.dylib object_dispose + 19 39 UIKitCore -[UIResponder dealloc] + 145 40 UIKitCore -[UIView dealloc] + 1041 41 UIKitCore -[UIScrollView dealloc] + 900 42 UIKitCore -[UITableView dealloc] + 473 43 libsystem_blocks.dylib _Block_release + 105 44 UIKitCore -[UIViewAnimationBlockDelegate .cxx_destruct] + 58 45 libobjc.A.dylib _ZL27object_cxxDestructFromClassP11objc_objectP10objc_class + 127 46 libobjc.A.dylib objc_destructInstance + 134 47 libobjc.A.dylib object_dispose + 19 48 libobjc.A.dylib _ZN11objc_object17sidetable_releaseEb + 202 49 CoreFoundation -[__NSDictionaryI dealloc] + 126 50 libobjc.A.dylib _ZN11objc_object17sidetable_releaseEb + 202 51 libobjc.A.dylib _ZN12_GLOBAL__N_119AutoreleasePoolPage3popEPv + 779 52 CoreFoundation _CFAutoreleasePoolPop + 22 53 CoreFoundation __CFRunLoopRun + 2318 54 CoreFoundation CFRunLoopRunSpecific + 626 55 XCTest -[XCTWaiter waitForExpectations:timeout:enforceOrder:] + 996 56 XCTest +[XCTWaiter waitForExpectations:timeout:enforceOrder:] + 104 57 XCTest -[XCTTestRunSession runTestsAndReturnError:] + 895 58 XCTest -[XCTestDriver runTestsAndReturnError:] + 446 59 XCTest _XCTestMain + 2333 60 xctest main + 263 61 libdyld.dylib start + 1 62 ??? 0x0 + 5 ```

The crash seems to be triggered by this sequence of events:

How

The proposed fix calls unbindIfNeeded(removesView: false) in deinit instead of just unbindIfNeeded() (which was internally calling unbindIfNeeded(removesView: true)).

This prevents the self.containedView = nil affectation and prevents triggering AutoLayout and the NSISEngine, thus preventing the crash

(This required unbindIfNeeded(removesView:) to be turned internal instead of private)

dmcrodrigues commented 5 years ago

@andersio @sergdort please have a look on this when you guys have time but it's going to be merged.