Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add database integrity check #407

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

Conversation

turekj
Copy link

@turekj turekj commented Apr 14, 2023

✨ What kind of change does this PR introduce? (Bug fix, feature, docs update...)

This change is in between a bug fix and a new feature.

⤵️ What is the current behavior?

We're currently using flutter_cache_manager through cached_network_image. We observe the following scenario in the crash logs:

SqfliteDatabaseException: DatabaseException(Error Domain=FMDatabase Code=11 "database disk image is malformed" UserInfo={NSLocalizedDescription=database disk image is malformed}) sql 'INSERT INTO cacheObject (url, key, relativePath, eTag, validTill, touched, length) VALUES (?, ?, ?, ?, ?, ?, ?)' args [https://cdn.everskies.com/media/badge/TMiBI9ss1X_c..., https://cdn.everskies.com/media/badge/TMiBI9ss1X_c..., cab86ba0-daab-11ed-ac21-0b56a114789e.png, "791d34fba03a4949d937107eedde8d96", 1682849089370, 1681466689377, 468]
  File "exception_impl.dart", line 11, in wrapDatabaseException
  File "<asynchronous suspension>"
  File "database_mixin.dart", line 392, in SqfliteDatabaseMixin.txnRawInsert.<fn>
  File "<asynchronous suspension>"
  File "basic_lock.dart", line 33, in BasicLock.synchronized
  File "<asynchronous suspension>"
  File "database_mixin.dart", line 344, in SqfliteDatabaseMixin.txnSynchronized
  File "<asynchronous suspension>"
  File "cache_object_provider.dart", line 101, in CacheObjectProvider.insert
  File "<asynchronous suspension>"
  File "cache_store.dart", line 59, in CacheStore.putFile

We can't tell how the DB got corrupted, but we know it happens mostly on iOS devices. This makes sense as according to sqflite docs:

Android and iOS handles corruption in a different way:

  • on iOS, it fails on first access to the database
  • on Android, the existing file is removed.

Why would fixing a DB corruption make sense in the library? The problem is that once the DB gets corrupted, there cache is not functional for the user. All update/insert operations fail and there's no indication that something is wrong when open() is called on the CacheObjectProvider. There's no easy way to recover from this scenario, especially in use cases such as cached_network_image.

🆕 What is the new behavior (if this is a feature change)?

When the db is opened, the additional pragma integrity_check (reference) query is run. If the db is corrupted, it drops the file before the db connection is opened, thus allowing the cache to "recover" from the failed state.

The obvious drawback of this approach is that the binary cache contents are not wiped. I will happily accept pointers on how to resolve this and other potential issues.

💥 Does this PR introduce a breaking change?

No.

🐛 Recommendations for testing

The way I tested this on iOS simulator (on macOS):

  1. Run the example app and cache some files.
  2. Print out the database path and open the DB in Xcode.
  3. Pick Navigate > Reveal in Project Navigator from the top bar.
  4. Right-click the file and pick Open As > Hex File.
  5. Scroll down till you can see relative paths of the cached files.
  6. Select and delete the hex values from the middle of one row to the middle of another row.
  7. Save the db file (it's now corrupted).
  8. Re-run the app.

At point 8. the library with no patch applied will stop caching any files.

📝 Links to relevant issues/docs

  • sqflite on handling corruption - docs
  • pragma integrity_check from SQLite - docs
  • Sample stacktrace we're observing:

🤔 Checklist before submitting

  • All projects build
  • Follows style guide lines (code style guide)
  • Relevant documentation was updated
  • Rebased onto current develop

contactjavas added a commit to tentram/flutter_cache_manager that referenced this pull request Sep 18, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants