ThreadSanitizer reports data race

[niklassaers]

Hey,
on branch "feature/swift-3", NetworkFetcher.swift line 64, is
if cancelled { return }

I'm getting errors from the thread sanitiser:

• Read of size 1 at 0x7d10000eef38 by thread T30
• Previous write of size 1 at 0x7d10000eef38 by main thread in cancelFetch on line 57
• Location is heap block of size 57 at 0x7d10000eef00 allocated by main thread

Any suggestions how to best make this Bool cancelled thread safe?

Cheers

Nik

[niklassaers]

Suggestion:

@@ -38,12 +38,16 @@ open class NetworkFetcher<T : DataConvertible> : Fetcher<T> {

     var task : URLSessionDataTask? = nil

+    var cancelQueue = DispatchQueue(label: "com.hanekeswift.networkfetcher.cancel")
     var cancelled = false

     // MARK: Fetcher

     open override func fetch(failure fail: @escaping ((Error?) -> ()), success succeed: @escaping (T.Result) -> ()) {
-        self.cancelled = false
+        cancelQueue.sync(flags: .barrier) {
+            self.cancelled = false
+        }
+
         self.task = self.session.dataTask(with: self.URL) {[weak self] (data, response, error) -> Void in
             if let strongSelf = self {
                 strongSelf.onReceive(data: data, response: response, error: error, failure: fail, success: succeed)
@@ -54,7 +58,9 @@ open class NetworkFetcher<T : DataConvertible> : Fetcher<T> {

     open override func cancelFetch() {
         self.task?.cancel()
-        self.cancelled = true
+        cancelQueue.sync(flags: .barrier) {
+            self.cancelled = true
+        }
     }


@@ -62,8 +68,14 @@ open class NetworkFetcher<T : DataConvertible> : Fetcher<T> {

     fileprivate func onReceive(data: Data!, response: URLResponse!, error: Error!, failure fail: @escaping ((Error?) -> ()), success succeed: @escaping (T.Result) -> ()) {

-        if cancelled { return }
-        
+        var shouldReturn = false
+        cancelQueue.sync(flags: .barrier) {
+            shouldReturn = cancelled
+        }
+        if shouldReturn == true {
+            return
+        }
+
         let URL = self.URL

         if let error = error {

I'm not sure how efficient this is, nor if it should use its own queue. The shuoldReturn dance also isn't all that easy to read, and it seems a bit of a roundabout way of solving the issue. It does, though, seem to solve it.

Cheers

Nik

[fatihyildizhan]

Thank you @niklassaers . Your way fixed the problem.

• I didn't catch the crash first then I can handled after enable the Thread Sanitizer.