Caching data by using the result of first running operation My code handles concurrent requests by waiting for the result of an already running operation. Requests for data may come in simultaneously with same/different credentials (including empty credentials).

For each unique set of credentials there can be at most one GetCurrentInternal call in progress, with the result from that one call returned to all queued waiters when it is ready.

private static readonly Credentials EmptyCredentials = new Credentials
{
    SqlCredentials = null,
    ExchangeCredentials = null,
};

public AgentMetadata GetCurrent(Credentials credentials)
{
    var agentMetadata = new Lazy&lt;AgentMetadata&gt;(() =&gt; GetCurrentInternal(credentials));

    var lazyMetadata = (Lazy&lt;AgentMetadata&gt;)MemoryCache.Default.AddOrGetExisting(
                       (credentials ?? EmptyMetadataCredentials).ToString(),
                       agentMetadata,
                       DateTime.Now.AddMinutes(5));

    try
    {
        return (lazyMetadata ?? agentMetadata).Value;
    }
    catch (InvalidOperationException ex)
    {
        _logger.ErrorException(ex, "An error occurs during getting full metadata");
        throw;
    }
}


Is it thread safe? Do I have a bugs? How to create a good key for the AddOrGetExisting?

Credentials its a DataContract that contains other DataContract like SqlCredentials, ExchangeCredentials. And they contains their own strings like user_name and password.
Yes, it seems thread safe.

I run a test to prove it which I give the details at the bottom, but first are some code review comments.


The code is missing the Credentials class which I think is also important for the review. The cache items are added with keys credentials.ToString() and it is important for this method to return the same value for Credentials objects having the same credential values, and distinct values for Credentials instances with different credential values. Is it?
Naming conventions. Class names should be PascalCase and the class AgentMetadata violates this rule. Should be AgentMetaData
To achieve better encapsulation and seperation of concerns, (The creation of an empty credential object should be a concern for the Credentials class, which has control over the internals of itself) instead of having a static readonly Credentials EmptyCredentials field in this class, it is better to have a public static readonly Credentials Empty field (or property getter) in Credentials class.


Instead of this:

Credentials empty = (credentials ?? EmptyMetadataCredentials);


Isn't it better to have this?

Credentials empty = (credentials ?? Credentials.Empty);



The variable names agentMetadata and lazyMetadata are not descriptive enough.


This is somewhat confusing:

return (lazyMetadata ?? agentMetadata).Value;


Having names like the following might eliminate that confusion:

var newLazyInstance = new Lazy&lt;AgentMetadata&gt;(() =&gt; GetCurrentInternal(credentials));

var cachedLazyInstance = (Lazy&lt;AgentMetadata&gt;)MemoryCache.Default.AddOrGetExisting(
                           (credentials ?? EmptyMetadataCredentials).ToString(),
                           newLazyInstance,
                           DateTime.Now.AddMinutes(5));

return (cachedLazyInstance ?? newLazyInstance).Value;


And here are the results of the test, which shows the GetCurrentInternal method is invoked a total of 5 times for all the 100 threads.

The test created 100 threads which access the GetCurrent method randomly within 500 ms. There is another thread for removing the item from the cache in 100 ms continuously (This is because, the cache policy doesn't seem to effectively remove the item at the exact point of time of expiration).

The GetCurrentInternal method lasts for a random time between 0 and 500 ms

With the following results, it can be seen that the GetCurrentInternal method is invoked a total of 5 times (which is the number of the cache not containing a Lazy instance for the given credentials, first one because the cache is empty, and for 4 more times because it is removed from the cache) 

I can't help myself thinking why the AddOrGetExisting of MemoryCache returns null  when the item is added to the cache. This would have been implemented differently (as returning the added instance as ConcurrentDictionary does) 

T:47    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 145,68
T:13    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 146,44
T:20    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 146,54
T:26    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 147,45
T:71    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 148,46
T:62    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 148,55
T:53    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 149,37
T:5 M:GetCurrent    E:AddOrGetExisting.Added?   R:False 146,68
T:22    M:GetCurrent    E:AddOrGetExisting.Added?   R:True  150,66
T:31    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 150,18
T:63    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 154,58
T:50    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 157,15
T:27    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 182,13
T:67    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 182,33
T:43    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 182,18
T:32    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 182,13
T:3 M:GetCurrent    E:AddOrGetExisting.Added?   R:False 187,24
T:55    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 194,8
T:29    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 197,13
T:28    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 214,17
T:4 M:GetCurrent    E:AddOrGetExisting.Added?   R:False 221,11
T:84    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 228,12
T:78    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 232,09
T:19    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 235,1
T:66    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 238,1
T:7 M:GetCurrent    E:AddOrGetExisting.Added?   R:False 242,14
T:61    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 245,11
T:49    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 246,15
T:23    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 250,08
T:85    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 251,1
T:81    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 254,1
T:98    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 254,12
T:44    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 254,13
T:91    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 255,25
T:65    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 257,2
T:59    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 259,33
T:56    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 269,1
T:34    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 271,11
T:89    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 271,12
T:41    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 279,12
T:100   M:GetCurrent    E:AddOrGetExisting.Added?   R:False 280,12
T:11    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 290,11
T:17    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 292,08
T:58    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 293,1
T:14    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 297,09
T:90    M:GetCurrent    E:AddOrGetExisting.Added?   R:True  303,12
T:75    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 312,15
T:46    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 318,17
T:45    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 321,11
T:10    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 322,1
T:92    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 328,1
T:80    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 339,12
T:8 M:GetCurrent    E:AddOrGetExisting.Added?   R:False 341,11
T:51    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 342,11
T:74    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 343,11
T:86    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 351,12
T:68    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 352,11
T:95    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 354,1
T:72    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 359,11
T:48    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 360,11
T:18    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 364,1
T:6 M:GetCurrent    E:AddOrGetExisting.Added?   R:False 374,12
T:35    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 377,09
T:15    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 377,11
T:93    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 378,1
T:33    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 379,1
T:9 M:GetCurrent    E:AddOrGetExisting.Added?   R:False 390,12
T:39    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 391,09
T:12    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 395,11
T:24    M:GetCurrent    E:AddOrGetExisting.Added?   R:True  401,11
T:40    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 402,11
T:30    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 404,09
T:70    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 411,11
T:102   M:GetCurrent    E:AddOrGetExisting.Added?   R:False 412,1
T:52    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 413,11
T:57    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 417,1
T:37    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 426,13
T:69    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 428,1
T:16    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 432,11
T:21    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 438,13
T:96    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 441,11
T:38    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 442,09
T:54    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 456,18
T:82    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 465,12
T:76    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 466,11
T:25    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 470,11
T:36    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 477,09
T:73    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 483,12
T:42    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 483,13
T:101   M:GetCurrent    E:AddOrGetExisting.Added?   R:False 491,13
T:64    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 494,12
T:60    M:GetCurrent    E:AddOrGetExisting.Added?   R:True  502,11
T:97    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 528,2
T:24    M:GetCurrentInternal    E:Invoked   R:AgentMetadata_ID=1    547,76
T:24    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=1    548,3
T:70    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=1    548,36
T:40    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=1    548,32
T:57    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=1    549,27
T:21    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=1    561,53
T:36    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=1    574,77
T:102   M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=1    548,4
T:38    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=1    563,24
T:101   M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=1    575,21
T:77    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 579,16
T:25    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=1    571,16
T:54    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=1    567,6
T:73    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=1    574,79
T:42    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=1    574,8
T:76    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=1    567,61
T:37    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=1    554,64
T:82    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=1    567,6
T:52    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=1    548,38
T:64    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=1    575,44
T:16    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=1    557,74
T:94    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 587,92
T:69    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=1    556,54
T:96    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=1    562,78
T:30    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=1    548,35
T:88    M:GetCurrent    E:AddOrGetExisting.Added?   R:True  616,19
T:103   M:GetCurrent    E:AddOrGetExisting.Added?   R:False 631,16
T:90    M:GetCurrentInternal    E:Invoked   R:AgentMetadata_ID=2    642,16
T:90    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=2    642,37
T:80    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=2    644,54
T:72    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    648,45
T:47    M:GetCurrentInternal    E:Invoked   R:AgentMetadata_ID=3    645,16
T:47    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    650,61
T:26    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    652,63
T:22    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    654,89
T:9 M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    654,43
T:86    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    646,88
T:18    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    648,75
T:6 M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    649,06
T:15    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    650,84
T:74    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    646,62
T:53    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    658,94
T:31    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    662,34
T:46    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=2    642,4
T:62    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    652,69
T:95    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    646,88
T:67    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    664,69
T:71    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    660,52
T:93    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    650,84
T:45    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=2    642,4
T:50    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    652,69
T:23    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    677,68
T:78    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    672,49
T:66    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    672,5
T:4 M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    671,15
T:10    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=2    642,44
T:5 M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    657,11
T:65    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    678,46
T:29    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    669,35
T:81    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    679,46
T:33    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    649,06
T:92    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=2    642,46
T:14    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    688,26
T:13    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    652,62
T:17    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    688,25
T:28    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    671,12
T:89    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    684,54
T:11    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    686,67
T:100   M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    686,67
T:58    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    688,25
T:61    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    676,52
T:20    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    660,51
T:34    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    684,53
T:68    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    646,88
T:85    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    674,46
T:44    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    677,66
T:49    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    673,36
T:32    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    667,5
T:3 M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    669,31
T:41    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    686,66
T:39    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    654,43
T:7 M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    672,5
T:48    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    648,47
T:12    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    655,34
T:19    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    672,5
T:83    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 672,09
T:98    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    680,64
T:63    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    662,4
T:91    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    677,96
T:8 M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=2    644,56
T:75    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=2    642,38
T:43    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    665,46
T:27    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    662,37
T:59    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    680,63
T:55    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    669,34
T:99    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 695,73
T:51    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    646,62
T:56    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    684,53
T:84    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    672,24
T:87    M:GetCurrent    E:AddOrGetExisting.Added?   R:False 691,88
T:35    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=3    650,56
T:60    M:GetCurrentInternal    E:Invoked   R:AgentMetadata_ID=4    988,26
T:60    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=4    988,88
T:77    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=4    988,96
T:94    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=4    989,03
T:97    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=4    988,97
T:88    M:GetCurrentInternal    E:Invoked   R:AgentMetadata_ID=5    1050,3
T:88    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=5    1050,85
T:83    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=5    1050,94
T:87    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=5    1052,18
T:103   M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=5    1050,91
T:99    M:GetCurrent    E:Lazy.Value    R:AgentMetadata_ID=5    1050,97