search

AliSoftware / Dip

Simple Swift Dependency container. Use protocols to resolve your dependencies and avoid singletons / sharedInstances!
MIT License
978 stars 75 forks source link

Thread safety question #217

Open ofirreuv72 opened 5 years ago

ofirreuv72 commented 5 years ago

Hi,

enable thread sanitiser in Xcode.

I used the sample app provided and added this code. in application didFinishLaunchingWithOptions after the configure function. 

            DispatchQueue.global(qos: .userInteractive).async {
                _ = try! self.container.resolve() as PersonProviderAPI
            }

i am getting the error below 

(lldb) thread info -s
thread #1: tid = 0xb5feba, 0x000000010846d210 libclang_rt.tsan_iossim_dynamic.dylib`__tsan_on_report, queue = 'com.apple.main-thread', stop reason = Data race detected

{
  "all_addresses_are_same" : true,
  "description" : "Data race",
  "instrumentation_class" : "ThreadSanitizer",
  "is_swift_access_race" : false,
  "issue_type" : "data-race",
  "location_description" : "Location is a 144-byte heap object at 0x7b24000196b0",
  "locs" : [
    {
      "address" : 0,
      "file_descriptor" : 0,
      "index" : 0,
      "object_type" : "",
      "size" : 144,
      "start" : 135394549143216,
      "suppressable" : 0,
      "thread_id" : 1,
      "trace" : [
        4433745643,
        4518380009,
        4432193247,
        4432193580,
        4480071751,
        4547030401
      ],
      "type" : "heap"
    }
  ],
  "memory_address" : 135394549143240,
  "mops" : [
    {
      "address" : 135394549143240,
      "index" : 0,
      "is_atomic" : false,
      "is_write" : true,
      "size" : 8,
      "thread_id" : 1,
      "trace" : [
        4455414645,
        4455417239,
        4455399549,
        4455413244,
        4455496011,
        4455490104,
        4455489180,
        4455488196,
        4432186403,
        4432192140,
        4480034918,
        4547030401
      ]
    },
    {
      "address" : 135394549143240,
      "index" : 1,
      "is_atomic" : false,
      "is_write" : false,
      "size" : 8,
      "thread_id" : 3,
      "trace" : [
        4455495051,
        4455490104,
        4455489180,
        4455488196,
        4432190235,
        4432190546,
        4432190881,
        4433864684,
        4546520124
      ]
    }
  ],
  "mutexes" : [
    {
      "address" : 135360189314752,
      "destroyed" : 0,
      "index" : 0,
      "mutex_id" : 1138,
      "trace" : [
        4433615524,
        4456469052,
        4455387904,
        4455395834,
        4455395015,
        4432193247,
        4432193580,
        4480071751,
        4547030401
      ]
    }
  ],
  "report_count" : 0,
  "sleep_trace" : [

  ],
  "stacks" : [

  ],
  "stop_description" : "Data race detected",
  "summary" : "Data race in closure #1 () throws -> A in Dip.DependencyContainer.inContext<A>(key: Dip.DefinitionKey, injectedInType: Swift.Optional<Any.Type>, injectedInProperty: Swift.Optional<Swift.String>, inCollaboration: Swift.Bool, container: Swift.Optional<Dip.DependencyContainer>, logErrors: Swift.Optional<Swift.Bool>, block: () throws -> A) throws -> A at 0x7b24000196b0",
  "tag" : 0,
  "threads" : [
    {
      "index" : 0,
      "name" : "",
      "parent_thread_id" : 1,
      "running" : 1,
      "thread_id" : 1,
      "thread_os_id" : 11927226,
      "trace" : [

      ]
    },
    {
      "index" : 1,
      "name" : "",
      "parent_thread_id" : 0,
      "running" : 1,
      "thread_id" : 3,
      "thread_os_id" : 11927348,
      "trace" : [

      ]
    }
  ],
  "unique_tids" : [

  ]
}

file:///Users/or84489/ws/Dip/Sources/Dip.swift: runtime: Threading Issues: Data race in closure #1 () throws -> A in Dip.DependencyContainer.inContext<A>(key: Dip.DefinitionKey, injectedInType: Swift.Optional<Any.Type>, injectedInProperty: Swift.Optional<Swift.String>, inCollaboration: Swift.Bool, container: Swift.Optional<Dip.DependencyContainer>, logErrors: Swift.Optional<Swift.Bool>, block: () throws -> A) throws -> A at 0x7b24000196b0

notice: Threading Issues: Location is a 144-byte heap object at 0x7b24000196b0

Write of size 8 by thread 1
#0  0x0000000109903b75 in closure #1 in DependencyContainer.inContext<A>(key:injectedInType:injectedInProperty:inCollaboration:container:logErrors:block:) at /Users/or84489/ws/Dip/Sources/Dip.swift:233
#1  0x0000000109904597 in partial apply for closure #1 in DependencyContainer.inContext<A>(key:injectedInType:injectedInProperty:inCollaboration:container:logErrors:block:) ()
#2  0x000000010990007d in DependencyContainer.threadSafe<A>(_:) at /Users/or84489/ws/Dip/Sources/Dip.swift:109
#3  0x00000001099035fc in DependencyContainer.inContext<A>(key:injectedInType:injectedInProperty:inCollaboration:container:logErrors:block:) at /Users/or84489/ws/Dip/Sources/Dip.swift:208
#4  0x000000010991794b in DependencyContainer._resolve(type:tag:builder:) at /Users/or84489/ws/Dip/Sources/Resolve.swift:141
#5  0x0000000109916238 in DependencyContainer.resolve(_:tag:builder:) at /Users/or84489/ws/Dip/Sources/Resolve.swift:123
#6  0x0000000109915e9c in DependencyContainer._resolve<A>(tag:builder:) at /Users/or84489/ws/Dip/Sources/Resolve.swift:131
#7  0x0000000109915ac4 in DependencyContainer.resolve<A>(tag:) at /Users/or84489/ws/Dip/Sources/Resolve.swift:50
#8  0x00000001082dcc23 in AppDelegate.application(_:didFinishLaunchingWithOptions:) at /Users/or84489/ws/Dip/SampleApp/DipSampleApp/AppDelegate.swift:31
#9  0x00000001082de28c in @objc AppDelegate.application(_:didFinishLaunchingWithOptions:) ()
#10 0x000000010b07e866 in -[UIApplication _handleDelegateCallbacksWithOptions:isSuspended:restoreState:] ()
#11 0x000000010f062d81 in start ()
Read of size 8 by thread 3
#0  0x000000010991758b in DependencyContainer._resolve(type:tag:builder:) at /Users/or84489/ws/Dip/Sources/Resolve.swift:141
#1  0x0000000109916238 in DependencyContainer.resolve(_:tag:builder:) at /Users/or84489/ws/Dip/Sources/Resolve.swift:123
#2  0x0000000109915e9c in DependencyContainer._resolve<A>(tag:builder:) at /Users/or84489/ws/Dip/Sources/Resolve.swift:131
#3  0x0000000109915ac4 in DependencyContainer.resolve<A>(tag:) at /Users/or84489/ws/Dip/Sources/Resolve.swift:50
#4  0x00000001082ddb1b in closure #1 in AppDelegate.application(_:didFinishLaunchingWithOptions:) at /Users/or84489/ws/Dip/SampleApp/DipSampleApp/AppDelegate.swift:28
#5  0x00000001082ddc52 in partial apply for closure #1 in AppDelegate.application(_:didFinishLaunchingWithOptions:) ()
#6  0x00000001082ddda1 in thunk for @escaping @callee_guaranteed () -> () ()
#7  0x00000001084767ec in __tsan::invoke_and_release_block(void*) ()
#8  0x000000010efe643c in _dispatch_client_callout ()
Heap block allocated by thread 1
#0  0x00000001084596eb in wrap_malloc ()
#1  0x000000010d5101e9 in swift_slowAlloc ()
#2  0x00000001082de6df in AppDelegate.init() at /Users/or84489/ws/Dip/SampleApp/DipSampleApp/AppDelegate.swift:17
#3  0x00000001082de82c in @objc AppDelegate.init() ()
#4  0x000000010b087847 in _UIApplicationMainPreparations ()
#5  0x000000010f062d81 in start ()
Mutex M1138 created
#0  0x0000000108439aa4 in wrap_pthread_mutex_init ()
#1  0x0000000109a0523c in -[NSRecursiveLock init] ()
#2  0x00000001098fd300 in NSRecursiveLock.__allocating_init() ()
#3  0x00000001098ff1fa in DependencyContainer.init(autoInjectProperties:configBlock:) at /Users/or84489/ws/Dip/Sources/Dip.swift:46
#4  0x00000001098feec7 in DependencyContainer.__allocating_init(autoInjectProperties:configBlock:) ()
#5  0x00000001082de6df in AppDelegate.init() at /Users/or84489/ws/Dip/SampleApp/DipSampleApp/AppDelegate.swift:17
#6  0x00000001082de82c in @objc AppDelegate.init() ()
#7  0x000000010b087847 in _UIApplicationMainPreparations ()
#8  0x000000010f062d81 in start ()

please advise

ilyapuchka commented 5 years ago

I've never used Thread Sanitizer before so can't say why it gives such a warning. I'd suggest playing with a minimum code snippet that would use a recursive lock and recursive function calls if you are curious. It might be just a sanitizer not playing well with recursive locks.

ofirreuv72 commented 5 years ago

i do think there is a race condition here.

the inContext function is protected by the lock, but also setting the context object. 

      context = Context(
        key: key,
        injectedInType: injectedInType,
        injectedInProperty: injectedInProperty,
        inCollaboration: inCollaboration
      )

the line at Resolve.swift ~ 141 

    return try inContext(key:key, injectedInType: context?.resolvingType) {

is not protected by a lock and it is access the context object which is not protected.

i think we need to protect the access to context?.resolvingType

making the change below fix this issue, but i am not sure if this is the correct fix. 

| =>git diff
diff --git a/Sources/Resolve.swift b/Sources/Resolve.swift
index 4cf0791..b84e8ca 100644
--- a/Sources/Resolve.swift
+++ b/Sources/Resolve.swift
@@ -46,7 +46,9 @@ extension DependencyContainer {
    - seealso: `register(_:type:tag:factory:)`
    */
   public func resolve<T>(tag: DependencyTagConvertible? = nil) throws -> T {
-    return try _resolve(tag: tag) { (factory: () throws -> T) in try factory() }
+    return try threadSafe {
+      return try _resolve(tag: tag) { (factory: () throws -> T) in try factory() }
+    }
   }

   /**
ilyapuchka commented 4 years ago

@ofirreuv72 is this still relevant?