Skip to content

feat(mobile): cancel BlockOperation in iOS RemoteImageImpl#27522

Closed
LeLunZ wants to merge 1 commit intorefactor/ios-image-requestfrom
ios-cancel-remote-image-block-operation
Closed

feat(mobile): cancel BlockOperation in iOS RemoteImageImpl#27522
LeLunZ wants to merge 1 commit intorefactor/ios-image-requestfrom
ios-cancel-remote-image-block-operation

Conversation

@LeLunZ
Copy link
Copy Markdown
Collaborator

@LeLunZ LeLunZ commented Apr 5, 2026

Description

Previously we could have queued a BlockOperation, cancelled it, and it would have launched a new thread just to check isCancelled.
With the changes in this PR the BlockOperation does internally in its start method see that the request got cancelled and just aborts launching a new thread.
I made the BlockOperation a part of the RemoteImageRequest and also cancel it incase request.cancel is called.

The kind of annoying part: we need another lock because we add the BlockOperation in the request thread, and read it from the main thread to call cancel on it.

I think conceptually this is cleaner, but performance wise I didn't see any positive/negative impact when comparing request/cancellation timings.

I also thought about just creating an empty BlockOperation in the main thread when creating the request, but then we would have to call operation.addExecutionBlock later. I think the whole BlockOperation is thread safe so this shouldn't be a problem. But I am then not sure what kind of lock they are using internally and that could be anything (possibly worse in performance than the UnsafeLock).

(Also removed id property from RemoteRequest as this wasn't used anywhere)

How Has This Been Tested?

Checklist:

  • I have carefully read CONTRIBUTING.md
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation if applicable
  • I have no unrelated changes in the PR.
  • I have confirmed that any new dependencies are strictly necessary.
  • I have written tests for new code (if applicable)
  • I have followed naming conventions/patterns in the surrounding code
  • All code in src/services/ uses repositories implementations for database calls, filesystem operations, etc.
  • All code in src/repositories/ is pretty basic/simple and does not have any immich specific logic (that belongs in src/services/)

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

@LeLunZ LeLunZ force-pushed the ios-cancel-remote-image-block-operation branch from 8e6aecd to 913e8a0 Compare April 5, 2026 21:42
@LeLunZ
Copy link
Copy Markdown
Collaborator Author

LeLunZ commented Apr 9, 2026

Closing this for now as the base PR won't get merged as #27672 exists now

@LeLunZ LeLunZ closed this Apr 9, 2026
@LeLunZ LeLunZ deleted the ios-cancel-remote-image-block-operation branch April 9, 2026 23:00
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.

1 participant