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

Ignore deleting a non-existing offline region #8917

Closed
tobrun opened this issue May 8, 2017 · 2 comments
Closed

Ignore deleting a non-existing offline region #8917

tobrun opened this issue May 8, 2017 · 2 comments
Assignees
Labels
Android Mapbox Maps SDK for Android archived Archived because of inactivity crash offline

Comments

@tobrun
Copy link
Member

tobrun commented May 8, 2017

With the testapp, I was able to produce a native crash by opening Delete region example and clicking twice on an item to delete it (by accident, very fast after eachother). This resulted in deleting the same region twice. While this could be handled by the UI, I think it would be cleaner to ignore possible crashing updates or call into an onError callback instead.

05-08 10:59:19.780 4029-4029/com.mapbox.mapboxsdk.testapp A/libc: Fatal signal 11 (SIGSEGV), code 1, fault addr 0x4 in tid 4029 (pboxsdk.testapp)
                                                                  
                                                                  [ 05-08 10:59:19.782  1287: 1287 W/         ]
                                                                  debuggerd: handling request: pid=4029 uid=10073 gid=10073 tid=4029
05-08 10:59:19.847 4203-4203/? A/DEBUG: *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** ***
05-08 10:59:19.847 4203-4203/? A/DEBUG: Build fingerprint: 'Android/sdk_google_phone_x86/generic_x86:7.1.1/NYC/3756122:userdebug/test-keys'
05-08 10:59:19.847 4203-4203/? A/DEBUG: Revision: '0'
05-08 10:59:19.847 4203-4203/? A/DEBUG: ABI: 'x86'
05-08 10:59:19.847 4203-4203/? A/DEBUG: pid: 4029, tid: 4029, name: pboxsdk.testapp  >>> com.mapbox.mapboxsdk.testapp <<<
05-08 10:59:19.847 4203-4203/? A/DEBUG: signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x4
05-08 10:59:19.847 4203-4203/? A/DEBUG:     eax 901f8bac  ebx 901f8bac  ecx bf916908  edx 00000000
05-08 10:59:19.847 4203-4203/? A/DEBUG:     esi 00000000  edi c0443d01
05-08 10:59:19.847 4203-4203/? A/DEBUG:     xcs 00000073  xds 0000007b  xes 0000007b  xfs 0000003b  xss 0000007b
05-08 10:59:19.847 4203-4203/? A/DEBUG:     eip 8f5f8e4d  ebp bf9168f8  esp bf916790  flags 00010286
05-08 10:59:19.849 4203-4203/? A/DEBUG: backtrace:
05-08 10:59:19.849 4203-4203/? A/DEBUG:     #00 pc 001c5e4d  /data/app/com.mapbox.mapboxsdk.testapp-1/lib/x86/libmapbox-gl.so
05-08 10:59:19.849 4203-4203/? A/DEBUG:     #01 pc 001d7c0b  /data/app/com.mapbox.mapboxsdk.testapp-1/lib/x86/libmapbox-gl.so
05-08 10:59:19.849 4203-4203/? A/DEBUG:     #02 pc 001d7b24  /data/app/com.mapbox.mapboxsdk.testapp-1/lib/x86/libmapbox-gl.so
05-08 10:59:19.849 4203-4203/? A/DEBUG:     #03 pc 001d7a85  /data/app/com.mapbox.mapboxsdk.testapp-1/lib/x86/libmapbox-gl.so
05-08 10:59:19.849 4203-4203/? A/DEBUG:     #04 pc 001d7cd3  /data/app/com.mapbox.mapboxsdk.testapp-1/lib/x86/libmapbox-gl.so
05-08 10:59:19.849 4203-4203/? A/DEBUG:     #05 pc 001d7c75  /data/app/com.mapbox.mapboxsdk.testapp-1/lib/x86/libmapbox-gl.so
05-08 10:59:19.849 4203-4203/? A/DEBUG:     #06 pc 0055becc  /data/app/com.mapbox.mapboxsdk.testapp-1/oat/x86/base.odex (offset 0x51f000)
@tobrun tobrun added Android Mapbox Maps SDK for Android crash offline labels May 8, 2017
@tobrun tobrun changed the title Ignore deleting an non-existing offline region Ignore deleting a non-existing offline region May 8, 2017
@kkaefer kkaefer added the Core The cross-platform C++ core, aka mbgl label May 9, 2017
@tobrun tobrun removed the Android Mapbox Maps SDK for Android label May 19, 2017
@kkaefer
Copy link
Member

kkaefer commented Jan 23, 2018

I investigated this further, and I believe it is an Android-specific crash. The Android wrapper code stores an OfflineRegion object, and calls std::move when deleting:

fileSource.deleteOfflineRegion(std::move(*region), [

This means that the OfflineRegion object stored in the unique_ptr is now considered invalid (since we have moved away from it) and shouldn't be used anymore. The Android JNI wrapper should protect against this and return an error (or throw) when attempting to call delete multiple times, or when trying to do other operations on an OfflineRegion object after it has been deleted (like querying information from it, or adding observers).

@kkaefer kkaefer added Android Mapbox Maps SDK for Android and removed Core The cross-platform C++ core, aka mbgl labels Jan 23, 2018
@tobrun tobrun added this to the android-v6.0.0 milestone Jan 23, 2018
@tobrun tobrun self-assigned this Jan 23, 2018
@tobrun tobrun modified the milestones: android-v6.0.0, android-v6.1.0 Apr 13, 2018
@tobrun tobrun removed this from the android-v6.1.0 milestone May 11, 2018
@stale stale bot added the archived Archived because of inactivity label Nov 7, 2018
@stale
Copy link

stale bot commented Nov 28, 2018

This issue has been automatically detected as stale because it has not had recent activity and will be archived. Thank you for your contributions.

@stale stale bot closed this as completed Nov 28, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Android Mapbox Maps SDK for Android archived Archived because of inactivity crash offline
Projects
None yet
Development

No branches or pull requests

2 participants