wordpress-mobile / WordPress-Android

WordPress for Android
http://android.wordpress.org
GNU General Public License v2.0
2.92k stars 1.3k forks source link

Crash: TransactionTooLargeException #5456

Closed oguzkocer closed 4 years ago

oguzkocer commented 7 years ago

The crash logs doesn't say much about where this crash occurs, but I did look into the TransactionTooLargeException a bit and it looks like it generally happens when huge amount of data is getting exchanged between a service and an application. So, I'd probably look into media related stuff first, since the amount of data would be the biggest in those operations.

Fatal Exception: java.lang.RuntimeException: android.os.TransactionTooLargeException: data parcel size 626308 bytes
       at android.app.ActivityThread$StopInfo.run(ActivityThread.java:3832)
       at android.os.Handler.handleCallback(Handler.java:751)
       at android.os.Handler.dispatchMessage(Handler.java:95)
       at android.os.Looper.loop(Looper.java:154)
       at android.app.ActivityThread.main(ActivityThread.java:6247)
       at java.lang.reflect.Method.invoke(Method.java)
       at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:872)
       at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:762)
Caused by android.os.TransactionTooLargeException: data parcel size 626308 bytes
       at android.os.BinderProxy.transactNative(Binder.java)
       at android.os.BinderProxy.transact(Binder.java:615)
       at android.app.ActivityManagerProxy.activityStopped(ActivityManagerNative.java:3678)
       at android.app.ActivityThread$StopInfo.run(ActivityThread.java:3824)
       at android.os.Handler.handleCallback(Handler.java:751)
       at android.os.Handler.dispatchMessage(Handler.java:95)
       at android.os.Looper.loop(Looper.java:154)
       at android.app.ActivityThread.main(ActivityThread.java:6247)
       at java.lang.reflect.Method.invoke(Method.java)
       at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:872)
       at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:762)
maxme commented 7 years ago

Apparently it only happens on Android 7.x devices

maxme commented 7 years ago

https://code.google.com/p/android/issues/detail?id=212316

maxme commented 7 years ago

I think we are experiencing this problem with the NotificationsDetailActivity. Timing looks to match this theory (number of crashes started to increase with the release of 6.9 that includes the notification detail paging feature).

oguzkocer commented 7 years ago

I think we are experiencing this problem with the NotificationsDetailActivity. Timing looks to match this theory (number of crashes started to increase with the release of 6.9 that includes the notification detail paging feature).

If I understand that issue correctly, it's due to the saved state's size, which seems to be small for NotificationsDetailActivity as we are only saving the title of the page and the id of the current notification.

I have checked all the onSaveInstanceStates and in general, I think we are doing a good job not adding too much info into the states. However, we do put models like the site and post as Serializable which I guess can get big. I think it'd be better if we just put the id of those models and fetch them from the DB.

Aside from that, I haven't noticed any particular class as the culprit, so not really sure what we should do about this particular crash.

maxme commented 7 years ago

@oguzkocer I was pointing to "comment 78" which is now "comment 77" (Google issue tracker changed recently), I updated the link above. And I'm pretty sure it's related to our use of FragmentStatePagerAdapter in Notifications (crash reports number increased after 6.9 release, when we introduced swiping between notifications).

oguzkocer commented 7 years ago

@oguzkocer I was pointing to "comment 78" which is now "comment 77" (Google issue tracker changed recently), I updated the link above. And I'm pretty sure it's related to our use of FragmentStatePagerAdapter in Notifications (crash reports number increased after 6.9 release, when we introduced swiping between notifications).

Oh, sorry, I think I missed that comment somehow. I am looking into it!

mzorz commented 7 years ago

This unfortuantely reappeared in 7.2 and 7.2.1, reopening

maxme commented 7 years ago

Another possible source: passing the PostModel in from the post list to the post editor. We should only pass the id (local) and reload it in the editor.

PostModel objects can be > 1Mb.

oguzkocer commented 6 years ago

It looks like we still have this issue, reopening..

maxme commented 6 years ago

Looking at some crash sessions, it seems the crash happens during edition in the hybrid editor. Last log line before the crash looks like:

69 | \| | 02:37:52:037 (UTC) | \| | D/CrashlyticsCore  d/WordPress-EDITOR: callback-response-string:  function=getHTMLForCallback~id=zss_field_title~contents=Saekano Girls  Side 3: The Main Heroine's Soliloquy
-- | -- | -- | -- | --
70 | \| | 02:37:52:038 (UTC) | \| | D/CrashlyticsCore d/WordPress-EDITOR: getContent() called from UI thread
71 | \| | 02:37:52:162 (UTC) | \| | .
72 | \| | 02:37:52:174 (UTC) | \| | D/CrashlyticsCore  d/WordPress-EDITOR: callback-response-string:  function=getHTMLForCallback~id=zss_field_title~contents=[]

I wonder if the problem comes from the underlying Editor Webview execJavaScriptFromString -> Webview evaluateJavascript

maxme commented 6 years ago

I tried to repro, but I hit another limit https://github.com/wordpress-mobile/WordPress-FluxC-Android/issues/517

maxme commented 6 years ago

I was able to open a 1Mb Post in the hybrid editor with Android 7.1.1, editing experience was very unresponsive (I was barely able to add characters), but nothing crashed.

Also tried with Aztec (much better experience but still unresponsive)

oguzkocer commented 6 years ago

I did similar tests and didn't get anywhere with them. The thing with TransactionTooLarge exception is that it's thrown for many different things and there is no useful trace for it. I was considering adding some logs around the app where we might think it's likely, but aside from local post preview (which we might remove soon) I can't think of much.

Also something to consider while testing is that this crash happens when the device is under load, so it's not actually app specific. Yes, it is possible that just passing a very large object in bundle might cause it, but it's very unlikely. It's probably more likely to happen when we pass large objects AND the device is under load.

oguzkocer commented 6 years ago

@maxme We have been struggling with this crash for a long time. How about we do something a bit radical and for the alpha builds add a log to onCreate, onResume, onPause, onDestroy of every activity/fragment and try to at least pinpoint in which screen(s) the crash is happening. We could be doing a witch hunt for a long time without understanding where the crash is.

maxme commented 6 years ago

I see 2 other references to serialized post (or page) that we should remove before trying other techniques:

oguzkocer commented 6 years ago

Hopefully this is fixed in #6399, so closing it for now. If we still have the crash, we'll re-open.

oguzkocer commented 6 years ago

Re-opening this as the crash is still happening after our last fix attempt.

mzorz commented 6 years ago

I think we should try what @oguzkocer suggested here before that is, adding logs, so we can gain a little more insight as to where this might be happening. I don't think performance would be hurt that much, and in any case we're going to remove that logging once we pinpoint what's happening. In the best case it's a couple of releases we do this for (1 cycle to get logs in and gather info, another cycle with a solution if we are able to repro and keep monitoring). Thoughts @maxme ?

hypest commented 6 years ago

FYI, there's at least one Fabric report with this happening on a Nexus 5X, running Android 7.1.1

khaykov commented 6 years ago

Not sure if this will be useful, but I did a bit of testing with this issue, and managed to consistently reproduce it in Visual Editor with 1mb and 460kb posts (250kb was ok). Crash happens when onSaveInstanceState in EditorFragment is called (turn on "Don't keep activities" to test it).

I also tried with a bunch of 60kb comments that make in total over 1mb, but we don't write them to bundle anywhere so everything was ok.

hypest commented 6 years ago

That's awesome @khaykov ! Can you please write up the steps to reproduce? For example, that 1MB and 460KB, are those just text or does it include images and such? Thanks!

maxme commented 6 years ago

Crash happens when onSaveInstanceState in EditorFragment is called (turn on "Don't keep activities" to test it).

Good call, I forgot we saved the title and content here, we made sure to remove the full PostModel save but didn't think about content being saved on its own. This is a problem for sure.

khaykov commented 6 years ago

Steps to reproduce:

  1. On the web write a huge post. I used plain text only (460kb and 1mb )
  2. On your device enable "Don't keep activities".
  3. Open the post in Visual Editor.
  4. Press Home button to leave application.
  5. Notice crash.
oguzkocer commented 6 years ago

I removed myself as assignee at least for now, which I should have done long time ago. I am just coming off of a long afk due to my move to Canada. I am probably not going to able to take a look at this again at least for a week again, so please feel free to take a stab at it. Hopefully last little bit that needs to be fixed is what @khaykov mentioned (thanks for that btw!).

byencho commented 6 years ago

Here's a thorough discussion of the issue : https://stackoverflow.com/questions/39098590/android-os-transactiontoolargeexception-on-nougat/44738411#44738411 . I created a little library to get around this : https://github.com/livefront/bridge . Basically, there's a limit to the amount of data you can save via onSavedInstanceState. If you start going over ~0.5MB of data you start to see the crash. My library fixes this by temporarily and safely saving all "saved state" data to disk so it can be restored from disk later by working in conjunction with the OS state saving mechanism.

designsimply commented 4 years ago

@maxme or @khaykov may we close this issue in favor of https://github.com/wordpress-mobile/WordPress-Android/issues/9685 ?

shiki commented 4 years ago

Closing. Along with #9685, this should be fixed by #10240.