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: Use SqlCipherDeletingErrorHandler in JobDatabase for corruption handling (#13762) #13798

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rishabh-bhatia
Copy link

@rishabh-bhatia rishabh-bhatia commented Nov 19, 2024

Previously, JobDatabase used SqlCipherErrorHandler, which did not delete corrupted signal-jobmanager.db. This fix ensures that SqlCipherDeletingErrorHandler is used, which deletes the database upon corruption.

Closes #13762

First time contributor checklist

Contributor checklist

  • Pixel 7a, Android 15
  • My contribution is fully baked and ready to be merged as is
  • I ensure that all the open issues my contribution fixes are mentioned in the commit message of my first commit using the Fixes #13762 syntax

Description

This pull request fixes issue #13762 by ensuring that the JobDatabase class uses SqlCipherDeletingErrorHandler instead of SqlCipherErrorHandler for database corruption handling. The previous implementation did not delete corrupted databases, leading to potential app crashes or unhandled exceptions.

Proposed Changes:

  • Updated the JobDatabase constructor to initialize with SqlCipherDeletingErrorHandler.
  • Added tests to verify that the SqlCipherDeletingErrorHandler is used and that database corruption triggers its onCorruption method, resulting in the deletion and recreation of the database.

Testing:

  • Verified the changes on the following devices:
    • Pixel 7a, Android 15
  • Added Unit tests to test the new change. There was no way to reliably corrupt the signal-jobmanager.db in way that would ensure that the SQLCipherErrorHandler's onCorruption method would be called. Since it was not possible to recreate the issue/error, best way to test it was Unit testing. Corrupting the data using hex editors doesn't call onCorruption for some reason, this could be something which might need further investigation. For now, this should resolve any issue where signal-jobmanager.db gets corrupted enough for our SQLiteOpenHelper to trigger a call to onCorruption method. By looking closely at the logs provided by the OP, seems like in their case onCorruption is being called so at least for users such as them, this fix should resolve the issue.

This fix ensures a better handling of corrupted databases and aligns with best practices for database error management.

…handling

Previously, JobDatabase used SqlCipherErrorHandler, which did not delete
corrupted `signal-jobmanager.db`. This fix ensures that SqlCipherDeletingErrorHandler
is used, which deletes the database upon corruption.

Closes signalapp#13762
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Signal crashes few seconds after Launch. Cause corrupted signal-jobmanager.db
1 participant