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

Make the dependency injection thread safe #24

Closed
wants to merge 2 commits into from

Conversation

Greybird
Copy link
Contributor

@Greybird Greybird commented Oct 6, 2019

Hi,

I just wanted to share this code, which I developed for our company to solve issue SpecFlowOSS/SpecFlow#657 from Specflow, using NUnit runner along with an Unity 3 plugin.
This change to make the dependency injection thread-safe allowed us to parallellize at the feature level without any issue, while it was constantly failing before the fix.

Of course, the fix is pretty crude, as its aim was to have no public surface change, but I though it was better to share it if it could be of use.

@gasparnagy
Copy link
Collaborator

@Greybird Thx. Unfortunately the diff cannot show the differences properly here, so it is hard to get an overview. Could you please summarize the change in a few sentences?

@Greybird
Copy link
Contributor Author

Greybird commented Oct 7, 2019

Yes sure!

Basically

  • features from the ObjectContainer are moved to the ObjectContainerUnsafe, Unsafe meaning not thread-safe
  • a new ObjectContainer class wraps an instance of the ObjectContainerUnsafe and delegates calls to it, after locking.

This design was chosen because it allowed:

  • in-place replacement for SpecFlow usage using a binding-redirect
  • locking in the interface implementation methods, without having to consider actual inner workings of the previously-named ObjectContainer.

Of course, the key is locking, and a simpler design could be used provided depending libraries code can be modified.

The goal of this PR is mainly to indicate that the SpecFlow issue reported is actually solvable through proper locking on the DI.

Hope this helps!

Arnaud

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

Successfully merging this pull request may close these issues.

None yet

2 participants