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

Crashed on launch after F-Droid update to 7.0 #55

Closed
bibo38 opened this issue Nov 23, 2022 · 14 comments
Closed

Crashed on launch after F-Droid update to 7.0 #55

bibo38 opened this issue Nov 23, 2022 · 14 comments

Comments

@bibo38
Copy link

bibo38 commented Nov 23, 2022

Upgraded from the F-Droid 6.2.1 release and the App crashed on launch.
The App cache was already empty and I don't want to clear the actual data and set it up again 😒 .

Here is the startup exception:

11-23 16:33:54.950 15702 15702 E AndroidRuntime: java.lang.RuntimeException: Unable to start activity ComponentInfo{com.katiearose.sobriety/com.katiearose.sobriety.activities.Main}: java.io.InvalidClassException: j$.time.t; class invalid for deserialization
11-23 16:33:54.950 15702 15702 E AndroidRuntime:        at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:3280)
11-23 16:33:54.950 15702 15702 E AndroidRuntime:        at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:3419)
11-23 16:33:54.950 15702 15702 E AndroidRuntime:        at android.app.servertransaction.LaunchActivityItem.execute(LaunchActivityItem.java:83)
11-23 16:33:54.950 15702 15702 E AndroidRuntime:        at android.app.servertransaction.TransactionExecutor.executeCallbacks(TransactionExecutor.java:135)
11-23 16:33:54.950 15702 15702 E AndroidRuntime:        at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:95)
11-23 16:33:54.950 15702 15702 E AndroidRuntime:        at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2026)
11-23 16:33:54.950 15702 15702 E AndroidRuntime:        at android.os.Handler.dispatchMessage(Handler.java:107)
11-23 16:33:54.950 15702 15702 E AndroidRuntime:        at android.os.Looper.loop(Looper.java:214)
11-23 16:33:54.950 15702 15702 E AndroidRuntime:        at android.app.ActivityThread.main(ActivityThread.java:7400)
11-23 16:33:54.950 15702 15702 E AndroidRuntime:        at java.lang.reflect.Method.invoke(Native Method)
11-23 16:33:54.950 15702 15702 E AndroidRuntime:        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:491)
11-23 16:33:54.950 15702 15702 E AndroidRuntime:        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:935)
11-23 16:33:54.950 15702 15702 E AndroidRuntime: Caused by: java.io.InvalidClassException: j$.time.t; class invalid for deserialization
11-23 16:33:54.950 15702 15702 E AndroidRuntime:        at java.io.ObjectStreamClass$ExceptionInfo.newInvalidClassException(ObjectStreamClass.java:154)
11-23 16:33:54.950 15702 15702 E AndroidRuntime:        at java.io.ObjectStreamClass.checkDeserialize(ObjectStreamClass.java:798)
11-23 16:33:54.950 15702 15702 E AndroidRuntime:        at java.io.ObjectInputStream.readOrdinaryObject(ObjectInputStream.java:1873)
11-23 16:33:54.950 15702 15702 E AndroidRuntime:        at java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1412)
11-23 16:33:54.950 15702 15702 E AndroidRuntime:        at java.io.ObjectInputStream.readObject(ObjectInputStream.java:427)
11-23 16:33:54.950 15702 15702 E AndroidRuntime:        at java.util.HashMap.readObject(HashMap.java:1408)
11-23 16:33:54.950 15702 15702 E AndroidRuntime:        at java.lang.reflect.Method.invoke(Native Method)
11-23 16:33:54.950 15702 15702 E AndroidRuntime:        at java.io.ObjectStreamClass.invokeReadObject(ObjectStreamClass.java:1066)
11-23 16:33:54.950 15702 15702 E AndroidRuntime:        at java.io.ObjectInputStream.readSerialData(ObjectInputStream.java:2013)
11-23 16:33:54.950 15702 15702 E AndroidRuntime:        at java.io.ObjectInputStream.readOrdinaryObject(ObjectInputStream.java:1899)
11-23 16:33:54.950 15702 15702 E AndroidRuntime:        at java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1412)
11-23 16:33:54.950 15702 15702 E AndroidRuntime:        at java.io.ObjectInputStream.readObject(ObjectInputStream.java:427)
11-23 16:33:54.950 15702 15702 E AndroidRuntime:        at java.util.ArrayList.readObject(ArrayList.java:791)
11-23 16:33:54.950 15702 15702 E AndroidRuntime:        at java.lang.reflect.Method.invoke(Native Method)
11-23 16:33:54.950 15702 15702 E AndroidRuntime:        at java.io.ObjectStreamClass.invokeReadObject(ObjectStreamClass.java:1066)
11-23 16:33:54.950 15702 15702 E AndroidRuntime:        at java.io.ObjectInputStream.readSerialData(ObjectInputStream.java:2013)
11-23 16:33:54.950 15702 15702 E AndroidRuntime:        at java.io.ObjectInputStream.readOrdinaryObject(ObjectInputStream.java:1899)
11-23 16:33:54.950 15702 15702 E AndroidRuntime:        at java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1412)
11-23 16:33:54.950 15702 15702 E AndroidRuntime:        at java.io.ObjectInputStream.readObject(ObjectInputStream.java:427)
11-23 16:33:54.950 15702 15702 E AndroidRuntime:        at j1.a.a(Unknown Source:105)
11-23 16:33:54.950 15702 15702 E AndroidRuntime:        at com.katiearose.sobriety.activities.Main.onCreate(Unknown Source:148)
11-23 16:33:54.950 15702 15702 E AndroidRuntime:        at android.app.Activity.performCreate(Activity.java:7824)
11-23 16:33:54.950 15702 15702 E AndroidRuntime:        at android.app.Activity.performCreate(Activity.java:7813)
11-23 16:33:54.950 15702 15702 E AndroidRuntime:        at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1307)
11-23 16:33:54.950 15702 15702 E AndroidRuntime:        at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:3255)
11-23 16:33:54.950 15702 15702 E AndroidRuntime:        ... 11 more

Maybe this issue is related to #48 , but I'm wondering why it's not mentioned in the release notes (or fixed before the F-Droid release), as the Issue seems to be known for about a month.

@unbiaseduser
Copy link
Contributor

Ok, I just investigated and I noticed something interesting.
This crash only affects the release builds but not the debug ones (which is why I didn't spot it in development). It probably has something to do with APK shrinking, but I'm not certain.

@KiARC
Copy link
Owner

KiARC commented Nov 26, 2022

@bibo38 as far as i'm aware, the issue is caused by a new cache format being implemented, except this time around we didn't implement one. What @unbiaseduser said is likely the case, and a fix will be out in the near future if I can figure out how to implement one.

@bibo38
Copy link
Author

bibo38 commented Nov 30, 2022

Based on the info of @unbiaseduser , that this has to do smth. with the apk shrinking, I've analyzed the cause of the issue.

The exception told me about the invalid deserialization of the class j$.time.t. After disassembling the APKs for 6.2.1 and 7.0.0 i've found the following contents of the j$.time.t class:

6.2.1

package j$.time;

import java.io.Externalizable;
import java.io.InvalidClassException;
import java.io.ObjectInput;
import java.io.ObjectOutput;
import java.io.Serializable;
import java.io.StreamCorruptedException;

final class t implements Externalizable {
  private static final long serialVersionUID = -7683839454370182990L;
  
  private byte a;
  [...]

7.0.0

package j$.time;

import j$.time.temporal.ChronoUnit;
import j$.time.temporal.a;

Looking for the serialVersionUID, I found that it belongs to java.time.Ser. The java.time.Instant used in the code delegates serialization to this class.

So the issue is the obfuscation done to the release apk, which renames Classes (and their content fields), which breaks the serialization using ObjectOutputStream, as these names change in different builds.

Let's look at possible solutions:

  • Disable Obfuscation for the serialization classes
    • Easy to implement
    • Manual task
    • If a class is missed, it might get unnoticed until a further update. And fixing this ObjectStream with obfuscation to be properly migrated is not trivial.
  • Disable Obfuscation altogether
    • Easy to implement
    • Bigger APK files
  • Manually implement the serialization
    • Can use DataOutputStream for a portable serialization
    • Needs more coding for the manual serialization
  • Use a Database
    • E.g. Room, which is also used in popular Apps like NewPipe
    • Maybe to overkill

Personally I prefer the manual serialization for these few entries of the Addiction class, as the Object serialization is just too fragile in my opinion and will probably cause more work when migrating between versions than it saves. If everyone is ok with this, I would be willing to implement it and create a PR.

@unbiaseduser
Copy link
Contributor

Thanks for the input. Anyways, as I mentioned in #50, I created a branch that reworks the serialization mechanism to not use Serializable anymore, and yup, it doesn't suffer from obfuscation shenanigans.
However, the future of the branch is, for now, unknown.

@KiARC
Copy link
Owner

KiARC commented Nov 30, 2022

Thank you both so much, I've been fighting this since launch and I'm so happy this is likely solved. @unbiaseduser, if you want to PR the fix go ahead.

@bibo38
Copy link
Author

bibo38 commented Dec 25, 2022

@unbiaseduser I didn't understand, why you merged the cache fix into a special iOS-Branch. As you commented, this would solve this issue in ongoing releases, so why don't add it to the mainline? Could you please explain?

@unbiaseduser
Copy link
Contributor

@bibo38 It's completely rewritten and uses a different data format. It would be just straight up incompatible with any previous releases. It's also because of that, that I decided to not mainline it so Kate can work on iOS support (because why jump on the multiplatform train when you don't actually support multiple platforms.). The rationale is that since this is a major breaking change, we might as well take our time to make the app turn over a new leaf(?) as a way to make up for our past sins mistakes and make way for more good stuff going forward.
P.S. Sorry if something sounds strange, I just have difficulties expressing myself sometimes...

@bibo38
Copy link
Author

bibo38 commented Dec 26, 2022

It would be just straight up incompatible with any previous releases.

That's correct, but the current state of the App also has the problem of being incompatible to previous versions due to obfuscations while using the ObjectOutputStream. So migration strategies have to be implemented in order to get away from the current state (and older versions like 6.2.1, so I can upgrade without loosing my data). But migration strategies between versions is just a necessity (for a good user experience and no App crashes after (even old) version updates) and has to be considered even in the new data storage. See NewPipe as an example, where they have dedicated SQL scripts to migrate from even the first version of the database schema.

I would be willing to look into adding the necessary migration from the current broken ObjectOutputStream data, so that all data of any previous release can be migrated to the new format.

because why jump on the multiplatform train when you don't actually support multiple platforms

because it saves work, as you don't need to rebase every time a PR is merged. Given that the plan is to have a shared logic codebase, that doesn't use any Android or iOS specific APIs, you can easily just have this in the master branch. This allows further PRs to separate the logic straight away. The iOS specific things then may be developed in a separate branch and rebasing the branch is much easier, as you only have to patch the iOS codebase.

The rationale is that since this is a major breaking change, we might as well take our time

I agree, that the multiplatform thing is a big feature and doesn't need to be rushed. But the data storage format needs to be reworked in the next release to prevent crashing/lost data due to the obfuscation randomness. Given that the current release has some bugs, where some are already fixed/being fixed in PRs, you want to push these little bugfixes fast to make the user happy. But currently this isn't possible, as it will probably result in a crashing app, if the user has data from previous releases.

As I said, I would be willing to create a manual serialization PR with proper migrations to fix this problem, if you don't want to merge the JSON serialization thingy of the iOS release yet. At a later point it is then still possible to migrate the manual serialization to something else, if desired.

@unbiaseduser
Copy link
Contributor

As I said, I would be willing to create a manual serialization PR with proper migrations to fix this problem, if you don't want to merge the JSON serialization thingy of the iOS release yet.

Oh yeah, I briefly forgot that I still have to support migrating from 6.2 to 7.0. My bad.
Yeah, just go ahead with the manual serialization fix, I'll add support for JSON migration later.

@KiARC
Copy link
Owner

KiARC commented Jan 13, 2023

@unbiaseduser I have attempted to merge your changes (I made the dev branch use multiplatform code even though it lacks iOS support so far, I just wanna get that started) and it doesn't work.

E/AndroidRuntime: FATAL EXCEPTION: main
    Process: com.katiearose.sobriety, PID: 4550
    java.lang.RuntimeException: Unable to start activity ComponentInfo{com.katiearose.sobriety/com.katiearose.sobriety.activities.Main}: kotlinx.serialization.json.internal.JsonDecodingException: Expected start of the array '[', but had 'EOF' instead at path: $
    JSON input: .....�pݡ��:���?8q�����K�U��e����E�A�s��nوk�R�P���Z�������������������
        at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:3676)
        at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:3813)
        at android.app.servertransaction.LaunchActivityItem.execute(LaunchActivityItem.java:101)
        at android.app.servertransaction.TransactionExecutor.executeCallbacks(TransactionExecutor.java:135)
        at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:95)
        at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2308)
        at android.os.Handler.dispatchMessage(Handler.java:106)
        at android.os.Looper.loopOnce(Looper.java:201)
        at android.os.Looper.loop(Looper.java:288)
        at android.app.ActivityThread.main(ActivityThread.java:7898)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:936)
     Caused by: kotlinx.serialization.json.internal.JsonDecodingException: Expected start of the array '[', but had 'EOF' instead at path: $
    JSON input: .....�pݡ��:���?8q�����K�U��e����E�A�s��nوk�R�P���Z�������������������
        at kotlinx.serialization.json.internal.JsonExceptionsKt.JsonDecodingException(JsonExceptions.kt:24)
        at kotlinx.serialization.json.internal.JsonExceptionsKt.JsonDecodingException(JsonExceptions.kt:32)
        at kotlinx.serialization.json.internal.AbstractJsonLexer.fail(AbstractJsonLexer.kt:530)
        at kotlinx.serialization.json.internal.AbstractJsonLexer.fail$default(AbstractJsonLexer.kt:528)
        at kotlinx.serialization.json.internal.AbstractJsonLexer.fail$kotlinx_serialization_json(AbstractJsonLexer.kt:224)
        at kotlinx.serialization.json.internal.AbstractJsonLexer.unexpectedToken(AbstractJsonLexer.kt:207)
        at kotlinx.serialization.json.internal.StringJsonLexer.consumeNextToken(StringJsonLexer.kt:74)
        at kotlinx.serialization.json.internal.StreamingJsonDecoder.beginStructure(StreamingJsonDecoder.kt:97)
        at kotlinx.serialization.internal.AbstractCollectionSerializer.merge(CollectionSerializers.kt:29)
        at kotlinx.serialization.internal.AbstractCollectionSerializer.deserialize(CollectionSerializers.kt:43)
        at kotlinx.serialization.json.internal.StreamingJsonDecoder.decodeSerializableValue(StreamingJsonDecoder.kt:70)
        at kotlinx.serialization.json.Json.decodeFromString(Json.kt:95)
        at com.katiearose.sobriety.shared.CacheHandler.readCache(CacheHandler.kt:47)
        at com.katiearose.sobriety.activities.Main.onCreate(Main.kt:101)
        at android.app.Activity.performCreate(Activity.java:8290)
        at android.app.Activity.performCreate(Activity.java:8269)
        at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1384)
        at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:3657)
        	... 12 more

This is with a v8.0.0 cache. Am I doing something stupid?

@KiARC
Copy link
Owner

KiARC commented Jan 13, 2023

Ah perhaps I am, wait a second

@KiARC
Copy link
Owner

KiARC commented Jan 13, 2023

I'm trying to make it so that v9.0.0 will be able to read <=v8.0.0 caches instead of just being forwards compatible, but it's a royal pain with all the classes being changed to their kotlinx.datetime equivalents. Any ideas?

@KiARC
Copy link
Owner

KiARC commented Jan 13, 2023

Agh it's just not gonna work

@unbiaseduser
Copy link
Contributor

unbiaseduser commented Jan 13, 2023

My plan is to:

  • Create a "intermediate" JSON data class on both versions I just realized I can just copy the new Addiction class from the new version to the old one and have a mapper function to convert the old one to the new one
  • Add backup/restore feature on both versions which will convert the above class to their currently used forms (and vice versa) Actually I can just create a new JSON file behind the scenes in the old version and just check its existence in the new version and import from that. God, I tend to overthink stuff way too often.

In the meantime, don't touch the old version yet. This will take quite a while.

@KiARC KiARC closed this as completed Jan 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants