Giter Site home page Giter Site logo

Comments (14)

unbiaseduser avatar unbiaseduser commented on June 17, 2024

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.

from sobriety.

KiARC avatar KiARC commented on June 17, 2024

@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.

from sobriety.

bibo38 avatar bibo38 commented on June 17, 2024

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.

from sobriety.

unbiaseduser avatar unbiaseduser commented on June 17, 2024

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.

from sobriety.

KiARC avatar KiARC commented on June 17, 2024

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.

from sobriety.

bibo38 avatar bibo38 commented on June 17, 2024

@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?

from sobriety.

unbiaseduser avatar unbiaseduser commented on June 17, 2024

@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...

from sobriety.

bibo38 avatar bibo38 commented on June 17, 2024

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.

from sobriety.

unbiaseduser avatar unbiaseduser commented on June 17, 2024

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.

from sobriety.

KiARC avatar KiARC commented on June 17, 2024

@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?

from sobriety.

KiARC avatar KiARC commented on June 17, 2024

Ah perhaps I am, wait a second

from sobriety.

KiARC avatar KiARC commented on June 17, 2024

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?

from sobriety.

KiARC avatar KiARC commented on June 17, 2024

Agh it's just not gonna work

from sobriety.

unbiaseduser avatar unbiaseduser commented on June 17, 2024

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.

from sobriety.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.