SDWebImage/Core/SDWebImageDownloaderOperation.m (1)

235-255: Consider rechecking isCancelled before assigning self.dataTask / executing and resuming

There is still a small window between the first and second @synchronized(self) blocks where cancel can run:

• start has already created dataTask and left the first @synchronized(self).
• Another thread calls cancel, which (via cancelInternal and reset) can:
  • Fire completion blocks and clear callbackTokens.
  • Invalidate the session.
  • Leave the operation in a cancelled state (isCancelled == YES) but with isExecuting == NO and isFinished == NO.

After that, the second @synchronized(self) in start will still execute, set self.dataTask = dataTask; self.executing = YES;, and [self.dataTask resume];, effectively “reviving” an operation that was just cancelled and had its callbacks fired.

This is a pre‑existing race, not introduced by this PR, but it becomes more visible now that dataTask handling is tightened. To align cancel semantics with expectations (cancel-before-or-during-start should prevent the request from ever starting), you could recheck cancellation inside the second synchronized block before assigning state:

-    if (dataTask) {
+    if (dataTask) {
         if (self.options & SDWebImageDownloaderHighPriority) {
             dataTask.priority = NSURLSessionTaskPriorityHigh;
         } else if (self.options & SDWebImageDownloaderLowPriority) {
             dataTask.priority = NSURLSessionTaskPriorityLow;
         } else {
             dataTask.priority = NSURLSessionTaskPriorityDefault;
         }
-        NSArray<SDWebImageDownloaderOperationToken *> *tokens;
-        @synchronized (self) {
-            tokens = [self.callbackTokens copy];
-            self.dataTask = dataTask;
-            self.executing = YES;
-        }
+        BOOL shouldStart = NO;
+        NSArray<SDWebImageDownloaderOperationToken *> *tokens = nil;
+        @synchronized (self) {
+            if (!self.isCancelled && !self.isFinished) {
+                tokens = [self.callbackTokens copy];
+                self.dataTask = dataTask;
+                self.executing = YES;
+                shouldStart = YES;
+            }
+        }
+        if (!shouldStart) {
+            // Operation was cancelled/finished after task creation; do not resume.
+            return;
+        }
         for (SDWebImageDownloaderOperationToken *token in tokens) {
             ...
         }
         // Put the URLSession task resume at the end of start method to avoid race condition
         [self.dataTask resume];
         ...

This keeps the “resume at end” behavior, but guarantees you never mark the operation executing or resume a task after a concurrent cancel/reset has already run.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro