Data Race Detected - _isFinished

[AndrewBarba]

Xcode 9 (App Store), iOS 11.0.0, iPhone 8 Simulator

==================
WARNING: ThreadSanitizer: data race (pid=61782)
  Write of size 1 at 0x7b10005207d9 by main thread:
  * #0 _T04Nuke9Operation33_29604A210E0CE46F50487484973D2524LLC10isFinishedSbfs Scheduler.swift:100 (Nuke:x86_64+0x5ab72)
    #1 _T04Nuke9Operation33_29604A210E0CE46F50487484973D2524LLC6finishyyFyycfU_ Scheduler.swift:128 (Nuke:x86_64+0x5b8b4)
    #2 _T04Nuke9Operation33_29604A210E0CE46F50487484973D2524LLC6finishyyFyycfU_TA Scheduler.swift (Nuke:x86_64+0x60a10)
    #3 _T0Ix_IyB_TR Deduplicator.swift (Nuke:x86_64+0x16cfc)
    #4 __tsan::invoke_and_release_block(void*) <null> (libclang_rt.tsan_iossim_dynamic.dylib:x86_64+0x637fb)
    #5 _dispatch_client_callout <null> (libdispatch.dylib:x86_64+0x343b)
    #6 _T04Nuke9Operation33_29604A210E0CE46F50487484973D2524LLC6finishyyF Scheduler.swift:130 (Nuke:x86_64+0x5b79b)
    #7 _T04Nuke9Operation33_29604A210E0CE46F50487484973D2524LLC5startyyFyycfU_yycfU_yycfU_ Scheduler.swift:118 (Nuke:x86_64+0x5b60c)
    #8 _T04Nuke9Operation33_29604A210E0CE46F50487484973D2524LLC5startyyFyycfU_yycfU_yycfU_TA Scheduler.swift (Nuke:x86_64+0x6094d)
    #9 _T04Nuke9PreheaterC15startPreheating33_97FB4EA46ED899E20B248C6B0E32EBD1LLyAA7RequestV4with_tFyyyccfU_yAA6ResultOySo7UIImageCGcfU_ Preheater.swift:48 (Nuke:x86_64+0x3b4f6)
    #10 _T04Nuke9PreheaterC15startPreheating33_97FB4EA46ED899E20B248C6B0E32EBD1LLyAA7RequestV4with_tFyyyccfU_yAA6ResultOySo7UIImageCGcfU_TA Preheater.swift (Nuke:x86_64+0x3eff6)
    #11 _T04Nuke7ManagerC9loadImageyAA7RequestV4with_AA17CancellationTokenVSg5tokenyAA6ResultOySo7UIImageCGc10completiontFyycfU_yAPcfU_yycfU_ Manager.swift:97 (Nuke:x86_64+0x3172e)
    #12 _T04Nuke7ManagerC9loadImageyAA7RequestV4with_AA17CancellationTokenVSg5tokenyAA6ResultOySo7UIImageCGc10completiontFyycfU_yAPcfU_yycfU_TA Manager.swift (Nuke:x86_64+0x340eb)
    #13 _T0Ix_IyB_TR Deduplicator.swift (Nuke:x86_64+0x16cfc)
    #14 __tsan::invoke_and_release_block(void*) <null> (libclang_rt.tsan_iossim_dynamic.dylib:x86_64+0x637fb)
    #15 _dispatch_client_callout <null> (libdispatch.dylib:x86_64+0x343b)
    #16 start <null> (libdyld.dylib:x86_64+0xd80)

  Previous read of size 1 at 0x7b10005207d9 by thread T2:
  * #0 _T04Nuke9Operation33_29604A210E0CE46F50487484973D2524LLC10isFinishedSbfg Scheduler.swift:97 (Nuke:x86_64+0x5a993)
    #1 _T04Nuke9Operation33_29604A210E0CE46F50487484973D2524LLC10isFinishedSbfgTo Scheduler.swift (Nuke:x86_64+0x5a8e0)
    #2 __NSOQSchedule_f <null> (Foundation:x86_64+0x35d2b)
    #3 _dispatch_client_callout <null> (libdispatch.dylib:x86_64+0x343b)

  Issue is caused by frames marked with "*".

  Location is heap block of size 56 at 0x7b10005207c0 allocated by thread T4:
    #0 calloc <null> (libclang_rt.tsan_iossim_dynamic.dylib:x86_64+0x46be2)
    #1 class_createInstance <null> (libobjc.A.dylib:x86_64+0xf011)
    #2 _T04Nuke23OperationQueueSchedulerC7executeyAA17CancellationTokenVSg5token_yyycc7closuretF Scheduler.swift:75 (Nuke:x86_64+0x59e71)
    #3 _T04Nuke23OperationQueueSchedulerCAA05AsyncD0A2aDP7executeyAA17CancellationTokenVSg5token_yyycc7closuretFTW Scheduler.swift (Nuke:x86_64+0x5a456)
    #4 _T04Nuke9PreheaterC15startPreheating33_97FB4EA46ED899E20B248C6B0E32EBD1LLyAA7RequestV4with_tF Preheater.swift:51 (Nuke:x86_64+0x3a5c9)
    #5 _T04Nuke9PreheaterC15startPreheating33_97FB4EA46ED899E20B248C6B0E32EBD1LLyAA7RequestV4with_tFTA Preheater.swift (Nuke:x86_64+0x3ebca)
    #6 _T0s8SequencePsE7forEachyy7ElementQzKcKFTfq4gn_n <null> (libswiftCore.dylib:x86_64+0x222198)
    #7 _T04Nuke9PreheaterC15startPreheatingySayAA7RequestVG4with_tFyycfU_TA Preheater.swift (Nuke:x86_64+0x38e68)
    #8 _T0Ix_IyB_TR Deduplicator.swift (Nuke:x86_64+0x16cfc)
    #9 __tsan::invoke_and_release_block(void*) <null> (libclang_rt.tsan_iossim_dynamic.dylib:x86_64+0x637fb)
    #10 _dispatch_client_callout <null> (libdispatch.dylib:x86_64+0x343b)

  Thread T2 (tid=1674465, running) is a GCD worker thread

  Thread T4 (tid=1674467, running) is a GCD worker thread

SUMMARY: ThreadSanitizer: data race Scheduler.swift:100 in _T04Nuke9Operation33_29604A210E0CE46F50487484973D2524LLC10isFinishedSbfs
==================

[michaelnisi]

I suggest to catch this earlier by adding guards, for example:

  override final var isFinished: Bool {
    get { return _isFinished }
    set {
      guard newValue != _isFinished else {
        // Just to be extra annoying.
        fatalError("Nuke Operation: already finished")
      }
      willChangeValue(forKey: "isFinished")
      _isFinished = newValue
      didChangeValue(forKey: "isFinished")
    }
  }

Casually, we might just return from the guard, but an Operation starting or finishing more than once is a programming error, don‘t you think, @kean?

[kean]

It looks to me that ThreadSanitizer is reporting a different kind of error. It seems to be unhappy with the fact that reads and writes to isFinished happen from two different threads (Main thread, and T2 thread in the report).

I'm not yet sure whether it's a genuine problem or a false positive. So far new debug tools (e.g. Memory Debugger) has caused me hours of lost time due to false positives. It would be nice to get rid of those warnings anyway to not annoy people using Nuke with them (this warning is very easy to reproduce). I'm not sure how to go about fixing it yet.

[kean]

I'm going to try to get rid of those warnings. I would appreciate some help. It would be nice to get rid of this Operation type altogether. All it does right now it limits a maximum number of concurrent tasks. There must be a simpler way to do that.

[kean]

So I've added a custom TaskQueue which limits the number of concurrent tasks (that's all that is needed really). It took less code than NSOperation subclass, it's simpler, and it was much easier to get right than NSOperation subclass... Going to add some unit tests and include it in Nuke 6.