Skip to content

fix(mobile): fix flutter cache eviction on thumbnails #27663

Merged
alextran1502 merged 6 commits intomainfrom
bugfix/cache-eviction-2
Apr 10, 2026
Merged

fix(mobile): fix flutter cache eviction on thumbnails #27663
alextran1502 merged 6 commits intomainfrom
bugfix/cache-eviction-2

Conversation

@LeLunZ
Copy link
Copy Markdown
Collaborator

@LeLunZ LeLunZ commented Apr 9, 2026

Description

In #27624 we fixed that requests get evicted from cache too late. When creating the PR I was too invested in checking that we don't do double evictions anywhere anymore and forgot that the FullImageProvider isn't actually everything and especially not what we use for thumbnails. The flag is missing from thumbhash providers and "normal" ImageProviders (which we use for thumbnails).
That currently results in the cache always being evicted for these images, after the corresponding widget is disposed, even if they are fully loaded.

Setting isFinished manually everywhere we need, is kind of a hassle and easily forgotten. (as noticed through this PR)
Thats why I made it part of the loadRequest methods, so it's part of the whole CancellableImageProviderMixin contract. And you actually have to opt out of setting the flag, which the "special" providers then have to do (eg. FullImageProvider).
Which means ThumbhashProvider and "normal" image provider don't have to set the flag.

@mertalev Sorry for the inconvenience.

Fixes #27679

How Has This Been Tested?

  • verified that the flag is now actually set to true (debugged this)

Checklist:

Please describe to which degree, if any, an LLM was used in creating this pull request.

/

Comment thread mobile/lib/presentation/widgets/images/image_provider.dart Outdated
Comment thread mobile/lib/presentation/widgets/images/image_provider.dart Outdated
Comment thread mobile/lib/presentation/widgets/images/image_provider.dart Outdated
Comment thread mobile/lib/presentation/widgets/images/image_provider.dart Outdated
Comment thread mobile/lib/presentation/widgets/images/image_provider.dart Outdated
Comment thread mobile/lib/presentation/widgets/images/image_provider.dart Outdated
Co-authored-by: Mert <101130780+mertalev@users.noreply.github.com>
@LeLunZ LeLunZ requested a review from danieldietzler as a code owner April 9, 2026 19:39
@immich-push-o-matic immich-push-o-matic Bot added documentation Improvements or additions to documentation 🧠machine-learning labels Apr 9, 2026
@LeLunZ
Copy link
Copy Markdown
Collaborator Author

LeLunZ commented Apr 9, 2026

bad rebase

@LeLunZ LeLunZ force-pushed the bugfix/cache-eviction-2 branch from 2411c65 to 8c6a1b6 Compare April 9, 2026 19:42
@LeLunZ LeLunZ removed documentation Improvements or additions to documentation 🧠machine-learning labels Apr 9, 2026
@LeLunZ LeLunZ removed the request for review from danieldietzler April 9, 2026 19:42
@LeLunZ
Copy link
Copy Markdown
Collaborator Author

LeLunZ commented Apr 9, 2026

actually @mertalev in the iOS code we have this:

 if let error = error {
      if request.isCancelled || (error as NSError).code == NSURLErrorCancelled {
        return request.completion(ImageProcessing.cancelledResult)
      }
      return request.completion(.failure(error))
    }

So we also return null incase the request errors with NSURLErrorCancelled, but I don't really know if that only happens if request.cancel is invoked, or if the system can also can requests

@LeLunZ
Copy link
Copy Markdown
Collaborator Author

LeLunZ commented Apr 9, 2026

Actually returning a cancellation incase the error code is NSURLErrorCancelled while isCancelled is false, doesn't seem right.
As this should still be an error, as we ourselves didn't invoke the cancellation

@mertalev
Copy link
Copy Markdown
Member

mertalev commented Apr 9, 2026

There is no case where the request is cancelled but we didn't request it. It's a slightly redundant check.

@LeLunZ
Copy link
Copy Markdown
Collaborator Author

LeLunZ commented Apr 9, 2026

Yeah I guess then this is finished?

Comment thread mobile/lib/presentation/widgets/images/image_provider.dart
Comment thread mobile/lib/presentation/widgets/images/image_provider.dart
@alextran1502 alextran1502 merged commit d39e7da into main Apr 10, 2026
54 checks passed
@alextran1502 alextran1502 deleted the bugfix/cache-eviction-2 branch April 10, 2026 15:28
eleboucher pushed a commit to eleboucher/homelab that referenced this pull request Apr 10, 2026
…4) (#139)

This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [immich-app/immich](https://github.com/immich-app/immich) | patch | `v2.7.3` → `v2.7.4` |

---

### Release Notes

<details>
<summary>immich-app/immich (immich-app/immich)</summary>

### [`v2.7.4`](https://github.com/immich-app/immich/releases/tag/v2.7.4)

[Compare Source](immich-app/immich@v2.7.3...v2.7.4)

### v2.7.4

This release addresses some issues with image rendering on the mobile app

#### What's Changed

##### 🐛 Bug fixes

- refactor(mobile): cleanup iOS image loading pipeline by [@&#8203;LeLunZ](https://github.com/LeLunZ) in [#&#8203;27672](immich-app/immich#27672)
- fix(server): hide original filename when not showing metadata by [@&#8203;meesfrensel](https://github.com/meesfrensel) in [#&#8203;27581](immich-app/immich#27581)
- fix(mobile): fix Flutter cache eviction on thumbnails    by [@&#8203;LeLunZ](https://github.com/LeLunZ) in [#&#8203;27663](immich-app/immich#27663)
- chore: pump cronet version by [@&#8203;shenlong-tanwen](https://github.com/shenlong-tanwen) in [#&#8203;27685](immich-app/immich#27685)

**Full Changelog**: <immich-app/immich@v2.7.3...v2.7.4>

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4xMDEuMSIsInVwZGF0ZWRJblZlciI6IjQzLjEwMS4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJyZW5vdmF0ZS9naXRodWItcmVsZWFzZSIsInR5cGUvcGF0Y2giXX0=-->

Reviewed-on: https://git.erwanleboucher.dev/eleboucher/homelab/pulls/139
Co-authored-by: bot-owl <bot@erwanleboucher.dev>
Co-committed-by: bot-owl <bot@erwanleboucher.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Mobile] Photo flickering when closing asset viewer after upgrading to v2.7.3

3 participants