The existing locking implementation in the CachingService class is not robust. It is possible for the policy of exclusive execution per key to be violated, because the lock is released without proper memory fences. The details about when and why this can happen, can be found in this StackOverflow question, where I asked specifically about the locking code of the CachingService class, and was answered by an expert.
Also the existing spinning logic, using the Interlocked.CompareExchange and Thread.Yield APIs, is unlikely to be as efficient as the simple lock statement. So with this pull request I am proposing to replace the existing int[] keyLocks with an object[] keyLocks, initialize the array with a new object() in each slot, and use the normal and trustworthy lock. It should have no observable difference compared with the current behavior, apart from enforcing properly the exclusive execution policy.
Note: The code in this pull request has not been tested, and might have syntax errors.
The existing locking implementation in the
CachingServiceclass is not robust. It is possible for the policy of exclusive execution per key to be violated, because the lock is released without proper memory fences. The details about when and why this can happen, can be found in this StackOverflow question, where I asked specifically about the locking code of theCachingServiceclass, and was answered by an expert.Also the existing spinning logic, using the
Interlocked.CompareExchangeandThread.YieldAPIs, is unlikely to be as efficient as the simplelockstatement. So with this pull request I am proposing to replace the existingint[] keyLockswith anobject[] keyLocks, initialize the array with anew object()in each slot, and use the normal and trustworthylock. It should have no observable difference compared with the current behavior, apart from enforcing properly the exclusive execution policy.Note: The code in this pull request has not been tested, and might have syntax errors.