Skip to content

fix: check cached file existence asynchronously before removing it - #540

Open
yamashita-room-335 wants to merge 1 commit into
Baseflow:mainfrom
yamashita-room-335:fix/remove-cached-file-async-exists
Open

yamashita-room-335 wants to merge 1 commit into
Baseflow:mainfrom
yamashita-room-335:fix/remove-cached-file-async-exists

Conversation

@yamashita-room-335

Copy link
Copy Markdown

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

Bug fix.

⤵️ What is the current behavior?

CacheStore._removeCachedFile calls the synchronous existsSync() before deleting a cached file. Cache cleanup, removeFile and emptyCache all go through it. On Android with Flutter 3.29 or later this blocks the platform thread and can cause ANRs.

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

The existence check uses await file.exists(). Because the check is now asynchronous, two concurrent removals of the same entry can both see the file and race on delete(). A failed delete is ignored when the file no longer exists and rethrown otherwise. Before, only PathNotFoundException was ignored, which MemoryFileSystem does not throw. The version is bumped to 3.4.6.

💥 Does this PR introduce a breaking change?

No.

🐛 Recommendations for testing

dart format ., flutter analyze and flutter test pass locally on Flutter 3.47.7. The new test in cache_store_test.dart uses a File whose existsSync() throws and fails without the fix. The existing test "Store should return null if file is not cached" covers the concurrent removal.

📝 Links to relevant issues/docs

Fixes #539

🤔 Checklist before submitting

  • All projects build (the example app was not built)
  • Follows style guide lines (code style guide)
  • Relevant documentation was updated
  • Rebased onto current main
  • Version bumped and dated CHANGELOG entry added

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.

existsSync in CacheStore._removeCachedFile blocks the main isolate (ANR on Android)

1 participant