NightOwl888 commented on issue #417: URL: https://github.com/apache/lucenenet/issues/417#issuecomment-779708526
@jeme Thanks for the insight. `Lazy<T>` does seem like a reasonable remedy in most (all?) cases where the call is expensive or needs to be atomic for another reason. > NOTE: I have read that [`LazyInitializer` is faster than `Lazy<T>`](https://stackoverflow.com/a/16116758), but given the fact that the former uses static methods I am not sure if we can find a way to make use of it in this scenario. However, `GetOrAdd` is not the only API we need to watch out for. There are several usages of APIs of collections that may have been assumed to be atomic where locks have been replaced with these API calls, where the documentation states the calls are made outside of the context of locks. ### ConditionalWeakTable<TKey, TValue> - [`GetValue`](https://docs.microsoft.com/en-us/dotnet/api/system.runtime.compilerservices.conditionalweaktable-2.getvalue?view=net-5.0) - [`AddOrUpdate`](https://docs.microsoft.com/en-us/dotnet/api/system.runtime.compilerservices.conditionalweaktable-2.addorupdate?view=net-5.0) - [`GetOrCreateValue`](https://docs.microsoft.com/en-us/dotnet/api/system.runtime.compilerservices.conditionalweaktable-2.getorcreatevalue?view=net-5.0) ### ConcurrentDictionary<TKey, TValue> - [`AddOrUpdate`](https://docs.microsoft.com/en-us/dotnet/api/system.collections.concurrent.concurrentdictionary-2.addorupdate?view=net-5.0) - [`GetOrAdd`](https://docs.microsoft.com/en-us/dotnet/api/system.collections.concurrent.concurrentdictionary-2.getoradd?view=net-5.0) ### LurchTable<TKey, TValue> - [`GetOrAdd`](https://github.com/NightOwl888/J2N/blob/b24a28da203b55aba860f16d955ce3b1788fcd42/src/J2N/Collections/Concurrent/LurchTable.cs#L1019-L1032) - [`GetOrAdd`](https://github.com/NightOwl888/J2N/blob/b24a28da203b55aba860f16d955ce3b1788fcd42/src/J2N/Collections/Concurrent/LurchTable.cs#L1096-L1112) - [`AddOrUpdate`](https://github.com/NightOwl888/J2N/blob/b24a28da203b55aba860f16d955ce3b1788fcd42/src/J2N/Collections/Concurrent/LurchTable.cs#L1114-L1160) In addition, we should expand the search to include all of Lucene.NET's dependencies that we maintain. - [J2N](https://github.com/NightOwl888/J2N) - [ICU4N](https://github.com/NightOwl888/ICU4N) - [Spatial4n](https://github.com/NightOwl888/Spatial4n) - [Morfologik.Stemming](https://github.com/NightOwl888/Morfologik.Stemming) Although the documentation doesn't specify that locking is an issue with `TryGet`, `TryAdd`, `TryRemove`, we should probably do a code review on those methods of the above collections (where applicable) as well as any caches that are built using them to ensure we are not missing any important locks. Just doing a quick survey, I noticed there are some calls that should have locks in J2N and it is suspected that this may be the source of concurrency issues with [`ThaiTokenizer`](https://github.com/apache/lucenenet/blob/f5cca0b1a7c855f2fe44a5cdd27763aab7acdf22/src/Lucene.Net.Analysis.Common/Analysis/Th/ThaiTokenizer.cs) and [`ICUTokenizer`](https://github.com/apache/lucenenet/blob/427e23013981b13d943ff1e5566c1c90241c8074/src/Lucene.Net.Analysis.ICU/Analysis/Icu/Segmentation/ICUTokenizer.cs#L60) tests of Lucene.NET, which are [known to fail](https://github.com/apache/lucenenet/issues/269) without extra locking that was not part of the original design as a result of missing locks in ICU4N. ---------------------------------------------------------------- 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]
