diff --git a/src/Microsoft.IdentityModel.Tokens/BaseConfigurationManager.cs b/src/Microsoft.IdentityModel.Tokens/BaseConfigurationManager.cs index 2124c416c4..425f57f599 100644 --- a/src/Microsoft.IdentityModel.Tokens/BaseConfigurationManager.cs +++ b/src/Microsoft.IdentityModel.Tokens/BaseConfigurationManager.cs @@ -119,7 +119,7 @@ public BaseConfiguration LastKnownGoodConfiguration _lastKnownGoodConfigFirstUse = DateTime.UtcNow; // LRU cache will remove the expired configuration - _lastKnownGoodConfigurationCache.SetValue(_lastKnownGoodConfiguration, DateTime.UtcNow + LastKnownGoodLifetime, DateTime.UtcNow + LastKnownGoodLifetime); + _lastKnownGoodConfigurationCache.TrySetValue(_lastKnownGoodConfiguration, DateTime.UtcNow + LastKnownGoodLifetime, DateTime.UtcNow + LastKnownGoodLifetime); } } diff --git a/src/Microsoft.IdentityModel.Tokens/EventBasedLRUCache.cs b/src/Microsoft.IdentityModel.Tokens/EventBasedLRUCache.cs index 5414c89064..ca1d1af6ea 100644 --- a/src/Microsoft.IdentityModel.Tokens/EventBasedLRUCache.cs +++ b/src/Microsoft.IdentityModel.Tokens/EventBasedLRUCache.cs @@ -50,34 +50,24 @@ internal class EventBasedLRUCache // if true, expired values will not be added to the cache and clean-up of expired values will occur on a 5 minute interval private readonly bool _removeExpiredValues; private readonly int _removeExpiredValuesIntervalInSeconds; - // for testing purpose only to verify the task count - private int _taskCount; private DateTime _timeForNextExpiredValuesRemoval; - private DateTime _timeForNextCompaction; #region event queue - private int _eventQueuePollingInterval = 50; - // The idle timeout, the _eventQueueTask will end after being idle for the specified time interval (execution continues even if the queue is empty to reduce the task startup overhead), default to 120 seconds. - // TODO: consider implementing a better algorithm that tracks and predicts the usage patterns and adjusts this value dynamically. - private long _eventQueueTaskIdleTimeoutInSeconds = 120; - // The time when the _eventQueueTask should end. The intent is to reduce the overhead costs of starting/ending tasks too frequently - // but at the same time keep the _eventQueueTask a short running task. - // Since Task is based on thread pool the overhead should be reasonable. - private DateTime _eventQueueTaskStopTime; - // task states used to ensure thread safety (Interlocked.CompareExchange) - private const int EventQueueTaskStopped = 0; // task not started yet - private const int EventQueueTaskRunning = 1; // task is running - private const int EventQueueTaskDoNotStop = 2; // force the task to continue even it has past the _eventQueueTaskStopTime, see StartEventQueueTaskIfNotRunning() for more details. - private int _eventQueueTaskState = EventQueueTaskStopped; - private const int ActionNotQueued = 0; // compaction action not in the event queue private const int ActionQueuedOrRunning = 1; // compaction action in the event queue or currently in progress + private int _compactValuesState = ActionNotQueued; private int _removeExpiredValuesState = ActionNotQueued; private int _processCompactedValuesState = ActionNotQueued; + private readonly Task _eventQTask; + private int _eventQueuePollingInterval = 50; // in milliseconds + + // In moving the creation of the task to the constructor, there will only be one Task. + // We kept the internal TaskCount and need to reference an instance variable to keep the method the same. + private int _taskCount = 1; // set to true when the AppDomain is to be unloaded or the default AppDomain process is ready to exit - private bool _shouldStopImmediately; + private bool _stopEventQueueTask; internal ItemExpired OnItemExpired { get; set; } @@ -95,17 +85,6 @@ internal ItemExpired OnItemRemoved internal ItemRemoved OnItemRemovedFromCompactedList { get; set; } internal ShouldRemove OnShouldRemoveFromCompactedList { get; set; } - - internal long EventQueueTaskIdleTimeoutInSeconds - { - get => _eventQueueTaskIdleTimeoutInSeconds; - set - { - if (value <= 0) - throw new ArgumentOutOfRangeException(nameof(value), "EventQueueTaskExecutionTimeInSeconds must be positive."); - _eventQueueTaskIdleTimeoutInSeconds = value; - } - } #endregion /// @@ -134,9 +113,8 @@ internal EventBasedLRUCache( _removeExpiredValues = removeExpiredValues; _compactIntervalInSeconds = compactIntervalInSeconds; _timeForNextExpiredValuesRemoval = DateTime.UtcNow.AddSeconds(_removeExpiredValuesIntervalInSeconds); - _timeForNextCompaction = DateTime.UtcNow.AddSeconds(_compactIntervalInSeconds); - _eventQueueTaskStopTime = DateTime.UtcNow; _maintainLRU = maintainLRU; + _eventQTask = Task.Run(EventQueueTaskAction); } /// @@ -157,20 +135,11 @@ internal EventBasedLRUCache( /// Stop the event queue task. /// This is provided mainly for users who have unit tests that check for running task(s) to stop the task at the end of each test. /// - internal void StopEventQueueTask() => StopEventQueueTaskImmediately(); - - /// - /// Stop the event queue task immediately if it is running. This allows the task/thread to terminate gracefully. - /// Currently there is no unmanaged resource, if any is added in the future it should be disposed of in this method. - /// - private void StopEventQueueTaskImmediately() => _shouldStopImmediately = true; + internal void StopEventQueueTaskImmediately() => _stopEventQueueTask = true; private void AddActionToEventQueue(Action action) { _eventQueue.Enqueue(action); - - // start the event queue task if it is not running - StartEventQueueTaskIfNotRunning(); } public bool Contains(TKey key) @@ -187,14 +156,13 @@ public bool Contains(TKey key) private void EventQueueTaskAction() { Interlocked.Increment(ref _taskCount); + try { - // Keep running until the queue is empty or the AppDomain is about to be unloaded or the application is ready to exit. - while (!_shouldStopImmediately) + // Keep running until instructed to stop. + // When the event queue is empty, the thread will sleep for a specified number of milliseconds before checking again. + while (!_stopEventQueueTask) { - // always set the state to EventQueueTaskRunning in case it was set to EventQueueTaskDoNotStop - Interlocked.Exchange(ref _eventQueueTaskState, EventQueueTaskRunning); - try { // remove expired items if needed @@ -209,23 +177,11 @@ private void EventQueueTaskAction() } } - // process all events in the queue and exit if (_eventQueue.TryDequeue(out var action)) { action?.Invoke(); } - else if (DateTime.UtcNow > _eventQueueTaskStopTime) // no more event to be processed, exit if expired - { - // Setting _eventQueueTaskState = EventQueueTaskStopped if the _eventQueueStopTime has past and _eventQueueTaskState == EventQueueTaskRunning. - // This means no other thread came in and it is safe to end this task. - // If another thread adds new events while this task is still running, it will set the _eventQueueTaskState = EventQueueTaskDoNotStop instead of starting a new task. - // The Interlocked.CompareExchange() call below will not succeed and the loop continues (until the event queue is empty and the _eventQueueTaskEndTime expires again). - // This should prevent a rare (but theoretically possible) scenario caused by context switching. - if (Interlocked.CompareExchange(ref _eventQueueTaskState, EventQueueTaskStopped, EventQueueTaskRunning) == EventQueueTaskRunning) - break; - - } - else // if empty, let the thread sleep for a specified number of milliseconds before attempting to retrieve another value from the queue + else { Thread.Sleep(_eventQueuePollingInterval); } @@ -237,11 +193,13 @@ private void EventQueueTaskAction() } } } - finally + catch (Exception ex) { - Interlocked.Decrement(ref _taskCount); - Interlocked.Exchange(ref _eventQueueTaskState, EventQueueTaskStopped); + if (LogHelper.IsEnabled(EventLogLevel.Warning)) + LogHelper.LogWarning(LogHelper.FormatInvariant(LogMessages.IDX10900, ex)); } + + Interlocked.Decrement(ref _taskCount); } /// @@ -250,7 +208,6 @@ private void EventQueueTaskAction() /// Number of items removed. internal void RemoveExpiredValuesLRU() { -#pragma warning disable CA1031 // Do not catch general exception types try { LinkedListNode> node = _doubleLinkedList.First; @@ -259,15 +216,19 @@ internal void RemoveExpiredValuesLRU() LinkedListNode> nextNode = node.Next; if (node.Value.ExpirationTime < DateTime.UtcNow) { - _doubleLinkedList.Remove(node); if (_map.TryRemove(node.Value.Key, out LRUCacheItem cacheItem)) + { OnItemExpired?.Invoke(cacheItem.Value); + _doubleLinkedList.Remove(node); + } } node = nextNode; } } +#pragma warning disable CA1031 // Do not catch general exception types catch (Exception ex) +#pragma warning restore CA1031 // Do not catch general exception types { if (LogHelper.IsEnabled(EventLogLevel.Warning)) LogHelper.LogWarning(LogHelper.FormatInvariant(LogMessages.IDX10902, LogHelper.MarkAsNonPII(nameof(RemoveExpiredValuesLRU)), ex)); @@ -277,7 +238,6 @@ internal void RemoveExpiredValuesLRU() _removeExpiredValuesState = ActionNotQueued; _timeForNextExpiredValuesRemoval = DateTime.UtcNow.AddSeconds(_removeExpiredValuesIntervalInSeconds); } -#pragma warning restore CA1031 // Do not catch general exception types } /// @@ -287,7 +247,6 @@ internal void RemoveExpiredValuesLRU() /// Number of items removed. internal void RemoveExpiredValues() { -#pragma warning disable CA1031 // Do not catch general exception types try { foreach (KeyValuePair> node in _map) @@ -299,19 +258,18 @@ internal void RemoveExpiredValues() } } } +#pragma warning disable CA1031 // Do not catch general exception types catch (Exception ex) +#pragma warning restore CA1031 // Do not catch general exception types { if (LogHelper.IsEnabled(EventLogLevel.Warning)) LogHelper.LogWarning(LogHelper.FormatInvariant(LogMessages.IDX10902, LogHelper.MarkAsNonPII(nameof(ProcessCompactedValues)), ex)); - } finally { _removeExpiredValuesState = ActionNotQueued; _timeForNextExpiredValuesRemoval = DateTime.UtcNow.AddSeconds(_removeExpiredValuesIntervalInSeconds); } - -#pragma warning restore CA1031 // Do not catch general exception types } /// @@ -319,7 +277,6 @@ internal void RemoveExpiredValues() /// internal void ProcessCompactedValues() { -#pragma warning disable CA1031 // Do not catch general exception types try { for (int i = _compactedItems.Count - 1; i >= 0; i--) @@ -331,7 +288,9 @@ internal void ProcessCompactedValues() } } } +#pragma warning disable CA1031 // Do not catch general exception types catch (Exception ex) +#pragma warning restore CA1031 // Do not catch general exception types { if (LogHelper.IsEnabled(EventLogLevel.Warning)) LogHelper.LogWarning(LogHelper.FormatInvariant(LogMessages.IDX10906, LogHelper.MarkAsNonPII(nameof(ProcessCompactedValues)), ex)); @@ -339,10 +298,7 @@ internal void ProcessCompactedValues() finally { _processCompactedValuesState = ActionNotQueued; - _timeForNextCompaction = DateTime.UtcNow.AddSeconds(_compactIntervalInSeconds); } - -#pragma warning restore CA1031 // Do not catch general exception types } /// @@ -358,10 +314,11 @@ private void CompactLRU() { LinkedListNode> node = _doubleLinkedList.Last; if (_map.TryRemove(node.Value.Key, out LRUCacheItem cacheItem)) + { OnItemMovedToCompactedList?.Invoke(cacheItem.Value); - - _compactedItems.Add(cacheItem); - _doubleLinkedList.RemoveLast(); + _compactedItems.Add(cacheItem); + _doubleLinkedList.RemoveLast(); + } } } finally @@ -414,25 +371,22 @@ protected int CalculateNewCacheSize() return currentCount - (int)(currentCount * _compactionPercentage); } - /// - /// This is the method that determines the end time for the event queue task. - /// The goal is to be able to track the incoming events and predict how long the task should run in order to - /// avoid a long running task and reduce the overhead costs of restarting tasks. - /// For example, maybe we can track the last three events' time and set the _eventQueueRunDurationInSeconds = 2 * average_time_between_events. - /// Note: tasks are based on thread pool so the overhead should not be huge but we should still try to minimize it. - /// - /// the time when the event queue task should end - private DateTime SetTaskEndTime() + public void SetValue(TKey key, TValue value) { - return DateTime.UtcNow.AddSeconds(EventQueueTaskIdleTimeoutInSeconds); + TrySetValue(key, value, DateTime.MaxValue); } - public void SetValue(TKey key, TValue value) + public bool SetValue(TKey key, TValue value, DateTime expirationTime) { - SetValue(key, value, DateTime.MaxValue); + return TrySetValue(key, value, expirationTime); } - public bool SetValue(TKey key, TValue value, DateTime expirationTime) + public bool TrySetValue(TKey key, TValue value) + { + return TrySetValue(key, value, DateTime.MaxValue); + } + + public bool TrySetValue(TKey key, TValue value, DateTime expirationTime) { if (key == null) throw LogHelper.LogArgumentNullException(nameof(key)); @@ -444,11 +398,15 @@ public bool SetValue(TKey key, TValue value, DateTime expirationTime) if (_removeExpiredValues && expirationTime < DateTime.UtcNow) return false; + if (Interlocked.CompareExchange(ref _compactValuesState, ActionQueuedOrRunning, ActionQueuedOrRunning) == ActionQueuedOrRunning) + return false; + // just need to update value and move it to the top if (_map.TryGetValue(key, out var cacheItem)) { cacheItem.Value = value; cacheItem.ExpirationTime = expirationTime; + if (_maintainLRU) { var localCacheItem = cacheItem; // avoid closure when !_maintainLRU @@ -472,79 +430,36 @@ public bool SetValue(TKey key, TValue value, DateTime expirationTime) else AddActionToEventQueue(Compact); - if (DateTime.UtcNow >= _timeForNextCompaction) - { - if (Interlocked.CompareExchange(ref _processCompactedValuesState, ActionQueuedOrRunning, ActionNotQueued) == ActionNotQueued) - { - _eventQueue.Enqueue(ProcessCompactedValues); - } - } + if (Interlocked.CompareExchange(ref _processCompactedValuesState, ActionQueuedOrRunning, ActionNotQueued) == ActionNotQueued) + AddActionToEventQueue(ProcessCompactedValues); } + + return false; } var newCacheItem = new LRUCacheItem(key, value, expirationTime); - - // add the new node to the _doubleLinkedList if _maintainLRU == true if (_maintainLRU) { - var localNewCacheItem = newCacheItem; // avoid closure when !_maintainLRU - var localThis = this; - AddActionToEventQueue(() => + if (_map.TryAdd(key, newCacheItem)) { - // Add a remove operation in case two threads are trying to add the same value. Only the second remove will succeed in this case. - localThis._doubleLinkedList.Remove(localNewCacheItem); - localThis._doubleLinkedList.AddFirst(localNewCacheItem); - }); + var localCacheItem = newCacheItem; // avoid closure on fast path or when !_maintainLRU + var localThis = this; + AddActionToEventQueue(() => + { + localThis._doubleLinkedList.Remove(localCacheItem); + localThis._doubleLinkedList.AddFirst(localCacheItem); + }); + } + } + else + { + _map[key] = newCacheItem; } - - _map[key] = newCacheItem; } return true; } - /// - /// This method is called after an item is added to the event queue. It will start the event queue task if one is not already running (_eventQueueTaskState != EventQueueTaskRunning). - /// Using CompareExchange to set the _eventQueueTaskState prevents multiple tasks from being started. - /// - private void StartEventQueueTaskIfNotRunning() - { - _eventQueueTaskStopTime = SetTaskEndTime(); // set the time when the _eventQueueTask should end - - // Setting _eventQueueTaskState to EventQueueTaskDoNotStop here will force the event queue task in EventQueueTaskAction to continue even it has past the _eventQueueTaskEndTime. - // It is mainly to prevent a rare (but theoretically possible) scenario caused by context switching - // For example: - // 1. the task execution in EventQueueTaskAction() checks event queue and it is empty (ready to exit) - // 2. the execution is switched to this thread (before the event queue task calls the Interlocked.CompareExchange() to set the _eventQueueTaskState to EventQueueTaskStopped) - // 3. now since the _eventQueueTaskState == EventQueueTaskRunning, it can be set to EventQueueTaskDoNotStop by the Interlocked.CompareExchange() below - // 4. if _eventQueueTaskState is successfully set to EventQueueTaskDoNotStop, the Interlocked.CompareExchange() in the EventQueueTaskAction() will fail - // and the task will continue the while loop and the new event will keep the task running - // 5. if _eventQueueTaskState is NOT set to EventQueueTaskDoNotStop because execution switches back to the EventQueueTaskAction() and the _eventQueueTaskState is - // set to EventQueueTaskStopped (task exits), then the second Interlocked.CompareExchange() below should set the _eventQueueTaskState to EventQueueTaskRunning - // and start a task again (though this scenario is unlikely to happen) - // - // Without the EventQueueTaskDoNotStop state check below, steps (3), (4) and (5) above will not be applicable. - // After step (2) the event queue task is still running and the state is still EventQueueTaskRunning (even though the EventQueueTaskAction() method has already checked that the queue is empty - // and is about to stop the task). This method (StartEventQueueTaskIfNotRunning()) will return, the execution will switch over to EventQueueTaskAction(), - // and the task will terminate. This means no new task would be started to process the newly added event. - // - // This scenario is unlikely to happen, as it can only occur if the event queue task ALREADY checked the queue and it was empty, and the new event was added AFTER that check but BEFORE the - // event queue task set the _eventQueueTaskState to EventQueueTaskStopped. - - if (Interlocked.CompareExchange(ref _eventQueueTaskState, EventQueueTaskDoNotStop, EventQueueTaskRunning) == EventQueueTaskRunning) - { - return; - } - - // If the task is stopped, set _eventQueueTaskState = EventQueueTaskRunning and start a new task. - // Note: we need to call the Task.Run() to start a new task on the default TaskScheduler (TaskScheduler.Default) so it does not interfere with - // the caller's TaskScheduler (if there is one) as some custom TaskSchedulers might be single-threaded and its execution can be blocked. - if (Interlocked.CompareExchange(ref _eventQueueTaskState, EventQueueTaskRunning, EventQueueTaskStopped) == EventQueueTaskStopped) - { - _ = Task.Run(EventQueueTaskAction); - } - } - internal KeyValuePair>[] ToArray() { return _map.ToArray(); @@ -652,6 +567,8 @@ public bool TryRemove(TKey key, out TValue value) /// /// FOR TESTING PURPOSES ONLY. /// This is for tests to verify all tasks exit at the end of tests if the queue is empty. + /// In moving the creation of the task to the constructor, there will only be one Task. + /// We kept the internal TaskCount and need to reference an instance variable to keep the method the same. /// internal int TaskCount => _taskCount; diff --git a/src/Microsoft.IdentityModel.Tokens/InMemoryCryptoProviderCache.cs b/src/Microsoft.IdentityModel.Tokens/InMemoryCryptoProviderCache.cs index 1cd4edf23e..94cbbc0427 100644 --- a/src/Microsoft.IdentityModel.Tokens/InMemoryCryptoProviderCache.cs +++ b/src/Microsoft.IdentityModel.Tokens/InMemoryCryptoProviderCache.cs @@ -122,7 +122,7 @@ private static string GetCacheKeyPrivate(SecurityKey securityKey, string algorit } /// - /// Trys to adds a to this cache. + /// Tries to add a to this cache. /// /// to cache. /// if signatureProvider is null. @@ -146,16 +146,18 @@ public override bool TryAdd(SignatureProvider signatureProvider) // The cache does NOT already have a crypto provider associated with this key. if (!signatureProviderCache.Contains(cacheKey)) { - signatureProviderCache.SetValue(cacheKey, signatureProvider); - signatureProvider.CryptoProviderCache = this; - return true; + if (signatureProviderCache.TrySetValue(cacheKey, signatureProvider)) + { + signatureProvider.CryptoProviderCache = this; + return true; + } } return false; } /// - /// Trys to find a to this cache. + /// Tries to find a in this cache. /// /// the key that is used to by the crypto provider. /// the algorithm that is used by the crypto provider. @@ -185,7 +187,7 @@ public override bool TryGetSignatureProvider(SecurityKey securityKey, string alg } /// - /// Trys to remove a from this cache. + /// Tries to remove a from this cache. /// /// to remove. /// if signatureProvider is null. @@ -243,8 +245,8 @@ protected virtual void Dispose(bool disposing) if (!_disposed) { // Stop the event queue tasks if they are running. - _signingSignatureProviders.StopEventQueueTask(); - _verifyingSignatureProviders.StopEventQueueTask(); + _signingSignatureProviders.StopEventQueueTaskImmediately(); + _verifyingSignatureProviders.StopEventQueueTaskImmediately(); _disposed = true; } diff --git a/src/Microsoft.IdentityModel.Tokens/InternalAPI.Unshipped.txt b/src/Microsoft.IdentityModel.Tokens/InternalAPI.Unshipped.txt index 4194681d26..5e92aed282 100644 --- a/src/Microsoft.IdentityModel.Tokens/InternalAPI.Unshipped.txt +++ b/src/Microsoft.IdentityModel.Tokens/InternalAPI.Unshipped.txt @@ -1,5 +1,9 @@ const Microsoft.IdentityModel.Tokens.AppContextSwitches.UseCapitalizedXMLTypeAttrSwitch = "Switch.Microsoft.IdentityModel.UseCapitalizedXMLTypeAttr" -> string const Microsoft.IdentityModel.Tokens.LogMessages.IDX10278 = "IDX10278: Unable to retrieve configuration from authority: '{0}'. \nProceeding with token decryption in case the relevant properties have been set manually on the TokenValidationParameters. Exception caught: \n {1}. See https://aka.ms/validate-using-configuration-manager for additional information." -> string +Microsoft.IdentityModel.Tokens.EventBasedLRUCache.TrySetValue(TKey key, TValue value) -> bool +Microsoft.IdentityModel.Tokens.EventBasedLRUCache.TrySetValue(TKey key, TValue value, System.DateTime expirationTime) -> bool +Microsoft.IdentityModel.Tokens.EventBasedLRUCache.StopEventQueueTaskImmediately() -> void +static Microsoft.IdentityModel.Tokens.AppContextSwitches.UseCapitalizedXMLTypeAttr.get -> bool static Microsoft.IdentityModel.Tokens.AppContextSwitches.UseCapitalizedXMLTypeAttr.get -> bool const Microsoft.IdentityModel.Telemetry.TelemetryConstants.BlockingTypeTag = "Blocking" -> string const Microsoft.IdentityModel.Telemetry.TelemetryDataRecorder.BackgroundConfigurationRefreshFailureCounterDescription = "Counter capturing configuration manager background refresh failures." -> string diff --git a/test/Microsoft.IdentityModel.Tokens.Tests/EventBasedLRUCacheTests.cs b/test/Microsoft.IdentityModel.Tokens.Tests/EventBasedLRUCacheTests.cs index 40dee7f54b..0254e9a23a 100644 --- a/test/Microsoft.IdentityModel.Tokens.Tests/EventBasedLRUCacheTests.cs +++ b/test/Microsoft.IdentityModel.Tokens.Tests/EventBasedLRUCacheTests.cs @@ -9,8 +9,9 @@ using Microsoft.IdentityModel.TestUtils; using Xunit; -namespace Microsoft.IdentityModel.Tokens.Tests +namespace Microsoft.IdentityModel.Tokens.EventBasedLRUCache.Tests { + [Collection("EventBasedLRU")] public class EventBasedLRUCacheTests { [Fact] @@ -19,7 +20,7 @@ public void Contains() TestUtilities.WriteHeader($"{this}.Contains"); var context = new CompareContext($"{this}.Contains"); var cache = new EventBasedLRUCache(10, removeExpiredValues: false); - cache.SetValue(1, "one"); + cache.TrySetValue(1, "one"); if (!cache.Contains(1)) context.AddDiff("Cache should contain the key value pair {1, 'one'}, but the Contains() method returned false."); @@ -49,7 +50,7 @@ public void DoNotRemoveExpiredValues() var context = new CompareContext($"{this}.DoNotRemoveExpiredValues"); var cache = new EventBasedLRUCache(11, removeExpiredValuesIntervalInSeconds: 5, removeExpiredValues: false); for (int i = 0; i <= 10; i++) - cache.SetValue(i, i.ToString(), DateTime.UtcNow + TimeSpan.FromSeconds(5)); + cache.TrySetValue(i, i.ToString(), DateTime.UtcNow + TimeSpan.FromSeconds(5)); Thread.Sleep(5000); @@ -147,9 +148,9 @@ private void AddItemsToCache(EventBasedLRUCache cache, int size, in { // Only even values should expire. if (i % 2 == 0) - cache.SetValue(i, i.ToString(), DateTime.UtcNow + TimeSpan.FromSeconds(expiredInSeconds)); + cache.TrySetValue(i, i.ToString(), DateTime.UtcNow + TimeSpan.FromSeconds(expiredInSeconds)); else - cache.SetValue(i, i.ToString()); + cache.TrySetValue(i, i.ToString()); } } @@ -178,24 +179,27 @@ public void SetValue() TestUtilities.WriteHeader($"{this}.SetValue"); var context = new CompareContext($"{this}.SetValue"); var cache = new EventBasedLRUCache(1, removeExpiredValues: false); - Assert.Throws(() => cache.SetValue(1, null)); + Assert.Throws(() => cache.TrySetValue(1, null)); - cache.SetValue(1, "one"); + cache.TrySetValue(1, "one"); if (!cache.Contains(1)) context.AddDiff("The key value pair {1, 'one'} should have been added to the cache, but the Contains() method returned false."); - cache.SetValue(1, "one"); + cache.TrySetValue(1, "one"); if (!cache.Contains(1)) context.AddDiff("The key value pair {1, 'one'} should have been added to the cache, but the Contains() method returned false."); // The LRU item should be removed, allowing this value to be added even though the cache is full. - cache.SetValue(2, "two"); - if (!cache.Contains(2)) + bool wasAdded = cache.TrySetValue(2, "two"); + if (cache.Contains(2)) context.AddDiff("The key value pair {2, 'two'} should have been added to the cache, but the Contains() method returned false."); + if (wasAdded) + context.AddDiff("The key value pair {2, 'two'} was added to the cache, but the cache was full."); + try { - cache.SetValue(null, "three"); + cache.TrySetValue(null, "three", DateTime.MaxValue); context.AddDiff("The first parameter passed into the SetValue() method was null, but no exception was thrown."); } catch (Exception ex) @@ -206,7 +210,7 @@ public void SetValue() try { - cache.SetValue(3, null); + cache.TrySetValue(3, null); context.AddDiff("The second parameter passed into the SetValue() method was null, but no exception was thrown."); } catch (Exception ex) @@ -224,7 +228,7 @@ public void TryGetValue() TestUtilities.WriteHeader($"{this}.TryGetValue"); var context = new CompareContext($"{this}.TryGetValue"); var cache = new EventBasedLRUCache(2, removeExpiredValues: false); - cache.SetValue(1, "one"); + cache.TrySetValue(1, "one"); if (!cache.TryGetValue(1, out var value)) { @@ -257,7 +261,7 @@ public void TryRemove() var context = new CompareContext($"{this}.RemoveValue"); var cache = new EventBasedLRUCache(1, removeExpiredValues: false); - cache.SetValue(1, "one"); + cache.TrySetValue(1, "one"); if (!cache.TryRemove(1, out _)) context.AddDiff("The key value pair {1, 'one'} should have been removed from the cache, but the TryRemove() method returned false."); @@ -287,7 +291,8 @@ public void MaintainLRUOrder() var cache = new EventBasedLRUCache(10, removeExpiredValues: false, maintainLRU: true); for (int i = 0; i <= 1000; i++) { - cache.SetValue(i, Guid.NewGuid().ToString()); + string guid = Guid.NewGuid().ToString(); + cache.TrySetValue(i, guid, DateTime.MaxValue); // check that list and map values match up every 10 items // every 10th item should result in two LRU items being removed @@ -307,8 +312,13 @@ public void MaintainLRUOrder() // and therefore are most recently used. if (!IsDescending(cache.LinkedList)) { - context.AddDiff("LRU order was not maintained."); + context.AddDiff($"LRU order was not maintained, i = '{i}'."); + foreach (var item in cache.LinkedList) + context.AddDiff($"Key: '{item.Key}', Value: '{item.Value}'."); } + + if (cache.MapCount != cache.LinkedList.Count) + context.AddDiff($"The cache map count: '{cache.MapCount}', does not match the linked list count: '{cache.LinkedListCount}'."); } } @@ -372,7 +382,7 @@ public async Task CacheOverflowTestMultithreaded() { taskList.Add(Task.Factory.StartNew(() => { - cache.SetValue(i, i.ToString()); + cache.TrySetValue(i, i.ToString()); })); } @@ -395,7 +405,7 @@ public void CacheOverflowTestSequential() for (int i = 0; i < 100000; i++) { - cache.SetValue(i, i.ToString()); + cache.TrySetValue(i, i.ToString()); } // Cache size should be less than the capacity (somewhere between 800-1000 items).