Looking at latest SC master, modifyBBoxMapData() uses !! on a function which may return null, which would lead to java.lang.NullPointerException crash IIUC:
|
for (way in addWays) { |
|
// !!: It should not be possible that a node referred to by a way is missing, |
|
// as when a node is removed, it is removed from the way, too |
|
val nodes = getWayNodes(way)!! |
and getWayNodes() can definitely return null; it even does so explicitly in line 293:
|
private fun getWayNodes(way: Way): Collection<Node>? = lock.withLock { |
|
val ids = way.nodeIds.toSet() |
|
val nodes = getNodes(ids) |
|
|
|
// If the way is (now) not complete, this is not acceptable |
|
if (nodes.size < ids.size) return null |
|
|
|
return nodes |
|
} |
Do both those comments and code look correct / make sense? I don't know enough about internals, but I would guess it is not intentional that one function returns null, and other crashes if that happens.
But the main question is, do we really want app to crash here (and not be able to start up again, see below)?
- if so, we should really
Log.w() some more useful debug info for tracing it down before intentionally crashing
- if we do not want to crash (which I would suggest), we should better avoid using
!! here and instead skip the problematic way (better to lose one answer then all answers?) and log the error (perhaps also asking the user to report the whole log, so we can find the source of the bug?)
The bug actually manifested itself in SCEE (Helium314#830) with SCEE 61.1 and before, but I think it might also be happening in StreetComplete, as that particular code looks the same; and the fix (whatever it is) would thus best be done in the StreetComplete.
But as it happens quite rarely, I can not really reproduce it at will (in SC or SCEE); and as the crash might be intentional (e.g. to catch bugs?) I not opening this issue as a Bug...
Looking at latest SC master,
modifyBBoxMapData()uses!!on a function which may returnnull, which would lead tojava.lang.NullPointerExceptioncrash IIUC:StreetComplete/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/MapDataWithEditsSource.kt
Lines 438 to 441 in 0528ac5
and
getWayNodes()can definitely returnnull; it even does so explicitly in line293:StreetComplete/app/src/commonMain/kotlin/de/westnordost/streetcomplete/data/osm/edits/MapDataWithEditsSource.kt
Lines 288 to 296 in 0528ac5
Do both those comments and code look correct / make sense? I don't know enough about internals, but I would guess it is not intentional that one function returns null, and other crashes if that happens.
But the main question is, do we really want app to crash here (and not be able to start up again, see below)?
Log.w()some more useful debug info for tracing it down before intentionally crashing!!here and instead skip the problematic way (better to lose one answer then all answers?) and log the error (perhaps also asking the user to report the whole log, so we can find the source of the bug?)The bug actually manifested itself in SCEE (Helium314#830) with SCEE 61.1 and before, but I think it might also be happening in StreetComplete, as that particular code looks the same; and the fix (whatever it is) would thus best be done in the StreetComplete.
But as it happens quite rarely, I can not really reproduce it at will (in SC or SCEE); and as the crash might be intentional (e.g. to catch bugs?) I not opening this issue as a Bug...