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

Possible concurrency issue in StructureMap.Util.Cache, version 3.1.6 #512

Closed
erik-kallen opened this Issue Oct 13, 2016 · 5 comments

Comments

Projects
None yet
2 participants
@erik-kallen

erik-kallen commented Oct 13, 2016

Hi, I am investigating a very strange issue in our production environment, and this led me to check the StructureMap.Util.Cache class (https://github.com/structuremap/structuremap/blob/1075e6f49a8b9b15aa86e16bf4a15425421e2450/src/StructureMap/Util/Cache.cs)

Most of the methods will take the lock represented by _rwLock, but the indexer and the GetEnumerator() implementations will not (and if the indexer did take it, GetValue() takes the lock twice in succession so it is not atomic).

Is this an issue? Looks like it to me.

@jeremydmiller

This comment has been minimized.

Contributor

jeremydmiller commented Oct 13, 2016

That all went away if you move up to StructureMap 4.*.

On Oct 13, 2016, at 4:58 AM, Erik Källén notifications@github.com wrote:

Hi, I am investigating a very strange issue in our production environment, and this led me to check the StructureMap.Util.Cache class (https://github.com/structuremap/structuremap/blob/1075e6f49a8b9b15aa86e16bf4a15425421e2450/src/StructureMap/Util/Cache.cs https://github.com/structuremap/structuremap/blob/1075e6f49a8b9b15aa86e16bf4a15425421e2450/src/StructureMap/Util/Cache.cs)

Most of the methods will take the lock represented by _rwLock, but the indexer and the GetEnumerator() implementations will not (and if the indexer did take it, GetValue() takes the lock twice in succession so it is not atomic).

Is this an issue? Looks like it to me.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub #512, or mute the thread https://github.com/notifications/unsubscribe-auth/AAK8C4DpnGI7vYiFZi2bKz8DpAKU5a4oks5qzgDEgaJpZM4KVs1H.

@erik-kallen

This comment has been minimized.

erik-kallen commented Oct 13, 2016

Unfortunately we are stuck with version 3 due to dependencies

@jeremydmiller

This comment has been minimized.

Contributor

jeremydmiller commented Oct 13, 2016

If you want, I can take a pull request to the 3.1 branch and push another update:https://github.com/structuremap/structuremap/commits/3.1

In 4, the Cache class was completely replaced with ConcurrentDictionary. You could repeat that change.

@erik-kallen

This comment has been minimized.

erik-kallen commented Oct 13, 2016

So you think it is possible to backport the Cache to ConcurrentDictionary change to 3.1? If so, I can look into that tomorrow

@jeremydmiller

This comment has been minimized.

Contributor

jeremydmiller commented Nov 1, 2016

This was done.

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