diff --git a/src/System.Device.Gpio.Tests/GpioControllerTestBase.cs b/src/System.Device.Gpio.Tests/GpioControllerTestBase.cs index c2fd94c396..d21b3b51de 100644 --- a/src/System.Device.Gpio.Tests/GpioControllerTestBase.cs +++ b/src/System.Device.Gpio.Tests/GpioControllerTestBase.cs @@ -595,6 +595,34 @@ void Callback(object sender, PinValueChangedEventArgs pinValueChangedEventArgs) } } + [Fact] + public void UsingPinAfterDriverDisposedCausesException() + { + var controller = new GpioController(PinNumberingScheme.Logical, GetTestDriver()); + var pin6 = controller.OpenPin(InputPin, PinMode.Input); + controller.Dispose(); + bool correctExceptionSeen = false; + try + { + pin6.Read(); + } + catch (ObjectDisposedException) + { + correctExceptionSeen = true; + } + + Assert.True(correctExceptionSeen); + } + + [Fact] + public void UsingControllerAfterDisposeCausesException() + { + var controller = new GpioController(PinNumberingScheme.Logical, GetTestDriver()); + var pin6 = controller.OpenPin(InputPin, PinMode.Input); + controller.Dispose(); + Assert.Throws(() => controller.OpenPin(InputPin, PinMode.Input)); + } + protected abstract GpioDriver GetTestDriver(); protected abstract PinNumberingScheme GetTestNumberingScheme(); } diff --git a/src/System.Device.Gpio.Tests/LibGpiodV1DriverTests.cs b/src/System.Device.Gpio.Tests/LibGpiodV1DriverTests.cs index 0cd8f4332f..a2ebf45c19 100644 --- a/src/System.Device.Gpio.Tests/LibGpiodV1DriverTests.cs +++ b/src/System.Device.Gpio.Tests/LibGpiodV1DriverTests.cs @@ -72,4 +72,36 @@ static void PinChanged(object sender, PinValueChangedEventArgs args) gc.UnregisterCallbackForPinValueChangedEvent(InputPin, PinChanged); } } + + /// + /// Ensure leaking instances of the driver doesn't cause a segfault + /// Regression test for https://github.com/dotnet/iot/issues/1849 + /// + [Fact] + public void LeakingDriverDoesNotCrash() + { + GpioController controller1 = new GpioController(PinNumberingScheme.Logical, new LibGpiodDriver()); + controller1.OpenPin(10, PinMode.Output); + GpioController controller2 = new GpioController(PinNumberingScheme.Logical, new LibGpiodDriver()); + controller2.OpenPin(11, PinMode.Output); + GpioController controller3 = new GpioController(PinNumberingScheme.Logical, new LibGpiodDriver()); + controller3.OpenPin(12, PinMode.Output); + GpioController controller4 = new GpioController(PinNumberingScheme.Logical, new LibGpiodDriver()); + controller4.OpenPin(13, PinMode.Output); + GpioController controller5 = new GpioController(PinNumberingScheme.Logical, new LibGpiodDriver()); + controller5.OpenPin(14, PinMode.Output); + + for (int i = 0; i < 10; i++) + { + GC.Collect(); + GpioController controller6 = new GpioController(PinNumberingScheme.Logical, new LibGpiodDriver()); + controller6.OpenPin(15, PinMode.Output); + controller6.ClosePin(15); + controller6.Dispose(); + GC.Collect(); + Thread.Sleep(20); + } + + GC.WaitForPendingFinalizers(); + } } diff --git a/src/System.Device.Gpio/CompatibilitySuppressions.xml b/src/System.Device.Gpio/CompatibilitySuppressions.xml index 75f2976ac2..1fc650add8 100644 --- a/src/System.Device.Gpio/CompatibilitySuppressions.xml +++ b/src/System.Device.Gpio/CompatibilitySuppressions.xml @@ -13,6 +13,12 @@ lib/netstandard2.0/System.Device.Gpio.dll lib/net6.0-windows10.0.17763/System.Device.Gpio.dll + + CP0002 + M:System.Device.Gpio.GpioDriver.Finalize + lib/netstandard2.0/System.Device.Gpio.dll + lib/net6.0-windows10.0.17763/System.Device.Gpio.dll + CP0002 M:System.Device.Gpio.PinEventTypes.#ctor diff --git a/src/System.Device.Gpio/Interop/Unix/libgpiod/V1/Interop.libgpiod.cs b/src/System.Device.Gpio/Interop/Unix/libgpiod/V1/Interop.libgpiod.cs index fa80c71e95..5c40bcca4b 100644 --- a/src/System.Device.Gpio/Interop/Unix/libgpiod/V1/Interop.libgpiod.cs +++ b/src/System.Device.Gpio/Interop/Unix/libgpiod/V1/Interop.libgpiod.cs @@ -81,7 +81,7 @@ static LibgpiodV1() /// The offset of the GPIO line /// Handle to the GPIO line or if an error occurred. [DllImport(LibgpiodLibrary, SetLastError = true)] - internal static extern SafeLineHandle gpiod_chip_get_line(SafeChipHandle chip, int offset); + internal static extern IntPtr gpiod_chip_get_line(SafeChipHandle chip, int offset); /// /// Reserve a single line, set the direction to input. @@ -90,7 +90,7 @@ static LibgpiodV1() /// Name of the consumer. /// 0 if the line was properly reserved, -1 on failure. [DllImport(LibgpiodLibrary, SetLastError = true)] - internal static extern int gpiod_line_request_input(SafeLineHandle line, string consumer); + internal static extern int gpiod_line_request_input(IntPtr line, string consumer); /// /// Reserve a single line, set the direction to input with flags @@ -100,7 +100,7 @@ static LibgpiodV1() /// Additional request flags. /// 0 if the line was properly reserved, -1 on failure. [DllImport(LibgpiodLibrary, SetLastError = true)] - internal static extern int gpiod_line_request_input_flags(SafeLineHandle line, string consumer, int flags); + internal static extern int gpiod_line_request_input_flags(IntPtr line, string consumer, int flags); /// /// Reserve a single line, set the direction to output. @@ -110,7 +110,7 @@ static LibgpiodV1() /// Initial value of the line /// 0 if the line was properly reserved, -1 on failure. [DllImport(LibgpiodLibrary, SetLastError = true)] - internal static extern int gpiod_line_request_output(SafeLineHandle line, string consumer, int default_val); + internal static extern int gpiod_line_request_output(IntPtr line, string consumer, int default_val); /// /// Set the value of a single GPIO line. @@ -119,7 +119,7 @@ static LibgpiodV1() /// New value. /// 0 if the operation succeeds. In case of an error this routine returns -1 and sets the last error number. [DllImport(LibgpiodLibrary)] - internal static extern int gpiod_line_set_value(SafeLineHandle line, int value); + internal static extern int gpiod_line_set_value(IntPtr line, int value); /// /// Read current value of a single GPIO line. @@ -127,7 +127,7 @@ static LibgpiodV1() /// GPIO line handle /// 0 or 1 if the operation succeeds. On error this routine returns -1 and sets the last error number. [DllImport(LibgpiodLibrary, SetLastError = true)] - internal static extern int gpiod_line_get_value(SafeLineHandle line); + internal static extern int gpiod_line_get_value(IntPtr line); /// /// Check if line is no used (not set as Input or Output, not listening events). @@ -135,7 +135,7 @@ static LibgpiodV1() /// GPIO line handle /// false if pin is used as Input/Output or Listening an event, true if it is free [DllImport(LibgpiodLibrary)] - internal static extern bool gpiod_line_is_free(SafeLineHandle line); + internal static extern bool gpiod_line_is_free(IntPtr line); /// /// Release a previously reserved line. @@ -153,7 +153,7 @@ static LibgpiodV1() /// GPIO line handle /// 1 for input, 2 for output [DllImport(LibgpiodLibrary)] - internal static extern int gpiod_line_direction(SafeLineHandle lineHandle); + internal static extern int gpiod_line_direction(IntPtr lineHandle); /// /// Read the GPIO line bias setting. @@ -161,7 +161,7 @@ static LibgpiodV1() /// GPIO line handle /// GPIOD_LINE_BIAS_PULL_UP (3), GPIOD_LINE_BIAS_PULL_DOWN (4), GPIOD_LINE_BIAS_DISABLE (2) or GPIOD_LINE_BIAS_UNKNOWN (1). [DllImport(LibgpiodLibrary)] - internal static extern int gpiod_line_bias(SafeLineHandle lineHandle); + internal static extern int gpiod_line_bias(IntPtr lineHandle); /// /// Request all event type notifications on a single line. @@ -170,7 +170,7 @@ static LibgpiodV1() /// Name of the consumer. /// 0 the operation succeeds, -1 on failure. [DllImport(LibgpiodLibrary, SetLastError = true)] - internal static extern int gpiod_line_request_both_edges_events(SafeLineHandle line, string consumer); + internal static extern int gpiod_line_request_both_edges_events(IntPtr line, string consumer); /// /// Wait for an event on a single line. @@ -179,7 +179,7 @@ static LibgpiodV1() /// The TimeSpec to wait for before timing out /// 0 if wait timed out, -1 if an error occurred, 1 if an event occurred. [DllImport(LibgpiodLibrary, SetLastError = true)] - internal static extern WaitEventResult gpiod_line_event_wait(SafeLineHandle line, ref TimeSpec timeout); + internal static extern WaitEventResult gpiod_line_event_wait(IntPtr line, ref TimeSpec timeout); /// /// Read the last event from the GPIO line. @@ -188,7 +188,7 @@ static LibgpiodV1() /// Reference to the gpio event that was detected /// 1 if rising edge event occurred, 2 on falling edge, -1 on error. [DllImport(LibgpiodLibrary, SetLastError = true)] - internal static extern int gpiod_line_event_read(SafeLineHandle line, ref GpioLineEvent gpioEvent); + internal static extern int gpiod_line_event_read(IntPtr line, ref GpioLineEvent gpioEvent); /// /// Open a gpiochip by number. diff --git a/src/System.Device.Gpio/Interop/Unix/libgpiod/V1/LineHandle.cs b/src/System.Device.Gpio/Interop/Unix/libgpiod/V1/LineHandle.cs new file mode 100644 index 0000000000..1b606fdad6 --- /dev/null +++ b/src/System.Device.Gpio/Interop/Unix/libgpiod/V1/LineHandle.cs @@ -0,0 +1,59 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Runtime.InteropServices; +using LibgpiodV1 = Interop.LibgpiodV1; + +namespace System.Device.Gpio.Libgpiod.V1; + +/// +/// Pointer to a pin (Not a real SafeLineHandle, because we need to align its finalization with the owning object) +/// +internal sealed class LineHandle : IDisposable +{ + private IntPtr _handle; + public LineHandle(IntPtr handle) + { + _handle = handle; + PinMode = PinMode.Input; + } + + public PinMode PinMode { get; set; } + + public IntPtr Handle + { + get + { + if (_handle == IntPtr.Zero) + { + throw new ObjectDisposedException(nameof(LineHandle)); + } + + return _handle; + } + set + { + _handle = value; + } + } + + /// + /// Release the lock on the line handle. + /// + public void ReleaseLock() + { + // Contrary to intuition, this does not invalidate the handle (see comment on declaration) + Interop.LibgpiodV1.gpiod_line_release(_handle); + } + + public bool IsInvalid => _handle == IntPtr.Zero || _handle == Interop.LibgpiodV1.InvalidHandleValue; + + public void Dispose() + { + if (_handle != IntPtr.Zero) + { + Interop.LibgpiodV1.gpiod_line_release(_handle); + _handle = IntPtr.Zero; + } + } +} diff --git a/src/System.Device.Gpio/Interop/Unix/libgpiod/V1/SafeLineHandle.cs b/src/System.Device.Gpio/Interop/Unix/libgpiod/V1/SafeLineHandle.cs deleted file mode 100644 index be7172fa31..0000000000 --- a/src/System.Device.Gpio/Interop/Unix/libgpiod/V1/SafeLineHandle.cs +++ /dev/null @@ -1,37 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Runtime.InteropServices; -using LibgpiodV1 = Interop.LibgpiodV1; - -namespace System.Device.Gpio.Libgpiod.V1; - -/// -/// Pointer to a pin. -/// -internal class SafeLineHandle : SafeHandle -{ - public PinMode PinMode { get; set; } - - public SafeLineHandle() - : base(IntPtr.Zero, true) - { - } - - protected override bool ReleaseHandle() - { - // Contrary to intuition, this does not invalidate the handle (see comment on declaration) - LibgpiodV1.gpiod_line_release(handle); - return true; - } - - /// - /// Release the lock on the line handle. - /// - public void ReleaseLock() - { - ReleaseHandle(); - } - - public override bool IsInvalid => handle == IntPtr.Zero || handle == LibgpiodV1.InvalidHandleValue; -} diff --git a/src/System.Device.Gpio/System/Device/Gpio/Drivers/Libgpiod/LibGpiodDriver.cs b/src/System.Device.Gpio/System/Device/Gpio/Drivers/Libgpiod/LibGpiodDriver.cs index d9b5c1beb9..d36ebc6209 100644 --- a/src/System.Device.Gpio/System/Device/Gpio/Drivers/Libgpiod/LibGpiodDriver.cs +++ b/src/System.Device.Gpio/System/Device/Gpio/Drivers/Libgpiod/LibGpiodDriver.cs @@ -11,7 +11,7 @@ namespace System.Device.Gpio.Drivers; /// public class LibGpiodDriver : UnixDriver { - private readonly GpioDriver _driver; + private GpioDriver _driver; /// /// Creates an instance of the LibGpiodDriver @@ -55,96 +55,114 @@ public LibGpiodDriver(int gpioChip, LibGpiodDriverVersion driverVersion) public static LibGpiodDriverVersion[] GetAvailableVersions() => LibGpiodDriverFactory.Instance.DriverCandidates; /// - protected internal override int PinCount => _driver.PinCount; + protected internal override int PinCount => GetDriver().PinCount; /// protected internal override void AddCallbackForPinValueChangedEvent(int pinNumber, PinEventTypes eventTypes, PinChangeEventHandler callback) { - _driver.AddCallbackForPinValueChangedEvent(pinNumber, eventTypes, callback); + GetDriver().AddCallbackForPinValueChangedEvent(pinNumber, eventTypes, callback); } /// protected internal override void ClosePin(int pinNumber) { - _driver.ClosePin(pinNumber); + GetDriver().ClosePin(pinNumber); } /// protected internal override int ConvertPinNumberToLogicalNumberingScheme(int pinNumber) { - return _driver.ConvertPinNumberToLogicalNumberingScheme(pinNumber); + return GetDriver().ConvertPinNumberToLogicalNumberingScheme(pinNumber); } /// protected internal override PinMode GetPinMode(int pinNumber) { - return _driver.GetPinMode(pinNumber); + return GetDriver().GetPinMode(pinNumber); } /// protected internal override bool IsPinModeSupported(int pinNumber, PinMode mode) { - return _driver.IsPinModeSupported(pinNumber, mode); + return GetDriver().IsPinModeSupported(pinNumber, mode); } /// protected internal override void OpenPin(int pinNumber) { - _driver.OpenPin(pinNumber); + GetDriver().OpenPin(pinNumber); } /// protected internal override PinValue Read(int pinNumber) { - return _driver.Read(pinNumber); + return GetDriver().Read(pinNumber); } /// protected internal override void Toggle(int pinNumber) { - _driver.Toggle(pinNumber); + GetDriver().Toggle(pinNumber); } /// protected internal override void RemoveCallbackForPinValueChangedEvent(int pinNumber, PinChangeEventHandler callback) { - _driver.RemoveCallbackForPinValueChangedEvent(pinNumber, callback); + GetDriver().RemoveCallbackForPinValueChangedEvent(pinNumber, callback); } /// protected internal override void SetPinMode(int pinNumber, PinMode mode) { - _driver.SetPinMode(pinNumber, mode); + GetDriver().SetPinMode(pinNumber, mode); } /// protected internal override void SetPinMode(int pinNumber, PinMode mode, PinValue initialValue) { - _driver.SetPinMode(pinNumber, mode, initialValue); + GetDriver().SetPinMode(pinNumber, mode, initialValue); } /// protected internal override WaitForEventResult WaitForEvent(int pinNumber, PinEventTypes eventTypes, CancellationToken cancellationToken) { - return _driver.WaitForEvent(pinNumber, eventTypes, cancellationToken); + return GetDriver().WaitForEvent(pinNumber, eventTypes, cancellationToken); } /// protected internal override void Write(int pinNumber, PinValue value) { - _driver.Write(pinNumber, value); + GetDriver().Write(pinNumber, value); } /// public override ComponentInformation QueryComponentInformation() { - return _driver.QueryComponentInformation(); + var ret = new ComponentInformation(this, "Libgpiod Wrapper driver"); + ret.Properties.Add("Version", Version.ToString()); + ret.AddSubComponent(GetDriver().QueryComponentInformation()); + return ret; + } + + private GpioDriver GetDriver() + { + if (_driver == null) + { + throw new ObjectDisposedException(nameof(LibGpiodDriver)); + } + + return _driver; } /// protected override void Dispose(bool disposing) { - _driver.Dispose(); + if (_driver != null && disposing) + { + _driver.Dispose(); + _driver = null!; + } + base.Dispose(disposing); } } diff --git a/src/System.Device.Gpio/System/Device/Gpio/Drivers/Libgpiod/V1/LibGpiodV1Driver.cs b/src/System.Device.Gpio/System/Device/Gpio/Drivers/Libgpiod/V1/LibGpiodV1Driver.cs index 926c4d686d..6d950d3ce5 100644 --- a/src/System.Device.Gpio/System/Device/Gpio/Drivers/Libgpiod/V1/LibGpiodV1Driver.cs +++ b/src/System.Device.Gpio/System/Device/Gpio/Drivers/Libgpiod/V1/LibGpiodV1Driver.cs @@ -20,7 +20,7 @@ internal class LibGpiodV1Driver : UnixDriver { private static string s_consumerName = Process.GetCurrentProcess().ProcessName; private readonly object _pinNumberLock; - private readonly ConcurrentDictionary _pinNumberToSafeLineHandle; + private readonly ConcurrentDictionary _pinNumberToSafeLineHandle; private readonly ConcurrentDictionary _pinNumberToEventHandler; private readonly int _pinCount; private readonly ConcurrentDictionary _pinValue; @@ -78,7 +78,7 @@ public LibGpiodV1Driver(int gpioChip = 0) _pinCount = LibgpiodV1.gpiod_chip_num_lines(_chip); _pinNumberToEventHandler = new ConcurrentDictionary(); - _pinNumberToSafeLineHandle = new ConcurrentDictionary(); + _pinNumberToSafeLineHandle = new ConcurrentDictionary(); _pinValue = new ConcurrentDictionary(); } catch (DllNotFoundException) @@ -114,12 +114,12 @@ private LibGpiodV1DriverEventHandler PopulateEventHandler(int pinNumber) { lock (_pinNumberLock) { - _pinNumberToSafeLineHandle.TryGetValue(pinNumber, out SafeLineHandle? pinHandle); + _pinNumberToSafeLineHandle.TryGetValue(pinNumber, out LineHandle? pinHandle); - if (pinHandle is null || (pinHandle is object && !LibgpiodV1.gpiod_line_is_free(pinHandle))) + if (pinHandle is null || (pinHandle is object && !LibgpiodV1.gpiod_line_is_free(pinHandle.Handle))) { pinHandle?.Dispose(); - pinHandle = LibgpiodV1.gpiod_chip_get_line(_chip, pinNumber); + pinHandle = new LineHandle(LibgpiodV1.gpiod_chip_get_line(_chip, pinNumber)); _pinNumberToSafeLineHandle[pinNumber] = pinHandle; } @@ -132,7 +132,7 @@ protected internal override void ClosePin(int pinNumber) { lock (_pinNumberLock) { - if (_pinNumberToSafeLineHandle.TryGetValue(pinNumber, out SafeLineHandle? pinHandle) && + if (_pinNumberToSafeLineHandle.TryGetValue(pinNumber, out LineHandle? pinHandle) && !IsListeningEvent(pinNumber)) { pinHandle?.Dispose(); @@ -157,7 +157,7 @@ protected internal override PinMode GetPinMode(int pinNumber) { lock (_pinNumberLock) { - if (!_pinNumberToSafeLineHandle.TryGetValue(pinNumber, out SafeLineHandle? pinHandle)) + if (!_pinNumberToSafeLineHandle.TryGetValue(pinNumber, out LineHandle? pinHandle)) { throw ExceptionHelper.GetInvalidOperationException(ExceptionResource.PinNotOpenedError, pin: pinNumber); @@ -185,13 +185,13 @@ protected internal override void OpenPin(int pinNumber) return; } - SafeLineHandle pinHandle = LibgpiodV1.gpiod_chip_get_line(_chip, pinNumber); + LineHandle pinHandle = new LineHandle(LibgpiodV1.gpiod_chip_get_line(_chip, pinNumber)); if (pinHandle == null) { throw ExceptionHelper.GetIOException(ExceptionResource.OpenPinError, Marshal.GetLastWin32Error()); } - int mode = LibgpiodV1.gpiod_line_direction(pinHandle); + int mode = LibgpiodV1.gpiod_line_direction(pinHandle.Handle); if (mode == 1) { pinHandle.PinMode = PinMode.Input; @@ -203,7 +203,7 @@ protected internal override void OpenPin(int pinNumber) if (s_isLibgpiodVersion1_5orHigher && pinHandle.PinMode == PinMode.Input) { - int bias = LibgpiodV1.gpiod_line_bias(pinHandle); + int bias = LibgpiodV1.gpiod_line_bias(pinHandle.Handle); if (bias == (int)RequestFlag.GPIOD_LINE_REQUEST_FLAG_BIAS_PULL_DOWN) { pinHandle.PinMode = PinMode.InputPullDown; @@ -225,9 +225,9 @@ protected internal override void OpenPin(int pinNumber) /// protected internal override PinValue Read(int pinNumber) { - if (_pinNumberToSafeLineHandle.TryGetValue(pinNumber, out SafeLineHandle? pinHandle)) + if (_pinNumberToSafeLineHandle.TryGetValue(pinNumber, out LineHandle? pinHandle)) { - int result = LibgpiodV1.gpiod_line_get_value(pinHandle); + int result = LibgpiodV1.gpiod_line_get_value(pinHandle.Handle); if (result == -1) { throw ExceptionHelper.GetIOException(ExceptionResource.ReadPinError, Marshal.GetLastWin32Error(), pinNumber); @@ -265,19 +265,19 @@ protected internal override void RemoveCallbackForPinValueChangedEvent(int pinNu /// protected internal override void SetPinMode(int pinNumber, PinMode mode) { - if (_pinNumberToSafeLineHandle.TryGetValue(pinNumber, out SafeLineHandle? pinHandle)) + if (_pinNumberToSafeLineHandle.TryGetValue(pinNumber, out LineHandle? pinHandle)) { // This call does not release the handle. It only releases the lock on the handle. Without this, changing the direction of a line is not possible. // Line handles cannot be freed and are cached until the chip is closed. pinHandle.ReleaseLock(); int requestResult = mode switch { - PinMode.Input => LibgpiodV1.gpiod_line_request_input(pinHandle, s_consumerName), - PinMode.InputPullDown => LibgpiodV1.gpiod_line_request_input_flags(pinHandle, s_consumerName, + PinMode.Input => LibgpiodV1.gpiod_line_request_input(pinHandle.Handle, s_consumerName), + PinMode.InputPullDown => LibgpiodV1.gpiod_line_request_input_flags(pinHandle.Handle, s_consumerName, (int)RequestFlag.GPIOD_LINE_REQUEST_FLAG_BIAS_PULL_DOWN), - PinMode.InputPullUp => LibgpiodV1.gpiod_line_request_input_flags(pinHandle, s_consumerName, + PinMode.InputPullUp => LibgpiodV1.gpiod_line_request_input_flags(pinHandle.Handle, s_consumerName, (int)RequestFlag.GPIOD_LINE_REQUEST_FLAG_BIAS_PULL_UP), - PinMode.Output => LibgpiodV1.gpiod_line_request_output(pinHandle, s_consumerName, 0), + PinMode.Output => LibgpiodV1.gpiod_line_request_output(pinHandle.Handle, s_consumerName, 0), _ => -1, }; @@ -297,19 +297,19 @@ protected internal override void SetPinMode(int pinNumber, PinMode mode) /// protected internal override void SetPinMode(int pinNumber, PinMode mode, PinValue initialValue) { - if (_pinNumberToSafeLineHandle.TryGetValue(pinNumber, out SafeLineHandle? pinHandle)) + if (_pinNumberToSafeLineHandle.TryGetValue(pinNumber, out LineHandle? pinHandle)) { // This call does not release the handle. It only releases the lock on the handle. Without this, changing the direction of a line is not possible. // Line handles cannot be freed and are cached until the chip is closed. pinHandle.ReleaseLock(); int requestResult = mode switch { - PinMode.Input => LibgpiodV1.gpiod_line_request_input(pinHandle, s_consumerName), - PinMode.InputPullDown => LibgpiodV1.gpiod_line_request_input_flags(pinHandle, s_consumerName, + PinMode.Input => LibgpiodV1.gpiod_line_request_input(pinHandle.Handle, s_consumerName), + PinMode.InputPullDown => LibgpiodV1.gpiod_line_request_input_flags(pinHandle.Handle, s_consumerName, (int)RequestFlag.GPIOD_LINE_REQUEST_FLAG_BIAS_PULL_DOWN), - PinMode.InputPullUp => LibgpiodV1.gpiod_line_request_input_flags(pinHandle, s_consumerName, + PinMode.InputPullUp => LibgpiodV1.gpiod_line_request_input_flags(pinHandle.Handle, s_consumerName, (int)RequestFlag.GPIOD_LINE_REQUEST_FLAG_BIAS_PULL_UP), - PinMode.Output => LibgpiodV1.gpiod_line_request_output(pinHandle, s_consumerName, initialValue == PinValue.High ? 1 : 0), + PinMode.Output => LibgpiodV1.gpiod_line_request_output(pinHandle.Handle, s_consumerName, initialValue == PinValue.High ? 1 : 0), _ => -1, }; @@ -378,13 +378,13 @@ private void WaitForEventResult(CancellationToken sourceToken, CancellationToken /// protected internal override void Write(int pinNumber, PinValue value) { - if (!_pinNumberToSafeLineHandle.TryGetValue(pinNumber, out SafeLineHandle? pinHandle)) + if (!_pinNumberToSafeLineHandle.TryGetValue(pinNumber, out LineHandle? pinHandle)) { throw ExceptionHelper.GetInvalidOperationException(ExceptionResource.PinNotOpenedError, pin: pinNumber); } - LibgpiodV1.gpiod_line_set_value(pinHandle, (value == PinValue.High) ? 1 : 0); + LibgpiodV1.gpiod_line_set_value(pinHandle.Handle, (value == PinValue.High) ? 1 : 0); _pinValue[pinNumber] = value; } @@ -416,7 +416,7 @@ protected override void Dispose(bool disposing) { foreach (int pin in _pinNumberToSafeLineHandle.Keys) { - if (_pinNumberToSafeLineHandle.TryGetValue(pin, out SafeLineHandle? pinHandle)) + if (_pinNumberToSafeLineHandle.TryGetValue(pin, out LineHandle? pinHandle)) { pinHandle?.Dispose(); } diff --git a/src/System.Device.Gpio/System/Device/Gpio/Drivers/Libgpiod/V1/LibGpiodV1DriverEventHandler.cs b/src/System.Device.Gpio/System/Device/Gpio/Drivers/Libgpiod/V1/LibGpiodV1DriverEventHandler.cs index 4f5a3aa0f3..c592389558 100644 --- a/src/System.Device.Gpio/System/Device/Gpio/Drivers/Libgpiod/V1/LibGpiodV1DriverEventHandler.cs +++ b/src/System.Device.Gpio/System/Device/Gpio/Drivers/Libgpiod/V1/LibGpiodV1DriverEventHandler.cs @@ -24,7 +24,7 @@ internal sealed class LibGpiodV1DriverEventHandler : IDisposable private readonly Task _task; private bool _disposing; - public LibGpiodV1DriverEventHandler(int pinNumber, SafeLineHandle safeLineHandle) + public LibGpiodV1DriverEventHandler(int pinNumber, LineHandle safeLineHandle) { _pinNumber = pinNumber; _cancellationTokenSource = new CancellationTokenSource(); @@ -34,9 +34,9 @@ public LibGpiodV1DriverEventHandler(int pinNumber, SafeLineHandle safeLineHandle public CancellationToken CancellationToken => _cancellationTokenSource.Token; - private void SubscribeForEvent(SafeLineHandle pinHandle) + private void SubscribeForEvent(LineHandle pinHandle) { - int eventSuccess = LibgpiodV1.gpiod_line_request_both_edges_events(pinHandle, s_consumerName); + int eventSuccess = LibgpiodV1.gpiod_line_request_both_edges_events(pinHandle.Handle, s_consumerName); if (eventSuccess < 0) { @@ -44,7 +44,7 @@ private void SubscribeForEvent(SafeLineHandle pinHandle) } } - private Task InitializeEventDetectionTask(CancellationToken token, SafeLineHandle pinHandle) + private Task InitializeEventDetectionTask(CancellationToken token, LineHandle pinHandle) { return Task.Run(() => { @@ -57,7 +57,7 @@ private Task InitializeEventDetectionTask(CancellationToken token, SafeLineHandl TvNsec = new IntPtr(50_000_000) }; - WaitEventResult waitResult = LibgpiodV1.gpiod_line_event_wait(pinHandle, ref timeout); + WaitEventResult waitResult = LibgpiodV1.gpiod_line_event_wait(pinHandle.Handle, ref timeout); if (waitResult == WaitEventResult.Error) { var errorCode = Marshal.GetLastWin32Error(); @@ -73,7 +73,7 @@ private Task InitializeEventDetectionTask(CancellationToken token, SafeLineHandl if (waitResult == WaitEventResult.EventOccured) { GpioLineEvent eventResult = new GpioLineEvent(); - int checkForEvent = LibgpiodV1.gpiod_line_event_read(pinHandle, ref eventResult); + int checkForEvent = LibgpiodV1.gpiod_line_event_read(pinHandle.Handle, ref eventResult); if (checkForEvent == -1) { throw ExceptionHelper.GetIOException(ExceptionResource.EventReadError, Marshal.GetLastWin32Error()); diff --git a/src/System.Device.Gpio/System/Device/Gpio/Drivers/SysFsDriver.cs b/src/System.Device.Gpio/System/Device/Gpio/Drivers/SysFsDriver.cs index 5bf0d062d7..af6209a016 100644 --- a/src/System.Device.Gpio/System/Device/Gpio/Drivers/SysFsDriver.cs +++ b/src/System.Device.Gpio/System/Device/Gpio/Drivers/SysFsDriver.cs @@ -35,6 +35,8 @@ public class SysFsDriver : UnixDriver private int _pinsToDetectEventsCount; private CancellationTokenSource? _eventThreadCancellationTokenSource; + private bool _isDisposed; + private static int ReadOffset() { IEnumerable fileNames = Directory.EnumerateFileSystemEntries(GpioBasePath); @@ -72,6 +74,8 @@ public SysFsDriver() { throw new PlatformNotSupportedException($"{GetType().Name} is only supported on Linux/Unix."); } + + _isDisposed = false; } /// @@ -108,6 +112,7 @@ internal TimeSpan StatusUpdateSleepTime /// The pin number in the driver's logical numbering scheme. protected internal override void OpenPin(int pinNumber) { + CheckValidDriver(); int pinOffset = pinNumber + s_pinOffset; string pinPath = $"{GpioBasePath}/gpio{pinOffset}"; // If the directory exists, this becomes a no-op since the pin might have been opened already by the some controller or somebody else. @@ -136,6 +141,7 @@ protected internal override void OpenPin(int pinNumber) /// The pin number in the driver's logical numbering scheme. protected internal override void ClosePin(int pinNumber) { + CheckValidDriver(); int pinOffset = pinNumber + s_pinOffset; string pinPath = $"{GpioBasePath}/gpio{pinOffset}"; // If the directory doesn't exist, this becomes a no-op since the pin was closed already. @@ -171,6 +177,7 @@ protected internal override void ClosePin(int pinNumber) /// The mode to be set. protected internal override void SetPinMode(int pinNumber, PinMode mode) { + CheckValidDriver(); if (mode == PinMode.InputPullDown || mode == PinMode.InputPullUp) { throw new PlatformNotSupportedException("This driver is generic so it does not support Input Pull Down or Input Pull Up modes."); @@ -233,6 +240,7 @@ private PinMode ConvertSysFsModeToPinMode(string sysFsMode) /// The value of the pin. protected internal override PinValue Read(int pinNumber) { + CheckValidDriver(); PinValue result = default; string valuePath = $"{GpioBasePath}/gpio{pinNumber + s_pinOffset}/value"; if (File.Exists(valuePath)) @@ -276,6 +284,7 @@ private PinValue ConvertSysFsValueToPinValue(string value) /// The value to be written to the pin. protected internal override void Write(int pinNumber, PinValue value) { + CheckValidDriver(); string valuePath = $"{GpioBasePath}/gpio{pinNumber + s_pinOffset}/value"; if (File.Exists(valuePath)) { @@ -307,6 +316,7 @@ private string ConvertPinValueToSysFs(PinValue value) /// The status if the pin supports the mode. protected internal override bool IsPinModeSupported(int pinNumber, PinMode mode) { + CheckValidDriver(); // Unix driver does not support pull up or pull down resistors. if (mode == PinMode.InputPullDown || mode == PinMode.InputPullUp) { @@ -325,6 +335,7 @@ protected internal override bool IsPinModeSupported(int pinNumber, PinMode mode) /// A structure that contains the result of the waiting operation. protected internal override WaitForEventResult WaitForEvent(int pinNumber, PinEventTypes eventTypes, CancellationToken cancellationToken) { + CheckValidDriver(); int pollFileDescriptor = -1; int valueFileDescriptor = -1; SetPinEventsToDetect(pinNumber, eventTypes); @@ -590,9 +601,18 @@ protected override void Dispose(bool disposing) ClosePin(_exportedPins.FirstOrDefault()); } + _isDisposed = true; base.Dispose(disposing); } + private void CheckValidDriver() + { + if (_isDisposed) + { + throw new ObjectDisposedException(nameof(SysFsDriver)); + } + } + /// /// Adds a handler for a pin value changed event. /// @@ -601,6 +621,7 @@ protected override void Dispose(bool disposing) /// Delegate that defines the structure for callbacks when a pin value changed event occurs. protected internal override void AddCallbackForPinValueChangedEvent(int pinNumber, PinEventTypes eventTypes, PinChangeEventHandler callback) { + CheckValidDriver(); if (!_devicePins.ContainsKey(pinNumber)) { _devicePins.Add(pinNumber, new UnixDriverDevicePin(Read(pinNumber))); @@ -722,6 +743,7 @@ private void DetectEvents() /// Delegate that defines the structure for callbacks when a pin value changed event occurs. protected internal override void RemoveCallbackForPinValueChangedEvent(int pinNumber, PinChangeEventHandler callback) { + CheckValidDriver(); if (!_devicePins.ContainsKey(pinNumber)) { throw new InvalidOperationException("Attempted to remove a callback for a pin that is not listening for events."); @@ -747,6 +769,7 @@ protected internal override void RemoveCallbackForPinValueChangedEvent(int pinNu /// The mode of the pin. protected internal override PinMode GetPinMode(int pinNumber) { + CheckValidDriver(); pinNumber += s_pinOffset; string directionPath = $"{GpioBasePath}/gpio{pinNumber}/direction"; if (File.Exists(directionPath)) diff --git a/src/System.Device.Gpio/System/Device/Gpio/GpioController.cs b/src/System.Device.Gpio/System/Device/Gpio/GpioController.cs index fd1ee6a960..985b41f72e 100644 --- a/src/System.Device.Gpio/System/Device/Gpio/GpioController.cs +++ b/src/System.Device.Gpio/System/Device/Gpio/GpioController.cs @@ -75,7 +75,14 @@ public GpioController(PinNumberingScheme numberingScheme) /// /// The number of pins provided by the controller. /// - public virtual int PinCount => _driver.PinCount; + public virtual int PinCount + { + get + { + CheckDriverValid(); + return _driver.PinCount; + } + } /// /// Returns the collection of open pins @@ -233,9 +240,18 @@ public virtual PinMode GetPinMode(int pinNumber) /// The status if the pin is open or closed. public bool IsPinOpen(int pinNumber) { + CheckDriverValid(); return _openPins.ContainsKey(pinNumber); } + private void CheckDriverValid() + { + if (_driver == null) + { + throw new ObjectDisposedException(nameof(GpioController)); + } + } + /// /// Checks if a pin supports a specific mode. /// @@ -244,6 +260,7 @@ public bool IsPinOpen(int pinNumber) /// The status if the pin supports the mode. public virtual bool IsPinModeSupported(int pinNumber, PinMode mode) { + CheckDriverValid(); int logicalPinNumber = GetLogicalPinNumber(pinNumber); return _driver.IsPinModeSupported(logicalPinNumber, mode); } diff --git a/src/System.Device.Gpio/System/Device/Gpio/GpioDriver.cs b/src/System.Device.Gpio/System/Device/Gpio/GpioDriver.cs index f4abedef9c..2c9aee12c0 100644 --- a/src/System.Device.Gpio/System/Device/Gpio/GpioDriver.cs +++ b/src/System.Device.Gpio/System/Device/Gpio/GpioDriver.cs @@ -12,6 +12,14 @@ namespace System.Device.Gpio; /// public abstract class GpioDriver : IDisposable { + /// + /// Finalizer to clean up unmanaged resources + /// + ~GpioDriver() + { + Dispose(false); + } + /// /// The number of pins provided by the driver. ///