Possible corruption of Relations in v22.0

[westnordost]

Yesterday evening I got this crash report which freaked me out a little:

de.westnordost.osmapi.common.errors.OsmBadUserInputException: Bad Request (400) - Placeholder way not found for reference -1019767 in relation 1757370
at de.westnordost.osmapi.OsmConnection.handleResponseCode(OsmConnection.java:14)
at de.westnordost.osmapi.OsmConnection.makeRequest(OsmConnection.java:3)
at de.westnordost.osmapi.map.MapDataDao.uploadChanges(MapDataDao.java:3)
at de.westnordost.streetcomplete.data.osm.osmquest.SingleOsmElementTagChangesUploader.upload(SingleOsmElementTagChangesUploader.kt:7)
at de.westnordost.streetcomplete.data.osm.osmquest.OsmQuestsUploader.uploadSingle(OsmQuestsUploader.kt:3)
at de.westnordost.streetcomplete.data.osm.upload.OsmInChangesetsUploader.uploadSingle(OsmInChangesetsUploader.kt:3)
at de.westnordost.streetcomplete.data.osm.upload.OsmInChangesetsUploader.upload(OsmInChangesetsUploader.kt:5)
at de.westnordost.streetcomplete.data.osm.osmquest.OsmQuestsUploader.upload(OsmQuestsUploader.kt:2)
at de.westnordost.streetcomplete.data.upload.UploadService.onHandleIntent(UploadService.kt:11)
at android.app.IntentService$ServiceHandler.handleMessage(IntentService.java:78)
at android.os.Handler.dispatchMessage(Handler.java:110)
at android.os.Looper.loop(Looper.java:219)
at android.os.HandlerThread.run(HandlerThread.java:67)

So, someone was trying to upload his changes on a relation and the OSM API rejected it because the ref was bullocks. Other than on splitting a way, StreetComplete does not add relation members, so what must have happened is that for some reason, the downloaded relation with its relation members were serialized (or deserialized) wrongly, resulting in a wrong ref for the relation member. StreetComplete uses a library named Kryo for serialization, it serializes all the fields of objects via reflection into a binary blob. In v22.0 and v22.0-beta1 we had an array of weird problems and I think some, if not all of them may have been caused by R8, Google's code shrinker becoming more aggressive somehow. (Maybe I inadvertedly made a change that caused the behavior to change, though I checked the history and I don't see what may have caused it - minor update of the gradle build tools? Hopefully not!) R8 is used to remove unused code (classes, functions, ...) before compilation to make the resulting APK smaller. In short, what was new is that R8 seems to have started not only removing whole classes if they were unused but also pick out single fields of classes if they weren't used. This doesn't go well together with a serializer that is based on reflection, because usage through reflection is not detected by code shrinkers automatically.

So what I think was happening is that R8 removed an unused field (boolean modified) from OsmRelationMember (data structure to represent an osm relation member) and Kryo saved it in v22.0 that way. In v22.1 (and any version previous to v22.0-beta1), where I told R8 to not throw away my stuff, especially stuff that is serialized, the boolean modified field is back and Kryo happily deserializes the relation members downloaded in v22.0, but wrongly because on serialization, there was one bit more to serialize.

Results:

• Uploading a change made on a relation with v22.0 if the relation was downloaded with v22.0 should be fine
• Uploading a change made on a relation with v22.1 or any version previous to v22 probably results in this error

Learnings:

• (Still unknown how the change in behavior of the code shrinker was triggered)
• Reflection + Code Shrinker = 💥. Well, this is not new, but:
• Binary serialization through reflection is especially fragile, already without a code shrinker involved. Problems could have been avoided by using serialization to JSON or something. In long term, might switch from Kryo to Gson to avoid such problems in the future

Immediate actions to take:

1. Release v22.2 which is the same as v22.1 but once on startup deletes all relations and quests based on relations from local storage. The dust hasn't settled yet on the problems encountered with v22.0, but this one issue is critical enough to do an immediate bugfix release
2. Monitor relations updated with v22.0 for a while. Is there any tool that can filter changesets by having a relation in it?

[westnordost]

I'll keep this open for a while because it will take some time until everyone has v22.2.

Also, any hints on how to efficiently monitor relation changes made with v22.1 are appreciated

[matkoniecz]

I'll keep this open for a while because it will take some time until everyone has v22.2.

Maybe it would be a good idea to ban v22.1?

[westnordost]

v22.2 is released.

Unfortunately, the F-Droid build bot just scanned the repos yesterday, so v22.1 will shortly be on F-Droid and then it will take again up to ~three days for v22.2 to appear.

Maybe it would be a good idea to ban v22.1?

Yes, good idea. Even though the actual error is in v22.0, the corrupted data is carried over to v22.1 and "primed" there.

[matkoniecz]

What kind of corruption is expected? Complete garbage data, subtle data loss or is it possible to see both versions? I want to review my edits (made with private fork) and I am not sure how carefully I should look for corruption.

[westnordost]

Ok, let me summarize again:

You are only affected if in version 22.1, you uploaded answers to quests downloaded in version 22.0. If you uploaded quests in 22.0 or if you downloaded quests in 22.1 and uploaded them in that version, you are not affected. Furthermore, only quest answers of relations are affected, so in most cases this will be buildings with courtyards.

The consequence for those elements that are affected is, that either you get an error during upload (see first post) and nothing will break, or that the relation will be updated to reference completely different ways after upload.

So to identify any such situations, one has to look at all changesets uploaded with v22.1 that contain relations and see if the relation members changed.

[westnordost]

Okay, I spent the last two hours manually clicking through all 800-something changesets created with v22.1 and found no issues. There were in total only 5 relations that have been edited and they all look fine. v22.1 is now blocked for uploading, so the issue is solved and there should be no further repercussions.

[dbdean]

I can't see v22.2 in the beta testing on Google play store yet, but given your description above, and that I'm only uploading quests done in v22.1, I'm assuming I'm safe to keep using v22.1 until the new version arrives in the play store.

[westnordost]

It's rolled out in the release channel. See https://play.google.com/store/apps/details?id=de.westnordost.streetcomplete

It's safe to use v22.1, but you can not upload anything until you update to v22.2 because it is blocked.

[kmpoppe]

Hi Tobias, some members of the German Telegram Group have got a whole load of edits not uploaded (1500+). What would happen to them if they updated to 22.2? Will all local changes be gone? As 22.1 is now barred from uploading, they might be stuck in either not uploading with 22.1 or living in the fear of loosing everything with 22.2... What would be the best course of action?

[matkoniecz]

looking at https://github.com/westnordost/StreetComplete/commit/ec05b452c2c1922523a52980bb5a93ab400075ec it seems to me that only edits to relations will be lost.

So, in case of StreetComplete it should affect mostly buildings with courtyards and leaf tagging of complex forests.

Given that relations are relatively rarely edited with SC I would update and upload (maybe someone can do it first and confirm that it actually works this way).

[kmpoppe]

Hi Mateusz,

yes, that commit clearly shows what happens on database-level, thanks. I will relay that information.

Kai

[eiro]

streetcomplete can't connect to the server with 22.1, i tried to download 22.2: doesn't want to install. i downgraded it to 22.0 and lost all the quests i had no chance to sync.

[matkoniecz]

i tried to download 22.2: doesn't want to install

What you exactly mean by that?

[Atrate]

What you exactly mean by that?

They most probably tried to install GH SC over FDroid SC.

[eiro]

Mateusz, Atrate, sorry for the poor bug report.

On Sun, Aug 16, 2020 at 07:10:30AM -0700, Atrate wrote:

What you exactly mean by that? They most probably tried to install GH SC over FDroid SC.

sorry for the poor quality of this bug report. Atrate is right: as 22.2 isn't available from f-droid right now and i wanted to report new quests, i downloaded the asset from the github tag 22.2.

The only message i get is "Application not installed". no stacktrace, no bug report to send ...

regards marc

[westnordost]

In short: you need to uninstall the app first, then reinstall the build from GitHub. When you want to go back to F-Droid, you need to uninstall the GitHub build and reinstall from F-Droid.

Long version: Android executables (APKs) are always signed with a certificate to ensure that an update you download are actually from the same author. Builds from GitHub and Google Play are signed with my private key, while F-Droid builds and signs all apps itself. So the signatures of F-Droid builds are incompatible with the signatures of GitHub builds. This is why just updating the F-Droid version to the GitHub version fails

[eiro]

hi Tobias,

Thank you so much. i made it. all quests seems to be lost but at least it's working.

Also thanks for the clear explainations. too bad there is no clean error message there.

regards marc

[matkoniecz]

too bad there is no clean error message there.

Sadly, this informationless message is controlled by Android system, not by application

[eiro]

Sadly, this informationless message is controlled by Android system, not by application

one of the reasons i hate android: this is a blackbox to me.

thank you all marc

[Atrate]

You can get the error message's content if you try to install the application through a terminal (pm install). Wish it'd show the error in the GUI tho.

[eiro]

hello,

You can get the error message's content if you try to install the application through a terminal (pm install). Wish it'd show the error in the GUI tho.

i just installed termux and there actually is a pm command. i'll dig further next time.

thanks for this. regards

marc

[Atrate]

hello,

i just installed termux and there actually is a pm command. i'll dig further next time. thanks for this. regards marc

You might need root access to utilize pm install though.