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

Make app resilient to state loss due to low-memory process deaths #2571

Open
BenHenning opened this issue Jan 27, 2021 · 11 comments
Open

Make app resilient to state loss due to low-memory process deaths #2571

BenHenning opened this issue Jan 27, 2021 · 11 comments
Labels
bug End user-perceivable behaviors which are not desirable. Impact: Medium Moderate perceived user impact (non-blocking bugs and general improvements). Issue: Needs Break-down Indicates that an issue is too large and should be broken into smaller chunks. Issue: Needs Clarification Indicates that an issue needs more detail in order to be able to be acted upon. Work: High It's not clear what the solution is. Z-ibt Temporary label for Ben to keep track of issues he's triaged.

Comments

@BenHenning
Copy link
Member

BenHenning commented Jan 27, 2021

Android will kill apps when they are backgrounded & the system runs low on memory. This manifests as a process death, and returning to the app (e.g. via backstack navigation or from the launcher) will trigger the app reopening & restoring via a bundle that the OS had captured & persisted on-disk prior to process death.

Note that we can simulate this scenario by (note that this requires a rooted device/emulator):

  • Run adb root (if your ADB isn't already in root mode; this may not be needed for emulators)
  • Running adb shell ps | grep oppia
  • Pass the PID found via the above to adb shell kill -9 <pid> to simulate a kill-on-OOM device

Non-rooted way to simulate this (thanks to https://stackoverflow.com/a/17829677):

  • Run adb shell ps -A | grep oppia
  • Run adb shell run-as org.oppia.android kill -9 <pid>

Bash one-liner:

adb shell run-as org.oppia.android kill -9 "$(adb shell ps -A | grep oppia | awk '{print $2}')"

(The current flow causes a crash in Oppia when in an exploration).

We can fix this issue by:

  • Introducing a common infrastructure in the domain layer to track all ephemeral app state
  • Introduce a mechanism that all Oppia activities can default opt into to pull/restore the app state (this will be easier after Add support for putting/getting protos in Bundles #2561 is merged in since all of the state can be represented via a single proto message)

This requires some architectural considerations both in the domain & app layer, so considering this to be a 'mini project'.

@prayutsu
Copy link
Contributor

@BenHenning I am interested to work on this.

@kartikeysaran
Copy link
Contributor

@BenHenning Can I work on this project if no one working on this !

@rt4914
Copy link
Contributor

rt4914 commented Feb 3, 2021

@prayutsu Can you please confirm if you are working on this or not, else it can be assigned to @kartikeysaran

@prayutsu
Copy link
Contributor

prayutsu commented Feb 4, 2021

@rt4914 As I am busy with other issues, I may not be able to prioritize this, so @kartikeysaran can take it. Thanks!

@kartikeysaran
Copy link
Contributor

@rt4914 , I am working on this issue

@FareesHussain
Copy link
Contributor

@kartikeysaran any updates on this?

@kartikeysaran
Copy link
Contributor

Was working on other issues will work on this

@BenHenning
Copy link
Member Author

@kartikeysaran I suggest starting with a starter or second issue before working on this. This project involves changing a lot of files across the codebase, and is likely going to be quite a bit trickier than starter issues. If after completing a few more issues you still want to work on this, please let us know. :)

@kartikeysaran
Copy link
Contributor

@kartikeysaran I suggest starting with a starter or second issue before working on this. This project involves changing a lot of files across the codebase, and is likely going to be quite a bit trickier than starter issues. If after completing a few more issues you still want to work on this, please let us know. :)

Okay i will work on some started issues , Yes i want to still work on this issue

@BenHenning
Copy link
Member Author

Idea: some state in the domain layer will require loading data from the filesystem. In addition to storing state in bundles (or perhaps in place of it), we may need to detect this state & temporarily transition to a loading activity that blocks on a special data provider which tracks restoring all domain layer state before returning & resuming in the UI.

@BenHenning
Copy link
Member Author

Another idea that maybe ties a bit better to UI components (and needs some thinking in how it interoperates with systems like Jetpack Compose and databinding): if we have assurances that all mutable data only exists in view models (though some thinking needs to be done for temporary references to activities, fragments, views, etc. since these can't go in view models) with proper view models implemented (per #1051), then we could introduce a serialization mechanism for view models that provides/derives from a proto (acting sort of similar to Android's parcelable). If we have a way to register these view models as UI-kept singletons, then the view model states can be serialized on disk for future proofing against process death.

This idea isn't necessarily exclusive from the one above. We still need a way to retain domain data, but it probably needs to happen more on a "session" basis.

One way to consolidate the two ideas might be to "register" session-tied lifecycle data both in the domain and UI layer such that the session's data can be serialized into one proto blob and restored from that blob. This is going to be highly dependent on the solution for #1051 as it's not yet clear how to hook up view models to the Dagger hierarchy since the current approach causes a context leak, and view models very likely need access to sub-singleton components in the graph.

@Broppia Broppia added issue_type_infrastructure Impact: Low Low perceived user impact (e.g. edge cases). labels Jul 29, 2022
@BenHenning BenHenning added Issue: Needs Break-down Indicates that an issue is too large and should be broken into smaller chunks. issue_user_developer and removed mini-project labels Sep 14, 2022
@BenHenning BenHenning added Impact: Medium Moderate perceived user impact (non-blocking bugs and general improvements). and removed Impact: Low Low perceived user impact (e.g. edge cases). labels Sep 16, 2022
@BenHenning BenHenning added Issue: Needs Clarification Indicates that an issue needs more detail in order to be able to be acted upon. Z-ibt Temporary label for Ben to keep track of issues he's triaged. labels Sep 16, 2022
@seanlip seanlip added bug End user-perceivable behaviors which are not desirable. and removed issue_type_infrastructure labels Mar 28, 2023
@MohitGupta121 MohitGupta121 added the Work: High It's not clear what the solution is. label Jun 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug End user-perceivable behaviors which are not desirable. Impact: Medium Moderate perceived user impact (non-blocking bugs and general improvements). Issue: Needs Break-down Indicates that an issue is too large and should be broken into smaller chunks. Issue: Needs Clarification Indicates that an issue needs more detail in order to be able to be acted upon. Work: High It's not clear what the solution is. Z-ibt Temporary label for Ben to keep track of issues he's triaged.
Development

No branches or pull requests

8 participants