Crash

[opk12]

It's a while I did not use StreetComplete. I opened it, it was at a very low zoom (showed Europe, Africa, part of America) and I zoomed a bit around South Egypt. The app crashed.

java.lang.RuntimeException: Tried to perform an operation on an invalid pointer! This means you may have used a MapData that has already been removed.
at com.mapzen.tangram.MapData.checkPointer(MapData.java:116)
at com.mapzen.tangram.MapData.setFeatures(MapData.java:36)
at de.westnordost.streetcomplete.screens.main.map.components.DownloadedAreaMapComponent.set(DownloadedAreaMapComponent.kt:32)
at de.westnordost.streetcomplete.screens.main.map.DownloadedAreaManager$update$1.invokeSuspend(DownloadedAreaManager.kt:51)
at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:106)
at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:584)
at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:793)
at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:697)
at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:684)
Suppressed: kotlinx.coroutines.internal.DiagnosticCoroutineContextException: [StandaloneCoroutine{Cancelling}@6afc11a, Dispatchers.Default]

Versions:
StreetComplete 53.3 from F-droid on Android 7.1

[Helium314]

Displaying the downloaded area sometime triggers crashes on startup. I think there also is a different one leading to crashes in the same line DownloadedAreaMapComponent.set(DownloadedAreaMapComponent.kt:32).

I tried a few things to fix it, but since the error is rather elusive, it takes a while to notice that a fix attempt is actually not working.
Currently I'm trying to have the set synchronized...

[opk12]

Currently I'm trying to have the set synchronized...

Regardless of this reported crash: Multi-threaded access to shared resources must always be inside synchronized (or the field must be volatile, which is effectively a shorthand) because the JVM (this is not Kotlin-specific) can legally

• reorder instructions (next line is executed before previous line) as long as it does not cross the synchronized boundaries
• have each thread work on its local copy of the heap and see random past updates from other threads (synchronized guarantees the sync of all heap copies across threads)
• (not relevant here, but funny: non-atomically read/write long or double fields)

More discussion and examples of what can go wrong are in the JLS section Memory model; from a quick google, Kotlin appears not to change all this (seems just an unreadable syntactic sugar around the same JVM?) so if that code is multithreaded, that should be synchronized right away.

As a general rule, it's synchronized(this.some_private_object) {...} not synchronized(this) { ... } just to make sure a client can't synchronize on the same object, messing with the lock.

These and other common mistakes are covered in the well-known book Effective Java by Joshua Bloch (I think that synchronization is Item 66) which is a must-read both for Java and Kotlin.

[tapetis]

Synchronization alone does not solve this problem as it only prevents concurrent accesses. If a MapData is disposed in one thread, other threads must still check that the data isn't disposed before calling any function on it when it's their turn to access the MapData. However, this is not currently supported by tangram. For details, see #2879.

[Helium314]

I managed to reproduce the crash by starting SC completely zoomed out with location off. Opening the undo history zooms to the position of the last edit and triggers the crash (a slow phone and a debug APK might help).

Using synchronized indeed seems to help (never crashed).

[tapetis]

Yes, you're right. In this case it's probably the following line disposing the MapData of a previous set call being executed concurrently on another thread:

StreetComplete/app/src/main/java/de/westnordost/streetcomplete/screens/main/map/components/DownloadedAreaMapComponent.kt

Line 18 in c2d325e

layer?.remove()

I thought that the problem may be related to some Android lifecycle changes where the entire tangram MapController is disposed of.

[Helium314]

I thought that the problem may be related to some Android lifecycle changes where the entire tangram MapController is disposed of.

I assume that could still trigger the crash, but more rarely and (presumably) only while the app is being closed.

[westnordost]

Possibly fixed by #5269