Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Thread safety question #217

Open
ofirreuv72 opened this issue Jan 1, 2019 · 3 comments
Open

Thread safety question #217

ofirreuv72 opened this issue Jan 1, 2019 · 3 comments

Comments

@ofirreuv72
Copy link

ofirreuv72 commented Jan 1, 2019

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

@ofirreuv72 ofirreuv72 changed the title Thread safety quetion Thread safety question Jan 1, 2019
@ilyapuchka
Copy link
Collaborator

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
Copy link
Author

ofirreuv72 commented Jan 2, 2019

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
Copy link
Collaborator

@ofirreuv72 is this still relevant?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants