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

Fix #13 KeyNotFoundException by using ConcurrentDictionary.GetOrAdd instead of indexer #26

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@FrankBakkerNl
Copy link

FrankBakkerNl commented Oct 13, 2017

No description provided.

@FrankBakkerNl FrankBakkerNl force-pushed the FrankBakkerNl:master branch from d2a9213 to 36b943b Oct 13, 2017

@Daniel15

This comment has been minimized.

Copy link
Owner

Daniel15 commented Oct 16, 2017

Can you please elaborate on how to reproduce this issue? CreateEngine sets the _metadata before the engine is added to _availableEngines, so this should not be necessary.

@FrankBakkerNl

This comment has been minimized.

Copy link

FrankBakkerNl commented Oct 16, 2017

To be honest I cannot explain exactly why this is a problem, I agree it all looks safe. However under heavy load we are getting loads of KeyNotFoundExceptions from production on this line right now. We are using version 0.4.1 at this moment but it seems like this specific code has not changed since then. We do want to upgrade, but reather be sure the problem is fixed. In issue #13 there is a comment saying 'most probably' fixed. So this is to be sure

@Daniel15

This comment has been minimized.

Copy link
Owner

Daniel15 commented Oct 16, 2017

It would be good to figure out exactly what is causing this... I suspect it's a race condition somewhere.

@FrankBakkerNl FrankBakkerNl reopened this Oct 16, 2017

@benokit

This comment has been minimized.

Copy link
Contributor

benokit commented Dec 17, 2018

This can be closed since it was resolved within #36.

@Daniel15

This comment has been minimized.

Copy link
Owner

Daniel15 commented Dec 21, 2018

I agree. Thanks @benokit!

@Daniel15 Daniel15 closed this Dec 21, 2018

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