Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash on Android while in background #2432

Closed
Helium314 opened this issue May 26, 2024 · 6 comments · Fixed by #2759
Closed

Crash on Android while in background #2432

Helium314 opened this issue May 26, 2024 · 6 comments · Fixed by #2759
Labels
android bug Something isn't working

Comments

@Helium314
Copy link
Contributor

Helium314 commented May 26, 2024

Describe the bug
I used an app with MapLibre 11.0.0, then did something else, and on next time opening the app about an hour later I noticed it had crashed, probably while not using the phone.
Stack Trace (update: replaced with the strack trace from 11.1.0 as this one shows line numbers):

FATAL EXCEPTION: main
Process: de.westnordost.streetcomplete.expert, PID: 677
java.lang.IllegalMonitorStateException
at java.util.concurrent.locks.ReentrantLock$Sync.tryRelease(ReentrantLock.java:156)
at java.util.concurrent.locks.AbstractQueuedSynchronizer.release(AbstractQueuedSynchronizer.java:1291)
at java.util.concurrent.locks.ReentrantLock.unlock(ReentrantLock.java:466)
at org.maplibre.android.storage.FileSource.unlockPathLoaders(FileSource.java:367)
at org.maplibre.android.storage.FileSource.access$100(FileSource.java:31)
at org.maplibre.android.storage.FileSource$FileDirsPathsTask.onPostExecute(FileSource.java:224)
at org.maplibre.android.storage.FileSource$FileDirsPathsTask.onPostExecute(FileSource.java:204)
at android.os.AsyncTask.finish(AsyncTask.java:755)
at android.os.AsyncTask.access$900(AsyncTask.java:192)
at android.os.AsyncTask$InternalHandler.handleMessage(AsyncTask.java:772)
at android.os.Handler.dispatchMessage(Handler.java:107)
at android.os.Looper.loop(Looper.java:214)
at android.app.ActivityThread.main(ActivityThread.java:7356)
at java.lang.reflect.Method.invoke(Native Method)
at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:491)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:940)

There is no app code involved, so I don't know whether the crash might be related to mis-handling MapLibre somehow.

To Reproduce
Unclear how to reproduce it. Possibly triggered by some stuff Android does with background apps.
The stack trace might not be particularly useful, it doesn't even contain line numbers. Is there anything I can do to help finding out what is wrong?

Platform information (please complete the following information):

  • OS: Android 10 / LineageOS 17.1
  • Platform Android
  • Version 11.0.0
@Helium314 Helium314 added the bug Something isn't working label May 26, 2024
@westnordost
Copy link
Collaborator

westnordost commented May 26, 2024

I've had a short look at the source code here: https://github.com/maplibre/maplibre-native/blob/main/platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/storage/FileSource.java#L204

Such a crash happens when the reentrant lock is attempted to be unlocked on a different thread than it was locked. Looking at the code, this should be impossible, however, given the information that the app was in the background for a while, my guess is that Android assigns a different thread for the UI thread when the application is re-launched.
So, the FileDirsPathsTask was started, never finished, and then one hour later, onCancelled was called, but on a different thread.

Note also that AsyncTask is deprecated (for reasons like this). Documentation recommends to replace that with Kotlin coroutines with lifecycle components. (But, well, as long as you did not fully migrate to Kotlin yet, this makes little sense.)

For now, I'd recommend to just catch that exception with a note comment about that Android doesn't guarantee that the UI thread when the AsyncTask started has the same thread id when it ended if the app was not in foreground for a while.

@westnordost
Copy link
Collaborator

westnordost commented May 26, 2024

FYI in Kotlin with coroutines, if one must offload getting the cache dir to an IO thread, it would look like this for internalCachePath

private val internalCachePath: String? = null
private val internalCachePathMutex = Mutex()

suspend fun getInternalCachePath(context: Context): String {
  internalCachePathMutex.withLock {
    if (internalCachePath == null) {
      withContext(IO) {
        internalCachePath = getCachePath(context)
      }
    }
    return internalCachePath
  }
}

(To initialize, just call the getter)

If there should not be a suspending function on the interface, the above would need to be wrapped in `runBlocking { }̀ to make the call blocking.

@louwers
Copy link
Collaborator

louwers commented May 27, 2024

Thanks a lot for the bug report @Helium314 and for the additional details and suggestions @westnordost 🙏

@Helium314
Copy link
Contributor Author

Updated the stack trace, the one from MapLibre 11.1.0 has line numbers.

@westnordost
Copy link
Collaborator

westnordost commented Aug 22, 2024

In our alpha test, it turns out that this happens very frequently - in the background or when the app has been in the background for a while.

Since the rework #2436 will only be included in MapLibre 12.x which might take a while until it is released/stable, I wonder if for MapLibre 11.x you'd accept a PR that just "fixes" this issue by catching and ignoring the exception?

@westnordost
Copy link
Collaborator

I've submitted a PR with what I believe to be an actual fix (not a "fix", as announced).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
android bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants