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

Assembly/Assembler need thread safety #221

Open
kdubb opened this issue Feb 12, 2017 · 7 comments · May be fixed by #445
Open

Assembly/Assembler need thread safety #221

kdubb opened this issue Feb 12, 2017 · 7 comments · May be fixed by #445

Comments

@kdubb
Copy link

kdubb commented Feb 12, 2017

Unless I'm missing something, there is currently no way to call synchronized from an Assembly and the resolver property doesn't apply it. So, using Assembly seems to always be unsafe.

For the moment I've copied SynchronizedResolver into our project (the version provided by Swinject is not public) and am wrapping the Assembly.resolver with it.

@kdubb
Copy link
Author

kdubb commented Feb 12, 2017

I should say I am using a heavily modified SynchronizedResolver that uses a serial DispatchQueue as a lock and only requires a Resolver.

@yoichitgy
Copy link
Member

I agree👍 As we talked in #139, I also want to enhance concurrency model. I'll try to implement concurrency for Assembly/Assembler. If you can show me your modified code, it's very helpful!

@geraldeersteling
Copy link

How is this coming along? I'm also worried about thread safety.

Right now I'm creating my Assembler with the SwinjectStoryboard's defaultContainer. I'm thinking of exposing the defaultContainer's threadsafe view (synchronize) instead of the Assembler's resolver. Thoughts?

@bobspryn
Copy link

So what is the mechanism for exposing a thread safe resolver from the Assembler?

@yonivav
Copy link

yonivav commented Apr 3, 2019

@sprynmr let synchronizedResolver = (assembler.resolver as! Container).synchronized()
from (https://stackoverflow.com/questions/49687342/crash-container-swift-line-299-container-resolvea-b-entry-serviceentrypro)

@devxoul devxoul linked a pull request Feb 25, 2020 that will close this issue
@devxoul
Copy link

devxoul commented Feb 25, 2020

I've created a patch: #445. I would really appreciate if somebody could review this! Thanks :)

@bryanwagner
Copy link

The PR submitted above #445 appears to fix the issue. As a workaround, I'm using the following extension function until the PR is merged:

extension Assembler {
    public var synchronizedResolver: Resolver {
        let internalResolver = resolver
        if let it = internalResolver as? Container {
            return it.synchronize()
        } else {
            return internalResolver
        }
    }
}

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

Successfully merging a pull request may close this issue.

7 participants