Chapter 3 - Race condition for stopDownloads property

📚 Official Books Modern Concurrency in Swift
1

I’m working on Chapter 3 and Xcode notified me that there is a race condition occurring when setting or reading the SuperStorageModel.stopDownloads property. This happens when tapping the “Cancel All” button after adding model.stopDownloads = true in the button action.

This does make sense because, if I understand correctly, the property is set on the Main Actor but it’s read in the downloadWithProgress(fileName:name:size:offset:) private method which is async.

I’m trying to come up with a solution by myself but so far I wasn’t successful. Has anyone managed to fix the issue? Maybe @icanzilb already fixed it.

2 Likes
2

I have to double check this, haven’t seen a warning in that chapter at the time of the last edit. Maybe more recent Xcodes are more vigilant about accessing shared state (which was the plan). Adding this to my to-do list :+1:t3:

2 Likes
3

I am also seeing this.

Thread sanitizer actually complains about this in the debugger console

==================
WARNING: ThreadSanitizer: data race (pid=26664)
  Write of size 1 at 0x7b0800031eb9 by main thread (mutexes: write M1138):
    #0 SuperStorageModel.stopDownloads.setter <compiler-generated> (SuperStorage:x86_64+0x100040c8e)
    #1 closure #1 in closure #2 in DownloadView.body.getter DownloadView.swift:103 (SuperStorage:x86_64+0x10004f6bb)
    #2 partial apply for closure #1 in closure #2 in DownloadView.body.getter <compiler-generated> (SuperStorage:x86_64+0x10005217a)
    #3 partial apply for implicit closure #2 in implicit closure #1 in WrappedButtonStyle.makeBody(configuration:) <null>:2 (SwiftUI:x86_64+0x399840)
    #4 main SuperStorageApp.swift (SuperStorage:x86_64+0x1000452c5)

  Previous read of size 1 at 0x7b0800031eb9 by thread T16:
    [failed to restore the stack]

  Location is heap block of size 26 at 0x7b0800031ea0 allocated by main thread:
    #0 __sanitizer_mz_malloc <null>:2 (libclang_rt.tsan_iossim_dynamic.dylib:x86_64+0x5378c)
    #1 _malloc_zone_malloc <null>:2 (libsystem_malloc.dylib:x86_64+0x14d14)
    #2 closure #1 in SuperStorageApp.body.getter SuperStorageApp.swift:39 (SuperStorage:x86_64+0x100044e7b)
    #3 WindowGroup.init(content:) <null>:2 (SwiftUI:x86_64+0x7fe166)
    #4 protocol witness for App.body.getter in conformance SuperStorageApp <compiler-generated> (SuperStorage:x86_64+0x10004524d)
    #5 partial apply for closure #1 in AppBodyAccessor.updateBody(of:changed:) <null>:2 (SwiftUI:x86_64+0x230949)
    #6 main SuperStorageApp.swift (SuperStorage:x86_64+0x1000452c5)

  Mutex M1138 (0x7b4000004400) created at:
    #0 pthread_mutex_init <null>:2 (libclang_rt.tsan_iossim_dynamic.dylib:x86_64+0x2dc35)
    #1 _MovableLockCreate <null>:2 (SwiftUI:x86_64+0x4cf9)
    #2 dispatch_once_f <null>:2 (libclang_rt.tsan_iossim_dynamic.dylib:x86_64+0x78dfb)
    #3 static Update.begin() <null>:2 (SwiftUI:x86_64+0x48c126)
    #4 main SuperStorageApp.swift (SuperStorage:x86_64+0x1000452c5)

  Thread T16 (tid=15178393, running) is a GCD worker thread

SUMMARY: ThreadSanitizer: data race <compiler-generated> in SuperStorageModel.stopDownloads.setter
==================
ThreadSanitizer report breakpoint hit. Use 'thread info -s' to get extended information about the report.
[graphics-project-ver-1.jpeg] 0.3 MB
4

in Xcode 14 beta5 ，if enable Thread Sanitizer in Project’s scheme, the race condition will appear.

Fix it is very simple, delete the stopDownloads variable directly, and use Task.isCancelled to determine whether the task exits:

while !Task.isCancelled, !accumulator.checkCompleted() {
    while !accumulator.isBatchCompleted,
      let byte = try await asyncDownloadIterator.next() {
      try Task.checkCancellation()
      accumulator.append(byte)
    }
    let progress = accumulator.progress
    Task.detached(priority: .medium) {
      await self
        .updateDownload(name: name, progress: progress)
    }

    print(accumulator.description)
  }

  //if stopDownloads, !Self.supportsPartialDownloads {
  if Task.isCancelled, !Self.supportsPartialDownloads {
    throw CancellationError()
  }
1 Like
5

Do we have any quick fix as of now?

6

Yeah, I got it reproduced and I’d say the solution is to put stopDownloads on the main actor, e.g. like so:

@MainActor var stopDownloads = false

My issue is, of course, that at this point of the book actors aren’t really introduced so that doesn’t fit well in the book narrative. I’ll either add a paragraph to suggest to the readers how to do this on their own or add a note that this code isn’t exactly safe before introducing actors.

Thanks for noticing this btw, this is exactly the reason we’ve enabled Thread sanitizer on all the projects in the book

7

On a second thought, in Chapter 2 I already ask the reader to add some @MainActor annotations so maybe it’s not a worry to do that also in Chapter 3.