Fix #3800: Mitigate indefinite crashes due to an initial crash or process death in low memory cases #3860
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There are a lot of details to cover here--see the PR for the complete context.
- Add missing codeowner - Add support for configuring base branch reference - Update CI for dev/alpha AAB builds to use 'develop' since there's no origin configured by default in the workflows
This is needed to open a PR on GitHub. This commit is being made so that the PR can start off in a broken Actions state. This also initially disables most non-Bazel workflows to make workflow iteration faster and less impacting on other team members.
This introduces a new mechanism for passing lists of tests to sharded test targets in CI, and hooks it up. No actual sharding is occurring yet. This led to some simplifications in the CI workflow since the script can be more dynamic in computing the full list of targets (which also works around a previous bug with instrumentation tests being run). Java proto lite also needed to be upgraded for the scripts to be able to use it. More testing/documentation needed as this functionality continues to expand.
This simply partitions bucketed groups of targets into chunks of 10 for each run. Only 3 buckets are currently retained to test sharding in CI before introducing full support.
Fixes some caching bucket and output bugs. Also, introduces while loop & keep_going to introduce resilience against app test build failures (or just test failures in general).
Also, enable other workflows. Note that CI shouldn't fully pass yet since some documentation and testing needs to be added yet, but this is meant to be a more realistic test of the CI environment before the PR is finished.
Adds a human-readable prefix to make the shards look a bit nicer. Also, adds more fine-tuned partitioning to bucket & reduce shard counts to improve overall timing. Will need to be tested in CI.
A newly computed variable wasn't updated to be used in an earlier change.
Add docstrings for proto.
…/oppia-android into add-bundles-proguard-build-flavors
See #3757 (comment) for context.
Neither 'mv -t' nor piping to mv work on OSX so we needed to find an alternative (in this case just trying to move everything). This actually works a bit better since it's doing a per-file move rather than accommodating for files that shouldn't be moved (which isn't an issue since the destination directory is different than the one containing the AAB file).
Documentation, thorough tests, and detailed description of these changes are still needed.
This demonstrates working string selection for system-based and overwritten app languages, including necessary activity recreation & layout direction overwriting. This also includes a bunch of Dagger infra refactoring so that some app layer packages can now be modularized (including the new packages).
…o localization-part5-introduce-app-string-translations-support-and-refactor
This involves MANY broad changes to ensure consistent string retrieval (for arrays and plurals), formatting, and string transformations throughout the codebase. Some extra patterns to added to fix things that were needed, and a few issues were fixed along the way.
|
Reopening to force CI to fully restart (to avoid false passing CI check for base branch since it wasn't set when CI was kicked off). |
|
@rt4914 PTAL for codeowners. |
|
Hi @BenHenning, this PR is ready to be merged. Please address any remaining comments prior to merging, and feel free to merge this PR once the CI checks pass and you're happy with it. Thanks! |
…answer-translations-support Conflicts: domain/BUILD.bazel
…answer-translations-support
…tions-support' into localization-part7-add-gate-for-selecting-written-language-and-add-todos
…-written-language-and-add-todos
…age-and-add-todos' into localization-part8-add-support-for-new-ratio-classifier
Base automatically changed from
localization-part8-add-support-for-new-ratio-classifier
to
develop
October 6, 2021 23:51
BenHenning
changed the title
…tion-part8-add-support-for-new-ratio-classifier
…' into localization-part9-add-mechanism-to-recover-from-low-memory-process-end
…er-from-low-memory-process-end
|
Thanks @rt4914! Merging now. |
BenHenning
deleted the
localization-part9-add-mechanism-to-recover-from-low-memory-process-end
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix #3800
Explanation
This PR fixes #3800 by introducing a fallback mechanism in AppLanguageWatcherMixin to initialize the profile in AppLanguageLocaleHandler for cases when the app loses the locale that it makes an effort to keep. The two known cases when this can happen:
The fallback mechanism isn't great since the user experience may lead to extra recreations of the activity (see video below, the fallback locale is English so non-English cases will trigger a recreation). However, this seems like the only option since there's no obvious way to pass the locale's context across thread boundaries, or to retrieve a persisted copy of the locale without blocking the main thread (& potentially triggering an ANR).
Some things to note:
Essential Checklist
For UI-specific PRs only
This PR only affects crash scenarios, but the videos are really helpful in demonstrating the fixes.
Simulated in-app foreground crash without the fix:
https://user-images.githubusercontent.com/12983742/135440867-0f25696d-7db3-4ce4-af7b-f2ddee6717e0.mp4
Simulated in-app foreground crash with the fix:
force_crash_reset_mechanism_with_fix.mp4
Low-memory death scenarios. Notice that when the app closes/crashes, it's being triggered by a simulated low memory death. Note also that the app is killed after it's backgrounded to simulate a more realistic scenario where the system kills one app to free up resources for another.
Without fix:
low_memory_death_reset_mechanism_without_fix.mp4
With the fix:
low_memory_death_reset_mechanism_with_fix.mp4
(note that some of the 'without' fixes might look like they partially work since, I think, the system can choose to reopen the app from launcher rather than at the specific activity in certain circumstances).