{"action":"created","issue":{"url":"https://api.github.com/repos/dotnet/coreclr/issues/63","labels_url":"https://api.github.com/repos/dotnet/coreclr/issues/63/labels{/name}","comments_url":"https://api.github.com/repos/dotnet/coreclr/issues/63/comments","events_url":"https://api.github.com/repos/dotnet/coreclr/issues/63/events","html_url":"https://github.com/dotnet/coreclr/pull/63","id":56540881,"number":63,"title":"Call TryGetValue in overload of ConcurrentDictionary.GetOrAdd to avoid acquiring lock","user":{"login":"russgray","id":57107,"avatar_url":"https://avatars.githubusercontent.com/u/57107?v=3","gravatar_id":"","url":"https://api.github.com/users/russgray","html_url":"https://github.com/russgray","followers_url":"https://api.github.com/users/russgray/followers","following_url":"https://api.github.com/users/russgray/following{/other_user}","gists_url":"https://api.github.com/users/russgray/gists{/gist_id}","starred_url":"https://api.github.com/users/russgray/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/russgray/subscriptions","organizations_url":"https://api.github.com/users/russgray/orgs","repos_url":"https://api.github.com/users/russgray/repos","events_url":"https://api.github.com/users/russgray/events{/privacy}","received_events_url":"https://api.github.com/users/russgray/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":1,"created_at":"2015-02-04T15:24:39Z","updated_at":"2015-02-04T15:31:02Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/dotnet/coreclr/pulls/63","html_url":"https://github.com/dotnet/coreclr/pull/63","diff_url":"https://github.com/dotnet/coreclr/pull/63.diff","patch_url":"https://github.com/dotnet/coreclr/pull/63.patch"},"body":"There are two overloads of ConcurrentDictionary.GetOrAdd. One accepts a\r\nfactory method to generate the value if the key is not found, and calls\r\nTryGetValue for the existence check which is a lock-free operation.\r\n\r\nThe other overload, however, does not call TryGetValue - it simply calls\r\nstraight through to TryAddInternal, which will obtain a bucket lock *before it\r\ndoes the existence check*. This violates the documented claim of\r\nConcurrentDictionary of lock-free reads. This commit simply adds the same\r\nTryGetValue check, making the two overloads consistent."},"comment":{"url":"https://api.github.com/repos/dotnet/coreclr/issues/comments/72875209","html_url":"https://github.com/dotnet/coreclr/pull/63#issuecomment-72875209","issue_url":"https://api.github.com/repos/dotnet/coreclr/issues/63","id":72875209,"user":{"login":"stephentoub","id":2642209,"avatar_url":"https://avatars.githubusercontent.com/u/2642209?v=3","gravatar_id":"","url":"https://api.github.com/users/stephentoub","html_url":"https://github.com/stephentoub","followers_url":"https://api.github.com/users/stephentoub/followers","following_url":"https://api.github.com/users/stephentoub/following{/other_user}","gists_url":"https://api.github.com/users/stephentoub/gists{/gist_id}","starred_url":"https://api.github.com/users/stephentoub/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/stephentoub/subscriptions","organizations_url":"https://api.github.com/users/stephentoub/orgs","repos_url":"https://api.github.com/users/stephentoub/repos","events_url":"https://api.github.com/users/stephentoub/events{/privacy}","received_events_url":"https://api.github.com/users/stephentoub/received_events","type":"User","site_admin":false},"created_at":"2015-02-04T15:31:02Z","updated_at":"2015-02-04T15:31:02Z","body":"Thanks, @russgray.  The fix LGTM. Stylewise, I'd prefer to see this simplified to avoid multiple return statements for the same thing, e.g. instead of:\r\n```C#\r\nTValue resultingValue;\r\nif (TryGetValue(key, out resultingValue))\r\n{\r\n    return resultingValue;\r\n}\r\nTryAddInternal(key, value, false, true, out resultingValue);\r\nreturn resultingValue;\r\n```\r\ndoing:\r\n```C#\r\nTValue resultingValue;\r\nif (!TryGetValue(key, out resultingValue))\r\n{\r\n    TryAddInternal(key, value, false, true, out resultingValue);\r\n}\r\nreturn resultingValue;\r\n```\r\nI realize you were just copying the style that was already there for the other overload.  Want to change both?"}}