Swinject: Simplify thread safety model

[yoichitgy]

Currently a thread-safe container is created by synchronize method wrapping the receiver container in SynchronizedResolver. It has turned out that this model is difficult to maintain to keep consistency of ResolverType with Container and SynchronizedResolver.

It looks good to pass a dispatch queue to Container initializers to use in resolve methods with dispatch_sync. If no dispatch queue is passed to the initializers, the container is not thread safe (but its performance is better).

Another motivation for the change on the thread safety model is OSSpinLock was previously used for the best performance, but it has been replaced with NSLock for its problem (#116). Using a dispatch queue is better in performance than NSLock, and looks good for usability of Swinject.

[yoichitgy]

I wrote like above, but maybe we might need something like a recursive lock??? though I prefer dispatch queue. (Also Apple does not recommend recursive lock in RECURSIVE LOCKS section in man page of dispatch_async.)

[kdubb]

@yoichitgy It seems like concurrency might better be handled at the ObjectScope level rather than the Container level. After all, it's the scope the determines if it needs thread-safety.

It seems to me...
container needs a simple lock to ensure it's only created once (e.g. like dispatch_once).
transient shouldn't need any specific locks
weak only needs an atomic test (handled by swift)
graph needs a recursive lock

I'm not quite sure how this is all implemented so maybe it isn't as easy as I've laid out. From an outside perspective the locking seems like it should act in this way though. Am I missing something?

[yoichitgy]

@kdubb thank you for your great idea😃 I was thinking passing a DispatchQueue to a Container might be good for concurrency, but handling at an ObjectScope level looks nice because @jakubvano improved extendablity of ObjectScope.

One thing we should care about is complexity in SwinjectStoryboard, SwinjectPropertyLoader and SwinjectAutoregistration. So far, for them, it looks better to handle concurrency in ObjectScope.

I'll investigate ObjectScope implementation for concurrency. If you got a good idea for implementation, please write here. Always welcome 😃

[jakubvano]

Handling concurrency at ObjectScope level is interesting idea, but I don't think it is the right place - in my mind ObjectScope is responsible for instance persistency, while Container is responsible for instance access / creation. Also, implementation would be complicated, as currently InstanceStorage has separate methods for storing and retrieving an instance, and ensuring thread safety requires synchronisation of both operations.

[jakubvano]

Closing due to inactivity