Mapbox-gl-native: Crash when calling offlineRegion.getStatus in native layer (6.3.0-SNAPSHOT)

[BharathMG]

@tobrun Sometimes, calling offlineRegion.getStatus() crashes the app with the following stacktrace,

java.lang.Error: The associated promise has been destructed prior to the associated state becoming ready.
                         E      at com.mapbox.mapboxsdk.storage.FileSource.deactivate(Native Method)
                         E      at com.mapbox.mapboxsdk.offline.OfflineRegion$2$1.run(OfflineRegion.java:328)
                         E      at android.os.Handler.handleCallback(Handler.java:815)
                         E      at android.os.Handler.dispatchMessage(Handler.java:104)
                         E      at android.os.Looper.loop(Looper.java:207)
                         E      at android.app.ActivityThread.main(ActivityThread.java:5852)
                         E      at java.lang.reflect.Method.invoke(Native Method)
                         E      at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:789)
                         E      at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:679)

This looks like happens when multiple offlineRegion.getStatus calls are in flight.

Mapbox SDK versions: 6.3.0-SNAPSHOT

[vidsag]

@lilykaiser @tobrun Saw the same crash on 6.5.0

java.lang.Error: The associated promise has been destructed prior to the associated state becoming ready. at com.mapbox.mapboxsdk.storage.FileSource.deactivate(Native Method) at com.mapbox.mapboxsdk.offline.OfflineRegion$2$1.run(OfflineRegion.java:335) at android.os.Handler.handleCallback(Handler.java:815) at android.os.Handler.dispatchMessage(Handler.java:104) at android.os.Looper.loop(Looper.java:207) at android.app.ActivityThread.main(ActivityThread.java:5852) at java.lang.reflect.Method.invoke(Native Method) at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:789) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:679)

[zugaldia]

Adding more color to the original report, this seems to happen when getStatus and the offline region delete operation or fetch all regions (listOfflineRegions) are called in parallel. I see that we activate and deactivate the filesource singleton within these calls, this can be a race condition where a deactivated filesource (by a separate call) is passed to the native method.

[tobrun]

@BharathMG @vidsag thank you for both reaching out and using our products! I'm trying to reproduce this issue but so far without any luck. A couple of things I tried was constantly calling multiple getStatus calls inside a recurring runnable:

    final Handler handler = new Handler();
    handler.postDelayed(new Runnable() {
      @Override
      public void run() {
        offlineRegion.getStatus(new OfflineRegion.OfflineRegionStatusCallback() {
          @Override
          public void onStatus(OfflineRegionStatus status) {
            Timber.e("OfflineRegion:1 %s", status.getCompletedResourceCount());
          }

          @Override
          public void onError(String error) {
            Timber.e("OfflineRegion:1:error %s", error);
          }
        });

        offlineRegion.getStatus(new OfflineRegion.OfflineRegionStatusCallback() {
          @Override
          public void onStatus(OfflineRegionStatus status) {
            Timber.e("OfflineRegion:2 %s", status.getCompletedResourceCount());
          }

          @Override
          public void onError(String error) {
            Timber.e("OfflineRegion:2:error %s", error);
          }
        });
        handler.postDelayed(this, 100);
      }
    }, 100);

And doing the same but wrapping this in an async task.

The output I'm seeing for that is:

E/OfflineActivity: OfflineRegion:2 21
E/OfflineActivity: OfflineRegion:1 1
E/OfflineActivity: OfflineRegion:1 41
E/OfflineActivity: OfflineRegion:2 61
E/OfflineActivity: OfflineRegion:1 81
E/OfflineActivity: OfflineRegion:2 101
E/OfflineActivity: OfflineRegion:1 121
E/OfflineActivity: OfflineRegion:2 141
E/OfflineActivity: OfflineRegion:1 161
E/OfflineActivity: OfflineRegion:2 181
E/OfflineActivity: OfflineRegion:1 201
E/OfflineActivity: OfflineRegion:2 221
E/OfflineActivity: OfflineRegion:1 241
E/OfflineActivity: OfflineRegion:2 261
E/OfflineActivity: OfflineRegion:1 281
E/OfflineActivity: OfflineRegion:2 301
E/OfflineActivity: OfflineRegion:1 321
E/OfflineActivity: OfflineRegion:2 341
E/OfflineActivity: OfflineRegion:1 361
E/OfflineActivity: OfflineRegion:2 381
E/OfflineActivity: OfflineRegion:1 401
E/OfflineActivity: OfflineRegion:2 421
E/OfflineActivity: OfflineRegion:1 441
E/OfflineActivity: OfflineRegion:2 461
E/OfflineActivity: OfflineRegion:1 481
E/OfflineActivity: OfflineRegion:2 501
E/OfflineActivity: OfflineRegion:1 521
E/OfflineActivity: OfflineRegion:2 541
E/OfflineActivity: OfflineRegion:1 561
E/OfflineActivity: OfflineRegion:2 581
E/OfflineActivity: OfflineRegion:1 601
E/OfflineActivity: OfflineRegion:2 621

Would love to get to the bottom of this crash, any additional information on triggering this crash would be valuable to get this issue resolved! Thanks again for reaching out.

[BharathMG]

@tobrun The issue is while some other operation like listOfflineRegions or delete is running, if we do getStatus multiple times, it runs into fileSource.deactivate() race issue. I don't have reproducible code for now. I will update here once I have it.

I think I can reproduce it if I have different handlers running these two different operations at same time recursively.

[tobrun]

@BharathMG been trying a couple of things out, so far not able to hit the crash from OP but sometimes hitting terminating with uncaught exception of type std::runtime_error: Malformed offline region definition but this was with cases that and developer should never try, eg. deleting a region while it's downloading.

that said, I'm wondering about a couple of things here.

• are you calling OfflineRegion methods from the main thread?
• could you clarify why you are calling getStatus recursively while you can set an observer instead?

```
  offlineRegion.setObserver(new OfflineRegion.OfflineRegionObserver() {
    @Override
    public void onStatusChanged(OfflineRegionStatus status) {
    ...
    }
  ...
  });
```

[BharathMG]

@tobrun Thanks for trying out it on your end. I did not get enough time to spend on reproducing it reliably with code. But, I spent time on making all the calls to offline region (getStatus, delete) non-concurrent. After doing that, we again ran into the same issue which is fixed by #13146.

We've also come across Malformed Region crashes once in a while. But I currently don't have steps to reproduce it. We call delete only after status is changed to STATUS_INACTIVE. The pseudo code flow for deleting is like,

    fetchRegion(bounds) -> // fetches region from listOfflineRegions callback
      .then(setInactive(this)) // sets download state as inactive
      .then(delete(this)) // calls region.delete().

But this happens very rarely.

To answer your questions,
1) Yes. All the calls to OfflineManager happens on Main thread.
2) getStatus calls are only made before we want to make some operations like, download, delete or cancel. Here the flow is like,

   listAllRegions()
         .then(region -> getStatus(region))
         .then(checkIfNotDownloaded(status))
         .then(startDownload(region))

To fix concurrent issues, we changed all the concurrent calls to strictly serial. And even after that, this crash was reproducible when we did,

     delete(region)  -> Deletes region
        .then(listAllRegions()) -> Waits for onDelete and calls listAllRegions
        .then(getStatusForEachRegion())   -> Waits for getStatus callback. -> Crash happens here on `com.mapbox.mapboxsdk.offline.OfflineRegion$3$1.run(OfflineRegion.java:381)`. This is `fileSource.deactivate()` after `callback.onDelete()`.

[tobrun]

Fixed with https://github.com/mapbox/mapbox-gl-native/pull/13146, thanks for the contribution @BharathMG !

[djavid]

A/libc: /usr/local/google/buildbot/src/android/ndk-release-r19/external/libcxx/../../external/libcxxabi/src/abort_message.cpp:73: abort_message: assertion "terminating with uncaught exception of type std::runtime_error: Malformed offline region definition" failed
    Fatal signal 6 (SIGABRT), code -6 (SI_TKILL) in tid 26380 (DefaultFileSour), pid 26255

I am getting this error when try to stop download and delete. Since you do not provide callback on download state change to inactive (am I missing it?), I am not able to exactly know when i can start deleting region. Using getStatus helped to reduce errors: on first call i set download to inactive state, then call getStatus again to check status and delete. The problem is that I keep getting "Malformed offline region definition" error, because sometimes second getStatus call also shows state active.
How can I exactly know that download stopped?