Top and bottom layout guide KVO causes crashes in some scenarios #9941
Comments
|
It seems that the KVO approach is unsafe. The KVO + manual layout approach was cleaner in several ways but I think that the reintroduction of constraints in #9995 should work around the KVO related crashes we are seeing and is a reasonable approach given the limitations we are working with. We should land #9995 before shipping v3.6.3 cc @fabian-guerra |
|
This was fixed in #9995 that was merged into |
|
We just had one of our QA's reproduce the issue still, running with Mapbox 3.6.3 Including relevant part of the stack trace: Looking at your repo, the tag |
|
Not able to reproduce. The Mapbox.framework/Info.plist in the downloaded Mapbox pod has the expected value for the MGLCommitHash key (9613582). Also, dwarfdump of the dSYM file from the v3.6.3 pod does not find the problematic method that was removed ( This seems like it might be related to a CocoaPods cache issue or the Mapbox version not actually updating somehow. |
|
I looked at the Info.plist inside the Mapbox.framework in both the built app I sideloaded onto the QA devices, and the one in the Pods folder of the app. The Pod's version said the bundle's version string was 3.6.3, but the build product was 3.6.2. Looks like I didn't do a "build clean" before creating the sideload. Sorry for sending you on a wild goose chase. Or a snipe hunt. Or your metaphor of choice. Shaka, when the walls fell. |
|
No worries @Curtis-Halbrook! Thanks for the update. |
Summary
Crashes have been reported that are related to KVO observation of the
boundsproperty oftopLayoutGuideandbottomLayoutGuide. So far, these crashes have only been observed when using a combination of Xcode 9 beta 6 (although the beta version of Xcode 9 may not matter) and testing on a device with iOS 10.x. The exact version of iOS may not matter although it does appear that the combination of Xcode 9 beta (iOS 11 SDK) and pre-iOS 11 on device may trigger this issue.Details
The
MGLMapViewhas several subviews that are drawn on top of the map (like a compass) that must be moved with respect to map view container's chrome. For example, if a navigation bar is initially hidden and then shown, the compass at the top of the map view should move down to stay under the bottom of the navigation bar and not be covered by it.In #7716 we removed code that had been used until that point to solve for updating the map view subviews with a constraint system. The constraints were replaced with a manual view layout approach that is invoked when the containing view controller's top and bottom layout guides
boundsproperty changes. This is done by observing the layout guides with KVO and invokingsetNeedsLayoutonMGLMapViewwhen theboundsof the layout guides is observed to have changed. Setting needs layout results in a call to the map view's implementation oflayoutSubviewsthat, in turn, calls a method that updates the location of the relevant map view subviews based on the map view'scontentInsetthat is based on the latest top and bottom layout guides sizes.Using the layout guides is a cleaner approach, especially since it allows us to avoid manipulating the map view's containing view controller's view's constraints.
We did fix a related crasher in #9109 that happened because it was now possible for layout guides that had previously been observed to be deallocated before observation could be removed..
The fix was to implement layout guide observation removal in
-[MGLMapView willMoveToWindow:]. This appeared to have fixed the problem. However, using the combination of Xcode 9 beta and iOS 10.x, crashes like this have been reported again:Note that, in iOS 11, the top and bottom layout guides have been deprecated. From Xcode 9b1 release notes:
A good writeup of the new
Safe Area Layout Guideis available here.I've also observed that
willMoveToWindowis called multiple times depending on the view hierarchy the map view is embedded in. This is especially true if container views like aUIPageViewControlleris used. It does seem like it may be possible for a race condition to occur where a second or third call towillMoveToWindowunexpectedly re-reregisters observers on a view controller's layout guides just before that view controller and its guides are deallocated. This may be relevant to at least one related report that an observer was attempted to be removed even though it had not apparently been registered previously:Actions
cc @frederoni @jmkiley @fabian-guerra @1ec5
The text was updated successfully, but these errors were encountered: