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

Synchronize read/write accesses to NamingScope.GetUniqueName #383

Merged
1 commit merged into from
Jun 18, 2018

Conversation

stakx
Copy link
Member

@stakx stakx commented Jun 18, 2018

NamingScope.GetUniqueName maintains and updates counters in an unsynchronized Dictionary<,>. This writing method is currently called from a location that allows several concurrent readers.

This changes BaseProxyGenerator.ObtainProxyType so that it goes from a read lock straight to a write lock. There seems little point in taking an upgradeable lock first if we're going to need a write lock anyway for all of the work. (Taking an upgradeable read lock first would only benefit the second, repeated cache check.)

`NamingScope.GetUniqueName` maintains and updates counters in an un-
synchronized `Dictionary<,>`. This writing method is currently called
from a location that allows several concurrent readers.

This changes `BaseProxyGenerator.ObtainProxyType` so that it goes from
a read lock straight to a write lock. There seems little point in
taking an upgradeable lock first if we're going to need a write lock
anyway for all of the work.
@@ -396,9 +396,9 @@ protected Type ObtainProxyType(CacheKey cacheKey, Func<string, INamingScope, Typ
}

// This is to avoid generating duplicate types under heavy multithreaded load.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit, you could probably move the comment 'This is to avoid generating duplicate types under heavy multithreaded load' to just after 'See if an earlier lock holder populated the cache.'

@TimLovellSmith
Copy link
Contributor

@stakx Looks beautiful to me!

@ghost ghost merged commit 886e764 into castleproject:master Jun 18, 2018
@stakx stakx deleted the thread-synchronization branch June 18, 2018 23:30
@jonorossi jonorossi added this to the vNext milestone Jun 20, 2018
stakx added a commit to stakx/Castle.Core that referenced this pull request Jun 20, 2018
@stakx stakx mentioned this pull request Jun 20, 2018
jonorossi added a commit that referenced this pull request Jun 20, 2018
This pull request was closed.
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.

3 participants