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

Fix crash on scan state restore. #1131

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

davidgyoung
Copy link
Member

This is a work in progress on fixing problems in #1129.

Changes compile and unit tests still pass, but end-to-end testing has not started.

if (file.length() > SANITY_FILE_SIZE_LIMIT) {
// make sure file size is reasonable. If over 100k, do not restore
// See issue #1129
LogManager.e(TAG, "Refusing to restore file of size "+file.length());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we maybe delete STATUS_PRESERVATION_FILE_NAME if it is too big or corrupted?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the code does something similar as is because as soon as the file is persisted (typically after app processing a few milliseconds later) the scan state is persisted with the newly emptier state, overwriting the problematic file.

@paolorotolo
Copy link
Contributor

So I did some tests with multiple files and it correctly marks them as corrupted or skips them for excessive size. It seems it's working fine.

@davidgyoung davidgyoung changed the title WIP fixing crash on scan state restore. Fix crash on scan state restore. Mar 5, 2023
@davidgyoung
Copy link
Member Author

davidgyoung commented Mar 5, 2023

This is being published as 2.19.6-beta3. It will be in a non-beta release after I verify it fixes crashes in production in an app published to Google Play.

@liedQM
Copy link

liedQM commented May 27, 2024

@davidgyoung any update on this PR? I guess the verifying should have taken place now? Can you please also have a look, why restore is called from the MainThread causing ANRs? The restore is accessing a file so doing an IO operation and should never be called from the MainThread.

See here: #1189
We've observed a similar issue

@liedQM
Copy link

liedQM commented Nov 7, 2024

@davidgyoung do you have an update? We're still seeing the issue in our stack.

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

Successfully merging this pull request may close these issues.

3 participants