Skip to content

Release transport references in Dispose regardless of _shouldDispose#2586

Merged
krwq merged 2 commits into
mainfrom
copilot/fix-dispose-handling
Jul 2, 2026
Merged

Release transport references in Dispose regardless of _shouldDispose#2586
krwq merged 2 commits into
mainfrom
copilot/fix-dispose-handling

Conversation

Copilot AI commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Fixes: #1971
Device bindings placed the field = null/= null! assignment inside the if (_shouldDispose) guard in Dispose. When the transport is caller-owned (_shouldDispose == false), the binding never cleared its reference, keeping the object rooted longer than necessary.

// before
if (_shouldDispose)
{
    _i2cDevice?.Dispose();
    _i2cDevice = null!;   // skipped when caller owns the transport
}

// after
if (_shouldDispose)
{
    _i2cDevice?.Dispose();
}

_i2cDevice = null!;

Changes

  • Moved null assignments outside the _shouldDispose guard across 40 device bindings — the Dispose() call stays guarded (caller-owned transports are not disposed), but the reference is always cleared.
  • else-branch cases (GpioButton, Ccs811, Dhtxx, KeyMatrix, Pcd8544) — the branch closes pins on caller-owned controllers, so the null assignment is placed after the whole if/else block, past the last use of the field.

Notes

  • Scoped to the null-assignment bug. The autoDispose → shouldDispose rename raised in the discussion is already complete in the codebase; the broader "missing null assignment" cases and the analyzer idea (Code-gen shouldDispose ctor and Dispose pattern #1971) are left as follow-ups.

Copilot AI linked an issue Jul 1, 2026 that may be closed by this pull request
@dotnet-policy-service dotnet-policy-service Bot added the area-device-bindings Device Bindings for audio, sensor, motor, and display hardware that can used with System.Device.Gpio label Jul 1, 2026
Copilot AI changed the title [WIP] Fix dispose not handled correctly in device code Release transport references in Dispose regardless of _shouldDispose Jul 1, 2026
Copilot AI requested a review from raffaeler July 1, 2026 14:37
@raffaeler raffaeler marked this pull request as ready for review July 1, 2026 15:06
Copilot AI review requested due to automatic review settings July 1, 2026 15:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates multiple .NET IoT device bindings to always release (set to null/null!) transport/controller fields during Dispose, even when the transport is caller-owned (_shouldDispose == false), reducing unnecessary object rooting and improving GC behavior.

Changes:

  • Moved transport/controller field null-assignment outside the _shouldDispose guard across many bindings.
  • Adjusted if/else-dispose patterns (close pins when caller-owned) so null-assignment happens after the last field use.
  • Standardized the pattern across I2C/SPI/GPIO/serial-based bindings and a few shared helper types.

Reviewed changes

Copilot reviewed 40 out of 40 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/devices/Vl53L0X/Vl53L0X.cs Always clears _i2cDevice reference after guarded disposal.
src/devices/Uln2003/Uln2003.cs Always clears _controller reference after guarded disposal.
src/devices/Tcs3472x/Tcs3472x.cs Always clears _i2cDevice reference after guarded disposal.
src/devices/Tca955x/Tca955x.cs Clears _controller reference outside _shouldDispose guard (but Dispose(bool) needs a fix; see PR comment).
src/devices/Ssd1351/Ssd1351.cs Clears _gpioDevice reference outside _shouldDispose guard.
src/devices/SoftwareSpi/SoftwareSpi.cs Clears _gpioController reference outside _shouldDispose guard.
src/devices/SoftPwm/SoftwarePwmChannel.cs Clears _controller reference outside _shouldDispose guard.
src/devices/ShiftRegister/ShiftRegister.cs Clears _controller reference outside _shouldDispose guard.
src/devices/PiJuice/PiJuice.cs Always clears _i2cDevice reference after guarded disposal.
src/devices/Pcd8544/Pcd8544.cs Clears _controller reference after if/else that may close pins.
src/devices/Pca95x4/Pca95x4.cs Always clears _controller reference after guarded disposal.
src/devices/Nrf24l01/Nrf24l01.cs Always clears _gpio reference after guarded disposal.
src/devices/Mpu6xxx9xxx/Mpu9250.cs Always clears _i2cDevice reference after guarded disposal.
src/devices/Mhz19b/Mhz19b.cs Always clears _serialPortStream reference after guarded disposal.
src/devices/Mfrc522/Mfrc522.cs Always clears _controller reference after guarded disposal.
src/devices/Mcp25xxx/Mcp25xxx.cs Always clears _gpioController reference after guarded disposal.
src/devices/Mcp23xxx/Mcp23xxx.cs Always clears _controller and _bus references after guarded disposal.
src/devices/LiquidLevel/LiquidLevelSwitch.cs Always clears _controller reference after guarded disposal.
src/devices/LidarLiteV3/LidarLiteV3.cs Always clears _gpioController reference after guarded disposal.
src/devices/KeyMatrix/KeyMatrix.cs Clears _gpioController reference after if/else that may close pins.
src/devices/Ht1632/Ht1632.cs Always clears _controller reference after guarded disposal.
src/devices/Hcsr501/Hcsr501.cs Always clears _controller reference after guarded disposal.
src/devices/Hcsr04/Hcsr04.cs Always clears _controller reference after guarded disposal.
src/devices/GrovePi/GrovePi.cs Always clears _i2cDevice reference after guarded disposal.
src/devices/ExplorerHat/Motors.cs Always clears _controller reference after guarded disposal.
src/devices/ExplorerHat/Lights.cs Always clears _controller reference after guarded disposal.
src/devices/ExplorerHat/Led.cs Always clears _controller reference after guarded disposal.
src/devices/ExplorerHat/ExplorerHat.cs Always clears _controller reference after guarded disposal.
src/devices/Dhtxx/DhtBase.cs Clears _controller reference after if/else that may close pin.
src/devices/DCMotor/DCMotor.cs Always clears Controller reference after guarded disposal.
src/devices/Common/Iot/Device/Multiplexing/GpioOutputSegment.cs Always clears _controller reference after guarded disposal.
src/devices/Charlieplex/CharlieplexSegment.cs Always clears _gpioController reference after guarded disposal.
src/devices/CharacterLcd/LcdInterface.Gpio.cs Always clears _controller reference after guarded disposal.
src/devices/Ccs811/Ccs811Sensor.cs Clears _controller reference after if/else that may close pins.
src/devices/Button/GpioButton.cs Clears _gpioController reference after if/else that may close the pin.
src/devices/Bno055/Bno055Sensor.cs Always clears _i2cDevice reference after guarded disposal.
src/devices/Bmm150/Bmm150.cs Always clears _i2cDevice reference after guarded disposal.
src/devices/Blinkt/Blinkt.cs Always clears _gpioController reference after guarded disposal.
src/devices/Ak8963/Ak8963.cs Always clears _i2cDevice reference after guarded disposal.
src/devices/Ads1115/Ads1115.cs Always clears _gpioController reference after guarded disposal.

Comment on lines +662 to 666
_controller = null;

_busDevice?.Dispose();

base.Dispose(true);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot open a new PR to fix this

Copilot AI commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

@krwq I've opened a new pull request, #2587, to work on those changes. Once the pull request is ready, I'll request review from you.

@krwq krwq merged commit 04156d0 into main Jul 2, 2026
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-device-bindings Device Bindings for audio, sensor, motor, and display hardware that can used with System.Device.Gpio

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Code-gen shouldDispose ctor and Dispose pattern Dispose not handled correctly

4 participants