SDWebImage/Core/SDWebImageManager.m (2)

311-333: Good alignment of cacheOperation locking with cancel’s monitor

Wrapping the operation.cacheOperation assignments in @synchronized(operation) is a solid fix: it uses the same monitor object as @synchronized(self) in SDWebImageCombinedOperation’s -isCancelled and -cancel, which removes races between the cancel path and manager-side cache-operation mutations while keeping the potentially heavy queryImageForKey: calls outside the lock.

One optional improvement you might consider (for both normal and original cache paths) is to immediately cancel a newly-assigned cache operation if the combined operation was already cancelled before this assignment ran, to avoid doing unnecessary work after user cancellation:

-        @synchronized (operation) {
-            operation.cacheOperation = cacheOperation;
-        }
+        @synchronized (operation) {
+            operation.cacheOperation = cacheOperation;
+            if (operation.isCancelled) {
+                [cacheOperation cancel];
+                operation.cacheOperation = nil;
+            }
+        }

Same pattern would apply in callOriginalCacheProcessForOperation: after the other cacheOperation assignment.

Also applies to: 368-388, 783-807

---

442-477: loaderOperation synchronization is consistent; consider mirroring the post-cancel guard

The new @synchronized(operation) around operation.loaderOperation = loaderOperation; correctly brings the download sub-operation under the same monitor used by SDWebImageCombinedOperation’s -cancel, so cancel and assignment can’t race anymore.

For symmetry with the cache side and to avoid starting network work for already-cancelled combined operations, you could optionally add a post-assignment check here:

-        @synchronized (operation) {
-            operation.loaderOperation = loaderOperation;
-        }
+        @synchronized (operation) {
+            operation.loaderOperation = loaderOperation;
+            if (operation.isCancelled) {
+                [loaderOperation cancel];
+                operation.loaderOperation = nil;
+            }
+        }

This doesn’t affect correctness of the current fix, but tightens cancel semantics and resource usage.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro