From 82293382687d64ca44bbe78acefcf013a22c4f6a Mon Sep 17 00:00:00 2001 From: UnladenCoconut Date: Thu, 11 Dec 2025 20:34:46 +0000 Subject: [PATCH 1/4] System.IO.Ports: check device capabilities for DTR/RTS before using in ctor/dispose (#121468) --- .../Windows/Kernel32/Interop.COMMPROP.cs | 2 + .../System/IO/Ports/SerialStream.Windows.cs | 71 ++++++++++--------- 2 files changed, 41 insertions(+), 32 deletions(-) diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.COMMPROP.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.COMMPROP.cs index 1b8d20024eaddb..7fa12747efaf98 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.COMMPROP.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.COMMPROP.cs @@ -16,6 +16,8 @@ internal static partial class Kernel32 // https://msdn.microsoft.com/en-us/library/windows/desktop/aa363189.aspx internal struct COMMPROP { + public const int PCF_DTRDSR = 0x1; + public ushort wPacketLength; public ushort wPacketVersion; public int dwServiceMask; diff --git a/src/libraries/System.IO.Ports/src/System/IO/Ports/SerialStream.Windows.cs b/src/libraries/System.IO.Ports/src/System/IO/Ports/SerialStream.Windows.cs index 158c3895489f61..22ae3c99878d66 100644 --- a/src/libraries/System.IO.Ports/src/System/IO/Ports/SerialStream.Windows.cs +++ b/src/libraries/System.IO.Ports/src/System/IO/Ports/SerialStream.Windows.cs @@ -634,17 +634,20 @@ internal SerialStream(string portName, int baudRate, Parity parity, int dataBits // set constant properties of the DCB InitializeDCB(baudRate, parity, dataBits, stopBits, discardNull); - DtrEnable = dtrEnable; - - // query and cache the initial RtsEnable value - // so that set_RtsEnable can do the (value != rtsEnable) optimization - _rtsEnable = (GetDcbFlag(Interop.Kernel32.DCBFlags.FRTSCONTROL) == Interop.Kernel32.DCBRTSFlowControl.RTS_CONTROL_ENABLE); - - // now set this.RtsEnable to the specified value. - // Handshake takes precedence, this will be a nop if - // handshake is either RequestToSend or RequestToSendXOnXOff - if ((handshake != Handshake.RequestToSend && handshake != Handshake.RequestToSendXOnXOff)) - RtsEnable = rtsEnable; + //if device doesnt support DTR and DTR is disabled, then dont try to set DTR + if (DtrEnable || ((_commProp.dwProvCapabilities & Interop.Kernel32.COMMPROP.PCF_DTRDSR) != 0)) { + DtrEnable = dtrEnable; + + // query and cache the initial RtsEnable value + // so that set_RtsEnable can do the (value != rtsEnable) optimization + _rtsEnable = (GetDcbFlag(Interop.Kernel32.DCBFlags.FRTSCONTROL) == Interop.Kernel32.DCBRTSFlowControl.RTS_CONTROL_ENABLE); + + // now set this.RtsEnable to the specified value. + // Handshake takes precedence, this will be a nop if + // handshake is either RequestToSend or RequestToSendXOnXOff + if ((handshake != Handshake.RequestToSend && handshake != Handshake.RequestToSendXOnXOff)) + RtsEnable = rtsEnable; + } // NOTE: this logic should match what is in the ReadTimeout property if (readTimeout == 0) @@ -717,29 +720,33 @@ protected override void Dispose(bool disposing) // turn off all events and signal WaitCommEvent Interop.Kernel32.SetCommMask(_handle, 0); - if (!Interop.Kernel32.EscapeCommFunction(_handle, Interop.Kernel32.CommFunctions.CLRDTR)) - { - int hr = Marshal.GetLastPInvokeError(); - - // access denied can happen if USB is yanked out. If that happens, we - // want to at least allow finalize to succeed and clean up everything - // we can. To achieve this, we need to avoid further attempts to access - // the SerialPort. A customer also reported seeing ERROR_BAD_COMMAND here. - // Do not throw an exception on the finalizer thread - that's just rude, - // since apps can't catch it and we may tear down the app. - const int ERROR_DEVICE_REMOVED = 1617; - if ((hr == Interop.Errors.ERROR_ACCESS_DENIED || hr == Interop.Errors.ERROR_BAD_COMMAND || hr == ERROR_DEVICE_REMOVED) && !disposing) + + //if device supports DTR then clear + if ((_commProp.dwProvCapabilities & Interop.Kernel32.COMMPROP.PCF_DTRDSR) != 0) { + if (!Interop.Kernel32.EscapeCommFunction(_handle, Interop.Kernel32.CommFunctions.CLRDTR)) { - skipSPAccess = true; - } - else - { - // should not happen - Debug.Fail($"Unexpected error code from EscapeCommFunction in SerialPort.Dispose(bool) Error code: 0x{(uint)hr:x}"); + int hr = Marshal.GetLastPInvokeError(); - // Do not throw an exception from the finalizer here. - if (disposing) - throw Win32Marshal.GetExceptionForLastWin32Error(); + // access denied can happen if USB is yanked out. If that happens, we + // want to at least allow finalize to succeed and clean up everything + // we can. To achieve this, we need to avoid further attempts to access + // the SerialPort. A customer also reported seeing ERROR_BAD_COMMAND here. + // Do not throw an exception on the finalizer thread - that's just rude, + // since apps can't catch it and we may tear down the app. + const int ERROR_DEVICE_REMOVED = 1617; + if ((hr == Interop.Errors.ERROR_ACCESS_DENIED || hr == Interop.Errors.ERROR_BAD_COMMAND || hr == ERROR_DEVICE_REMOVED) && !disposing) + { + skipSPAccess = true; + } + else + { + // should not happen + Debug.Fail($"Unexpected error code from EscapeCommFunction in SerialPort.Dispose(bool) Error code: 0x{(uint)hr:x}"); + + // Do not throw an exception from the finalizer here. + if (disposing) + throw Win32Marshal.GetExceptionForLastWin32Error(); + } } } From 52e3721aeae5e1bada5b824e6a11300b7dc00c92 Mon Sep 17 00:00:00 2001 From: UnladenCoconut Date: Thu, 18 Dec 2025 01:22:41 +0000 Subject: [PATCH 2/4] fix formatting --- .../src/System/IO/Ports/SerialStream.Windows.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libraries/System.IO.Ports/src/System/IO/Ports/SerialStream.Windows.cs b/src/libraries/System.IO.Ports/src/System/IO/Ports/SerialStream.Windows.cs index 22ae3c99878d66..f6586bc10c58c4 100644 --- a/src/libraries/System.IO.Ports/src/System/IO/Ports/SerialStream.Windows.cs +++ b/src/libraries/System.IO.Ports/src/System/IO/Ports/SerialStream.Windows.cs @@ -634,7 +634,7 @@ internal SerialStream(string portName, int baudRate, Parity parity, int dataBits // set constant properties of the DCB InitializeDCB(baudRate, parity, dataBits, stopBits, discardNull); - //if device doesnt support DTR and DTR is disabled, then dont try to set DTR + //if device doesnt support DTR and DTR is disabled, then dont try to set DTR if (DtrEnable || ((_commProp.dwProvCapabilities & Interop.Kernel32.COMMPROP.PCF_DTRDSR) != 0)) { DtrEnable = dtrEnable; @@ -720,7 +720,6 @@ protected override void Dispose(bool disposing) // turn off all events and signal WaitCommEvent Interop.Kernel32.SetCommMask(_handle, 0); - //if device supports DTR then clear if ((_commProp.dwProvCapabilities & Interop.Kernel32.COMMPROP.PCF_DTRDSR) != 0) { if (!Interop.Kernel32.EscapeCommFunction(_handle, Interop.Kernel32.CommFunctions.CLRDTR)) From 0386801accfcf88e4a418fd6a88c2af97e4897a4 Mon Sep 17 00:00:00 2001 From: UnladenCoconut <48599249+Benkol003@users.noreply.github.com> Date: Sat, 27 Dec 2025 22:24:49 +0000 Subject: [PATCH 3/4] fix formatting Co-authored-by: kasperk81 <83082615+kasperk81@users.noreply.github.com> --- .../src/System/IO/Ports/SerialStream.Windows.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.IO.Ports/src/System/IO/Ports/SerialStream.Windows.cs b/src/libraries/System.IO.Ports/src/System/IO/Ports/SerialStream.Windows.cs index f6586bc10c58c4..cecc87750bd4a6 100644 --- a/src/libraries/System.IO.Ports/src/System/IO/Ports/SerialStream.Windows.cs +++ b/src/libraries/System.IO.Ports/src/System/IO/Ports/SerialStream.Windows.cs @@ -721,7 +721,8 @@ protected override void Dispose(bool disposing) // turn off all events and signal WaitCommEvent Interop.Kernel32.SetCommMask(_handle, 0); //if device supports DTR then clear - if ((_commProp.dwProvCapabilities & Interop.Kernel32.COMMPROP.PCF_DTRDSR) != 0) { + if ((_commProp.dwProvCapabilities & Interop.Kernel32.COMMPROP.PCF_DTRDSR) != 0) + { if (!Interop.Kernel32.EscapeCommFunction(_handle, Interop.Kernel32.CommFunctions.CLRDTR)) { int hr = Marshal.GetLastPInvokeError(); From 07e6fa27a9cb99638d3e7ac6049f4e7115fce396 Mon Sep 17 00:00:00 2001 From: UnladenCoconut <48599249+Benkol003@users.noreply.github.com> Date: Sat, 27 Dec 2025 22:25:04 +0000 Subject: [PATCH 4/4] fix formatting Co-authored-by: kasperk81 <83082615+kasperk81@users.noreply.github.com> --- .../src/System/IO/Ports/SerialStream.Windows.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.IO.Ports/src/System/IO/Ports/SerialStream.Windows.cs b/src/libraries/System.IO.Ports/src/System/IO/Ports/SerialStream.Windows.cs index cecc87750bd4a6..d9c1f01ca68197 100644 --- a/src/libraries/System.IO.Ports/src/System/IO/Ports/SerialStream.Windows.cs +++ b/src/libraries/System.IO.Ports/src/System/IO/Ports/SerialStream.Windows.cs @@ -635,7 +635,8 @@ internal SerialStream(string portName, int baudRate, Parity parity, int dataBits InitializeDCB(baudRate, parity, dataBits, stopBits, discardNull); //if device doesnt support DTR and DTR is disabled, then dont try to set DTR - if (DtrEnable || ((_commProp.dwProvCapabilities & Interop.Kernel32.COMMPROP.PCF_DTRDSR) != 0)) { + if (DtrEnable || ((_commProp.dwProvCapabilities & Interop.Kernel32.COMMPROP.PCF_DTRDSR) != 0)) + { DtrEnable = dtrEnable; // query and cache the initial RtsEnable value