diff --git a/Yubico.Core/tests/Yubico/Core/Devices/Hid/HidTranslatorTests.cs b/Yubico.Core/tests/Yubico/Core/Devices/Hid/HidTranslatorTests.cs index da42e12ba..177c3f5ae 100644 --- a/Yubico.Core/tests/Yubico/Core/Devices/Hid/HidTranslatorTests.cs +++ b/Yubico.Core/tests/Yubico/Core/Devices/Hid/HidTranslatorTests.cs @@ -146,7 +146,10 @@ public void TestSpecializedKeyboardSupportsModhexString(KeyboardLayout layout) #if Windows #pragma warning disable CA1825 [Theory] +#pragma warning disable CA1825 // Avoid zero-length array allocations + // The compiler mistakenly thinks that this somehow involves a zero-length array. [MemberData(nameof(GetTestData))] +#pragma warning restore CA1825 // Avoid zero-length array allocations public void GetChar_GivenHidCode_ReturnsCorrectChar(KeyboardLayout layout, (char, byte)[] testData) { var hid = HidCodeTranslator.GetInstance(layout); diff --git a/Yubico.YubiKey/src/Yubico/YubiKey/YubiKeyDeviceListener.cs b/Yubico.YubiKey/src/Yubico/YubiKey/YubiKeyDeviceListener.cs index 4dc359c66..fd9c2830e 100644 --- a/Yubico.YubiKey/src/Yubico/YubiKey/YubiKeyDeviceListener.cs +++ b/Yubico.YubiKey/src/Yubico/YubiKey/YubiKeyDeviceListener.cs @@ -17,6 +17,7 @@ using System.Linq; using System.Threading; using Microsoft.Extensions.Logging; +using System.Threading.Tasks; using Yubico.Core.Devices; using Yubico.Core.Devices.Hid; using Yubico.Core.Devices.SmartCard; @@ -44,82 +45,84 @@ public class YubiKeyDeviceListener : IDisposable /// /// An instance of a . /// - public static YubiKeyDeviceListener Instance => _lazyInstance.Value; + public static YubiKeyDeviceListener Instance => _lazyInstance ??= new YubiKeyDeviceListener(); - private static readonly Lazy _lazyInstance = - new Lazy(() => new YubiKeyDeviceListener()); + /// + /// Disposes and closes the singleton instance of . + /// + public static void ResetInstance() + { + if (_lazyInstance != null) + { + _lazyInstance.Dispose(); + _lazyInstance = null; + } + } - private static readonly ReaderWriterLockSlim RwLock = new ReaderWriterLockSlim(LockRecursionPolicy.NoRecursion); + private static YubiKeyDeviceListener? _lazyInstance; - private readonly ILogger _log = Log.GetLogger(); + private readonly ReaderWriterLockSlim _rwLock = new ReaderWriterLockSlim(LockRecursionPolicy.NoRecursion); + private readonly Logger _log = Log.GetLogger(); private readonly Dictionary _internalCache = new Dictionary(); private readonly HidDeviceListener _hidListener = HidDeviceListener.Create(); private readonly SmartCardDeviceListener _smartCardListener = SmartCardDeviceListener.Create(); - private readonly Thread? _listenerThread; - private readonly bool _isListening; + private readonly SemaphoreSlim _semaphore = new SemaphoreSlim(1); + private readonly Task _listenTask; + private readonly CancellationTokenSource _tokenSource = new CancellationTokenSource(); + private CancellationToken CancellationToken => _tokenSource.Token; + + private bool _isListening; private YubiKeyDeviceListener() { - _listenerThread = new Thread(ListenForChanges) { IsBackground = true }; - _isListening = true; + _log.LogInformation("Creating YubiKeyDeviceListener instance."); + + SetupDeviceListeners(); _log.LogInformation("Performing initial cache population."); Update(); - _listenerThread.Start(); + _listenTask = ListenForChanges(); } internal List GetAll() => _internalCache.Keys.ToList(); - private void ListenForChanges() - { - using var updateEvent = new ManualResetEvent(false); - - _log.LogInformation("YubiKey device listener thread started. ThreadID is {ThreadID}.", Environment.CurrentManagedThreadId); - - _smartCardListener.Arrived += (s, e) => - { - _log.LogInformation("Arrival of smart card {SmartCard} is triggering update.", e.Device); - _ = updateEvent.Set(); - }; - - _smartCardListener.Removed += (s, e) => - { - _log.LogInformation("Removal of smart card {SmartCard} is triggering update.", e.Device); - _ = updateEvent.Set(); - }; - - _hidListener.Arrived += (s, e) => - { - _log.LogInformation("Arrival of HID {HidDevice} is triggering update.", e.Device); - _ = updateEvent.Set(); - }; + private void ListenerHandler(object? sender, EventArgs e) => _semaphore.Release(); - _hidListener.Removed += (s, e) => - { - _log.LogInformation("Removal of HID {HidDevice} is triggering update.", e.Device); - _ = updateEvent.Set(); - }; + private void SetupDeviceListeners() + { + _smartCardListener.Arrived += ListenerHandler; + _smartCardListener.Removed += ListenerHandler; + _hidListener.Arrived += ListenerHandler; + _hidListener.Removed += ListenerHandler; + } + private async Task ListenForChanges() + { + _isListening = true; while (_isListening) { - _ = updateEvent.WaitOne(); - Thread.Sleep(200); // I really dislike sleeps, but here, it does seem like a good idea to give the - // system some time to quiet down in terms of PnP activity. - _ = updateEvent.Reset(); - Update(); + try + { + await _semaphore.WaitAsync(CancellationToken).ConfigureAwait(false); + // Give events a chance to coalesce. + await Task.Delay(200, CancellationToken).ConfigureAwait(false); + Update(); + // Reset any outstanding events. + _ = await _semaphore.WaitAsync(0, CancellationToken).ConfigureAwait(false); + } + catch (OperationCanceledException) + { + break; + } } - - // KeepAlive seems to be necessary here as the collector doesn't know that it shouldn't dispose the - // event until the very end of this function/thread. - GC.KeepAlive(updateEvent); } private void Update() { - RwLock.EnterWriteLock(); + _rwLock.EnterWriteLock(); _log.LogInformation("Entering write-lock."); ResetCacheMarkers(); @@ -209,7 +212,7 @@ private void Update() } _log.LogInformation("Exiting write-lock."); - RwLock.ExitWriteLock(); + _rwLock.ExitWriteLock(); } private List GetDevices() @@ -349,9 +352,7 @@ private static void ErrorHandler(Exception exception) => .GetLogger(typeof(YubiKeyDeviceListener).FullName!) .LogWarning($"Exception caught: {exception}"); - #region IDisposable Support - - private bool _disposedValue; + private bool _isDisposed; /// /// Disposes the objects. @@ -359,13 +360,38 @@ private static void ErrorHandler(Exception exception) => /// protected virtual void Dispose(bool disposing) { - if (!_disposedValue) + if (!_isDisposed) { if (disposing) { - RwLock.Dispose(); + // Signal the listen thread that it's time to end. + _tokenSource.Cancel(); + + // Shut down the listener handlers. + _hidListener.Arrived -= ListenerHandler; + _hidListener.Removed -= ListenerHandler; + if (_hidListener is IDisposable hidDisp) + { + hidDisp.Dispose(); + } + + _smartCardListener.Arrived -= ListenerHandler; + _smartCardListener.Removed -= ListenerHandler; + if (_smartCardListener is IDisposable scDisp) + { + scDisp.Dispose(); + } + + // Give the listen thread a moment (likely is already done). + _ = !_listenTask.Wait(100); + _listenTask.Dispose(); + + // Get rid of synchronization objects. + _rwLock.Dispose(); + _semaphore.Dispose(); + _tokenSource.Dispose(); } - _disposedValue = true; + _isDisposed = true; } } @@ -385,7 +411,5 @@ public void Dispose() Dispose(true); GC.SuppressFinalize(this); } - - #endregion } }