Skip to content

Commit

Permalink
Restore add and removal logging
Browse files Browse the repository at this point in the history
Also, added some documentation and a simple test.

Also, rebased on develop.
  • Loading branch information
jamiehankins committed Aug 16, 2024
1 parent 63eea0e commit 8c1230a
Show file tree
Hide file tree
Showing 3 changed files with 100 additions and 13 deletions.
5 changes: 5 additions & 0 deletions Yubico.YubiKey/src/Yubico/YubiKey/YubiKeyDevice.Static.cs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ public partial class YubiKeyDevice
/// <see cref="IYubiKeyDevice"/> using their serial number. If they cannot be matched,
/// each connection will be returned as a separate <see cref="IYubiKeyDevice"/>.
/// </para>
/// <para>
/// If your application no longer needs to watch for insertion or removal notifications,
/// you can call <see cref="YubiKeyDeviceListener.StopListening"/> to release resources
/// and avoid the logging and other actions from the listeners.
/// </para>
/// </remarks>
/// <param name="transport">
/// Argument controls which devices are searched for. Values <see cref="Transport.None"/>
Expand Down
84 changes: 71 additions & 13 deletions Yubico.YubiKey/src/Yubico/YubiKey/YubiKeyDeviceListener.cs
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,32 @@ public class YubiKeyDeviceListener : IDisposable
/// </summary>
public static YubiKeyDeviceListener Instance => _lazyInstance ??= new YubiKeyDeviceListener();

internal static bool IsListenerRunning => !(_lazyInstance is null);

/// <summary>
/// Disposes and closes the singleton instance of <see cref="YubiKeyDeviceListener"/>.
/// </summary>
public static void ResetInstance()
/// <remarks>
/// <para>
/// Enumerating YubiKeys is actually done via a cache. As such, this cache must be maintained
/// and kept up-to-date. This is done by starting several listeners that run in the background.
/// These listen for the relevant OS device arrival and removal events.
/// </para>
/// <para>
/// Normally, these background listeners will run starting with the first enumeration call to the
/// SDK and remain active until the process shuts down. But there are cases where you may not want
/// the overhead of these listeners running all the time. While they do their best to not consume
/// excessive resources, they can sometimes generate log noise, exceptions, etc.
/// </para>
/// <para>
/// This method allows you to stop these
/// background listeners and reclaim resources, as possible. This will not invalidate any existing
/// IYubiKeyDevice instances, however you will not receive any additional events regarding that device.
/// Any subsequent calls to <see cref="YubiKeyDevice.FindAll"/>, <see cref="YubiKeyDevice.FindByTransport"/>,
/// or <see cref="Instance"/> will restart the listeners.
/// </para>
/// </remarks>
public static void StopListening()
{
if (_lazyInstance != null)
{
Expand All @@ -63,7 +85,7 @@ public static void ResetInstance()

private readonly ReaderWriterLockSlim _rwLock = new ReaderWriterLockSlim(LockRecursionPolicy.NoRecursion);

private readonly Logger _log = Log.GetLogger<YubiKeyDeviceListener>();
private readonly ILogger _log = Log.GetLogger<YubiKeyDeviceListener>();
private readonly Dictionary<IYubiKeyDevice, bool> _internalCache = new Dictionary<IYubiKeyDevice, bool>();
private readonly HidDeviceListener _hidListener = HidDeviceListener.Create();
private readonly SmartCardDeviceListener _smartCardListener = SmartCardDeviceListener.Create();
Expand All @@ -77,7 +99,7 @@ public static void ResetInstance()

private YubiKeyDeviceListener()
{
_log.LogInformation("Creating YubiKeyDeviceListener instance.");
_log.LogInformation($"Creating {nameof(YubiKeyDeviceListener)} instance.");

SetupDeviceListeners();

Expand All @@ -89,14 +111,45 @@ private YubiKeyDeviceListener()

internal List<IYubiKeyDevice> GetAll() => _internalCache.Keys.ToList();

private void ListenerHandler(object? sender, EventArgs e) => _semaphore.Release();
private void ArriveHandler(object? sender, EventArgs e) => ListenerHandler("Arrival", e);

private void RemoveHandler(object? sender, EventArgs e) => ListenerHandler("Removal", e);

private void ListenerHandler(string eventType, EventArgs e)
{
object? device;
string deviceType;
if (e is SmartCardDeviceEventArgs se)
{
deviceType = "smart card";
device = se.Device;
}
else if (e is HidDeviceEventArgs he)
{
deviceType = "HID";
device = he.Device;
}
else
{
// Given this is a private method, this case isn't likely.
deviceType = "unknown";
device = "undefined";
}

_log.LogInformation(
"{EventType} of {DeviceType} {Device} is triggering update.",
eventType,
deviceType,
device);
_ = _semaphore.Release();
}

private void SetupDeviceListeners()
{
_smartCardListener.Arrived += ListenerHandler;
_smartCardListener.Removed += ListenerHandler;
_hidListener.Arrived += ListenerHandler;
_hidListener.Removed += ListenerHandler;
_smartCardListener.Arrived += ArriveHandler;
_smartCardListener.Removed += RemoveHandler;
_hidListener.Arrived += ArriveHandler;
_hidListener.Removed += RemoveHandler;
}

private async Task ListenForChanges()
Expand Down Expand Up @@ -368,28 +421,33 @@ protected virtual void Dispose(bool disposing)
_tokenSource.Cancel();

// Shut down the listener handlers.
_hidListener.Arrived -= ListenerHandler;
_hidListener.Removed -= ListenerHandler;
_hidListener.Arrived -= ArriveHandler;
_hidListener.Removed -= RemoveHandler;
if (_hidListener is IDisposable hidDisp)
{
hidDisp.Dispose();
}

_smartCardListener.Arrived -= ListenerHandler;
_smartCardListener.Removed -= ListenerHandler;
_smartCardListener.Arrived -= ArriveHandler;
_smartCardListener.Removed -= RemoveHandler;
if (_smartCardListener is IDisposable scDisp)
{
scDisp.Dispose();
}

// Give the listen thread a moment (likely is already done).
// Give the listen task a moment (likely is already done).
_ = !_listenTask.Wait(100);
_listenTask.Dispose();

// Get rid of synchronization objects.
_rwLock.Dispose();
_semaphore.Dispose();
_tokenSource.Dispose();

if (ReferenceEquals(_lazyInstance, this))
{
_lazyInstance = null;
}
}
_isDisposed = true;
}
Expand Down
24 changes: 24 additions & 0 deletions Yubico.YubiKey/tests/integration/Yubico/YubiKey/YubiKeyTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -221,5 +221,29 @@ public void GetYubiKeys_SingleTransport_RapidSwitching()
_testOutputHelper.WriteLine($"\t({keys.Count}) -{sw.ElapsedMilliseconds,5}ms");
}
}

[Fact]
public void TestResettingDeviceListener()
{
// Get devices (if any) and ensure the listeners are running.
List<IYubiKeyDevice> beforeDevices = YubiKeyDevice.FindAll().ToList();
_testOutputHelper.WriteLine($"Found {beforeDevices.Count} YubiKey devices before reset");

// Test that the listeners are running.
Assert.True(YubiKeyDeviceListener.IsListenerRunning, $"{nameof(YubiKeyDeviceListener.Instance)} is not active");

// Stop the listeners.
YubiKeyDeviceListener.StopListening();

// Test that we really stopped it.
Assert.False(YubiKeyDeviceListener.IsListenerRunning, $"{nameof(YubiKeyDeviceListener.Instance)} is still active");

// Make sure we can still enumerate devices.
List<IYubiKeyDevice> afterDevices = YubiKeyDevice.FindAll().ToList();
_testOutputHelper.WriteLine($"Found {afterDevices.Count} YubiKey devices after reset");

// Check that we have the same devices as the first check.
Assert.True(afterDevices.SequenceEqual(beforeDevices), "Before and after aren't the same.");
}
}
}

0 comments on commit 8c1230a

Please sign in to comment.