jeme edited a comment on issue #417: URL: https://github.com/apache/lucenenet/issues/417#issuecomment-779662497
David Fowlers statement makes it sound like the implementation would call "the" callback (singular) multiple times, I don't feel like that is quite an accurate way of describing the behavior. In the actual behavior, it makes a bit more sense why this happens. The basics is that the factory is called outside of a lock, which means if you call GetOrAdd simultaneously on two threads, each or their value factories will be executed. This may be a good read: https://andrewlock.net/making-getoradd-on-concurrentdictionary-thread-safe-using-lazy/ Usages: ## ConcurrentDictionary<TKey, TValue> Reference source: * https://github.com/dotnet/runtime/blob/8399bae467ad521af58d37ae0fa27fd7df9e5e26/src/libraries/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentDictionary.cs * https://github.com/microsoft/referencesource/blob/master/mscorlib/system/collections/Concurrent/ConcurrentDictionary.cs * https://github.com/dotnet/corefx/blob/v3.1.12/src/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentDictionary.cs ### GetOrAdd **Lucene.Net:** https://github.com/apache/lucenenet/blob/master/src/Lucene.Net/Support/IO/FileSupport.cs#L363 -> Probably not that big of a deal, paths are always of a limited length so the code should be reasonably fast and produce the same result. https://github.com/apache/lucenenet/blob/master/src/Lucene.Net/Support/Configuration/EnvironmentVariablesConfigurationProvider.cs#L67 -> Probably not that big of a deal, It should produce the same result and is not likely to be that expensive. https://github.com/apache/lucenenet/blob/master/src/Lucene.Net/Search/SearcherLifetimeManager.cs#L174 -> Already uses lazy so the worst case scenario here is that we end up creating to Lazy objects, then discarding one of them immediately and never running it's factory. **Lucene.Net.TestFramework:** https://github.com/apache/lucenenet/blob/f5cca0b1a7c855f2fe44a5cdd27763aab7acdf22/src/Lucene.Net.TestFramework/Support/Configuration/TestConfigurationFactory.cs#L56 -> Could be candidate for using Lazy pattern unless we say that this is "TestFramework" and therefore unlikely to cause a problem. Also I did not dive deep into what the builder does and how expensive it is. The fix is not that problematic in the end though. **Lucene.Net.Spatial:** https://github.com/apache/lucenenet/blob/d5c5ca4b066ae7ba4839519ec162594844fbc10a/src/Lucene.Net.Spatial/Prefix/PrefixTreeStrategy.cs#L225 -> Assuming inexpensive call to constructor. ### AddOrUpdate _Could not find any usages._ ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected]
