Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

MGLOfflineStorage calls DefaultFileSource::resume() more than once #8464

Closed
boundsj opened this issue Mar 18, 2017 · 3 comments
Closed

MGLOfflineStorage calls DefaultFileSource::resume() more than once #8464

boundsj opened this issue Mar 18, 2017 · 3 comments
Assignees
Labels
crash iOS Mapbox Maps SDK for iOS release blocker Blocks the next final release
Milestone

Comments

@boundsj
Copy link
Contributor

boundsj commented Mar 18, 2017

Platform: iOS
Mapbox SDK version: v3.5.0-beta.2, v3.5.0-beta.3, v3.5.0-rc.1

#8194 and #8125 introduced a mechanism to pause and resume mbgl file source activity. #8125 specifically modified MGLOfflineStorage to pause the file source in response to UIApplicationDidEnterBackgroundNotification and resume in response to UIApplicationWillEnterForegroundNotification.

Based on a crash reported captured from an internal testing application (see below), it appears that the resume method is called twice in some cases which leads to a crash.

Thread 0 name:
Thread 0 Crashed:
0   libc++.1.dylib                	0x000000018060e37c std::__1::promise<void>::set_value() + 12 (future.cpp:246)
1   Mapbox                        	0x0000000100409734 mbgl::DefaultFileSource::resume() + 24
2   Mapbox                        	0x0000000100409734 mbgl::DefaultFileSource::resume() + 24
3   Mapbox                        	0x00000001003c9c1c _hidden#8730_ + 24 (__hidden#8849_:59)
4   CoreFoundation                	0x0000000181beab10 __CFNOTIFICATIONCENTER_IS_CALLING_OUT_TO_AN_OBSERVER__ + 20 

cc @mapbox/ios

@boundsj boundsj added crash iOS Mapbox Maps SDK for iOS release blocker Blocks the next final release labels Mar 18, 2017
@boundsj boundsj added this to the ios-v3.5.1 milestone Mar 18, 2017
@boundsj boundsj self-assigned this Mar 18, 2017
@boundsj boundsj modified the milestones: ios-v3.5.0, ios-v3.5.1 Mar 18, 2017
@boundsj
Copy link
Contributor Author

boundsj commented Mar 18, 2017

Noting that it does not look like it is unheard of for observers of life cycle notifications to be tricked by the system https://openradar.appspot.com/14075101.

I think our mbgl implementation is correct to enforce symmetry in the calls to pause and resume. Along with some extra logging, I'm going to try adding a boolean in MGLOfflineStorage to keep the state of pause and resume and guard against invalid calls to mbgl. This will hopefully allow me to confirm that this type of solution stops the crash and, unfortunately, it is the only solution I can think of at the moment.

@1ec5
Copy link
Contributor

1ec5 commented Mar 18, 2017

Noting that it does not look like it is unheard of for observers of life cycle notifications to be tricked by the system https://openradar.appspot.com/14075101.

This reminds me of how we had to work around the fact that iOS 9.0 (and above?) would send redundant reachability notifications on launch: #1911. In #1958, we similarly worked around the issue by keeping track of the old and new reachability states.

@boundsj
Copy link
Contributor Author

boundsj commented Mar 19, 2017

This was fixed in #8465 on the release-ios-v3.5.0-android-v5.0.0 branch.

@boundsj boundsj closed this as completed Mar 19, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
crash iOS Mapbox Maps SDK for iOS release blocker Blocks the next final release
Projects
None yet
Development

No branches or pull requests

2 participants