Improve/fix SerialStream.Dispose (dotnet/corefx#33221)

* Improve/fix SerialStream.Dispose

* Review feedback

* remove unused using statement

* Use ToFileDescriptor

Commit migrated from https://github.com/dotnet/corefx/commit/9b9e018fef877c8151aaf6367098cd5f0a34adc4

diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.Shutdown.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.Shutdown.cs
index ad5e729..7e6e440 100644 (file)
--- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.Shutdown.cs
+++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.Shutdown.cs
@@ -12,5 +12,8 @@ internal static partial class Interop
     {
         [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_Shutdown")]
         internal static extern Error Shutdown(SafeHandle socket, SocketShutdown how);
+
+        [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_Shutdown")]
+        internal static extern Error Shutdown(IntPtr socket, SocketShutdown how);
     }
 }

diff --git a/src/libraries/Native/Unix/System.IO.Ports.Native/pal_serial.c b/src/libraries/Native/Unix/System.IO.Ports.Native/pal_serial.c
index 9b00589..cca1c53 100644 (file)
--- a/src/libraries/Native/Unix/System.IO.Ports.Native/pal_serial.c
+++ b/src/libraries/Native/Unix/System.IO.Ports.Native/pal_serial.c
@@ -3,6 +3,7 @@
 // See the LICENSE file in the project root for more information.
 
 #include "pal_types.h"
+#include "pal_utilities.h"
 #include <fcntl.h>
 #include <errno.h>
 #include <pal_serial.h>
@@ -32,3 +33,14 @@ intptr_t SystemIoPortsNative_SerialPortOpen(const char * name)
 
     return fd;
 }
+
+int SystemIoPortsNative_SerialPortClose(intptr_t handle)
+{
+    int fd = ToFileDescriptor(handle);
+    // some devices don't unlock handles from exclusive access
+    // preventing reopening after closing the handle
+
+    // ignoring the error - best effort
+    ioctl(fd, TIOCNXCL);
+    return close(fd);
+}

diff --git a/src/libraries/Native/Unix/System.IO.Ports.Native/pal_serial.h b/src/libraries/Native/Unix/System.IO.Ports.Native/pal_serial.h
index 954abd8..b998de8 100644 (file)
--- a/src/libraries/Native/Unix/System.IO.Ports.Native/pal_serial.h
+++ b/src/libraries/Native/Unix/System.IO.Ports.Native/pal_serial.h
@@ -6,4 +6,4 @@
 #include "pal_compiler.h"
 
 DLLEXPORT intptr_t SystemIoPortsNative_SerialPortOpen(const char * name);
-
+DLLEXPORT int SystemIoPortsNative_SerialPortClose(intptr_t fd);

diff --git a/src/libraries/System.IO.Ports/src/Interop/Unix/Interop.Serial.cs b/src/libraries/System.IO.Ports/src/Interop/Unix/Interop.Serial.cs
index d7f1134..66822a7 100644 (file)
--- a/src/libraries/System.IO.Ports/src/Interop/Unix/Interop.Serial.cs
+++ b/src/libraries/System.IO.Ports/src/Interop/Unix/Interop.Serial.cs
@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using System;
 using System.IO.Ports;
 using System.Runtime.InteropServices;
 using Microsoft.Win32.SafeHandles;
@@ -11,6 +12,9 @@ internal static partial class Interop
     internal static partial class Serial
     {
         [DllImport(Libraries.IOPortsNative, EntryPoint = "SystemIoPortsNative_SerialPortOpen", SetLastError = true)]
-        internal static extern SafeFileHandle SerialPortOpen(string name);
+        internal static extern SafeSerialDeviceHandle SerialPortOpen(string name);
+
+        [DllImport(Libraries.IOPortsNative, EntryPoint = "SystemIoPortsNative_SerialPortClose", SetLastError = true)]
+        internal static extern int SerialPortClose(IntPtr handle);
     }
 }

diff --git a/src/libraries/System.IO.Ports/src/Interop/Unix/Interop.Termios.cs b/src/libraries/System.IO.Ports/src/Interop/Unix/Interop.Termios.cs
index 29743b9..1aa9d40 100644 (file)
--- a/src/libraries/System.IO.Ports/src/Interop/Unix/Interop.Termios.cs
+++ b/src/libraries/System.IO.Ports/src/Interop/Unix/Interop.Termios.cs
@@ -27,30 +27,30 @@ internal static partial class Interop
         }
 
         [DllImport(Libraries.IOPortsNative, EntryPoint = "SystemIoPortsNative_TermiosReset", SetLastError = true)]
-        internal static extern int TermiosReset(SafeFileHandle handle, int speed, int data, StopBits stop, Parity parity, Handshake flow);
+        internal static extern int TermiosReset(SafeSerialDeviceHandle handle, int speed, int data, StopBits stop, Parity parity, Handshake flow);
 
         [DllImport(Libraries.IOPortsNative, EntryPoint = "SystemIoPortsNative_TermiosGetSignal", SetLastError = true)]
-        internal static extern int TermiosGetSignal(SafeFileHandle handle, Signals signal);
+        internal static extern int TermiosGetSignal(SafeSerialDeviceHandle handle, Signals signal);
 
         [DllImport(Libraries.IOPortsNative, EntryPoint = "SystemIoPortsNative_TermiosSetSignal", SetLastError = true)]
-        internal static extern int TermiosGetSignal(SafeFileHandle handle, Signals signal, int set);
+        internal static extern int TermiosGetSignal(SafeSerialDeviceHandle handle, Signals signal, int set);
 
         [DllImport(Libraries.IOPortsNative, EntryPoint = "SystemIoPortsNative_TermiosSetSpeed", SetLastError = true)]
-        internal static extern int TermiosSetSpeed(SafeFileHandle handle, int speed);
+        internal static extern int TermiosSetSpeed(SafeSerialDeviceHandle handle, int speed);
 
         [DllImport(Libraries.IOPortsNative, EntryPoint = "SystemIoPortsNative_TermiosGetSpeed", SetLastError = true)]
-        internal static extern int TermiosGetSpeed(SafeFileHandle handle);
+        internal static extern int TermiosGetSpeed(SafeSerialDeviceHandle handle);
 
         [DllImport(Libraries.IOPortsNative, EntryPoint = "SystemIoPortsNative_TermiosAvailableBytes", SetLastError = true)]
-        internal static extern int TermiosGetAvailableBytes(SafeFileHandle handle, Queue input);
+        internal static extern int TermiosGetAvailableBytes(SafeSerialDeviceHandle handle, Queue input);
 
         [DllImport(Libraries.IOPortsNative, EntryPoint = "SystemIoPortsNative_TermiosDiscard", SetLastError = true)]
-        internal static extern int TermiosDiscard(SafeFileHandle handle, Queue input);
+        internal static extern int TermiosDiscard(SafeSerialDeviceHandle handle, Queue input);
 
         [DllImport(Libraries.IOPortsNative, EntryPoint = "SystemIoPortsNative_TermiosDrain", SetLastError = true)]
-        internal static extern int TermiosDrain(SafeFileHandle handle);
+        internal static extern int TermiosDrain(SafeSerialDeviceHandle handle);
 
         [DllImport(Libraries.IOPortsNative, EntryPoint = "SystemIoPortsNative_TermiosSendBreak", SetLastError = true)]
-        internal static extern int TermiosSendBreak(SafeFileHandle handle, int duration);
+        internal static extern int TermiosSendBreak(SafeSerialDeviceHandle handle, int duration);
     }
 }

diff --git a/src/libraries/System.IO.Ports/src/System.IO.Ports.csproj b/src/libraries/System.IO.Ports/src/System.IO.Ports.csproj
index a73de9e..89466f4 100644 (file)
--- a/src/libraries/System.IO.Ports/src/System.IO.Ports.csproj
+++ b/src/libraries/System.IO.Ports/src/System.IO.Ports.csproj
@@ -152,6 +152,7 @@
     <Compile Include="System\IO\Ports\SerialStream.Win32.cs" />
   </ItemGroup>
   <ItemGroup Condition=" '$(TargetsLinux)' == 'true' ">
+    <Compile Include="System\IO\Ports\SafeSerialDeviceHandle.Unix.cs" />
     <Compile Include="System\IO\Ports\SerialPort.Linux.cs" />
     <Compile Include="System\IO\Ports\SerialStream.Unix.cs" />
     <Compile Include="Interop\Unix\Interop.Termios.cs" />

diff --git a/src/libraries/System.IO.Ports/src/System/IO/Ports/SafeSerialDeviceHandle.Unix.cs b/src/libraries/System.IO.Ports/src/System/IO/Ports/SafeSerialDeviceHandle.Unix.cs
new file mode 100644 (file)
index 0000000..dfdac02
--- /dev/null
+++ b/src/libraries/System.IO.Ports/src/System/IO/Ports/SafeSerialDeviceHandle.Unix.cs
@@ -0,0 +1,48 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System;
+using System.Diagnostics;
+using System.IO;
+using System.Net.Sockets;
+using System.Runtime.InteropServices;
+using Microsoft.Win32.SafeHandles;
+
+namespace System.IO.Ports
+{
+    internal sealed class SafeSerialDeviceHandle : SafeHandleMinusOneIsInvalid
+    {
+        private SafeSerialDeviceHandle() : base(ownsHandle: true)
+        {
+        }
+
+        internal static SafeSerialDeviceHandle Open(string portName)
+        {
+            Debug.Assert(portName != null);
+            SafeSerialDeviceHandle handle = Interop.Serial.SerialPortOpen(portName);
+
+            if (handle.IsInvalid)
+            {
+                // exception type is matching Windows
+                throw new UnauthorizedAccessException(
+                    SR.Format(SR.UnauthorizedAccess_IODenied_Port, portName),
+                    Interop.GetIOException(Interop.Sys.GetLastErrorInfo()));
+            }
+
+            return handle;
+        }
+
+        protected override bool ReleaseHandle()
+        {
+            Interop.Sys.Shutdown(handle, SocketShutdown.Both);
+            int result = Interop.Serial.SerialPortClose(handle);
+
+            Debug.Assert(result == 0, string.Format(
+                             "Close failed with result {0} and error {1}",
+                             result, Interop.Sys.GetLastErrorInfo()));
+
+            return result == 0;
+        }
+    }
+}

diff --git a/src/libraries/System.IO.Ports/src/System/IO/Ports/SerialStream.Unix.cs b/src/libraries/System.IO.Ports/src/System/IO/Ports/SerialStream.Unix.cs
index 43995c6..64a239f 100644 (file)
--- a/src/libraries/System.IO.Ports/src/System/IO/Ports/SerialStream.Unix.cs
+++ b/src/libraries/System.IO.Ports/src/System/IO/Ports/SerialStream.Unix.cs
@@ -2,7 +2,6 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-using Microsoft.Win32.SafeHandles;
 using System;
 using System.Collections;
 using System.Collections.Concurrent;
@@ -11,7 +10,6 @@ using System.Runtime.InteropServices;
 using System.IO.Ports;
 using System.Threading;
 using System.Threading.Tasks;
-using System.Net.Sockets;
 
 namespace System.IO.Ports
 {
@@ -22,6 +20,7 @@ namespace System.IO.Ports
         private const int IOLoopIdleTimeout = 2000;
         private bool _ioLoopFinished = false;
 
+        private SafeSerialDeviceHandle _handle = null;
         private int _baudRate;
         private StopBits _stopBits;
         private Parity _parity;
@@ -507,17 +506,6 @@ namespace System.IO.Ports
             }
         }
 
-        internal SafeFileHandle OpenPort(string portName)
-        {
-            SafeFileHandle handle = Interop.Serial.SerialPortOpen(portName);
-            if (handle.IsInvalid)
-            {
-                throw new UnauthorizedAccessException(string.Format(SR.UnauthorizedAccess_IODenied_Port, portName));
-            }
-
-            return handle;
-        }
-
         // this method is used by SerialPort upon SerialStream's creation
         internal SerialStream(string portName, int baudRate, Parity parity, int dataBits, StopBits stopBits, int readTimeout, int writeTimeout, Handshake handshake,
             bool dtrEnable, bool rtsEnable, bool discardNull, byte parityReplace)
@@ -531,7 +519,7 @@ namespace System.IO.Ports
 
             // Error checking done in SerialPort.
 
-            SafeFileHandle tempHandle = OpenPort(portName);
+            SafeSerialDeviceHandle tempHandle = SafeSerialDeviceHandle.Open(portName);
 
             try
             {
@@ -569,7 +557,7 @@ namespace System.IO.Ports
             {
                 // if there are any exceptions after the call to CreateFile, we need to be sure to close the
                 // handle before we let them continue up.
-                tempHandle.Close();
+                tempHandle.Dispose();
                 _handle = null;
                 throw;
             }
@@ -623,12 +611,9 @@ namespace System.IO.Ports
 
                 FinishPendingIORequests();
 
-                // Signal the other side that we're closing.  Should do regardless of whether we've called
-                // Close() or not Dispose()
-                if (_handle != null && !_handle.IsInvalid)
+                if (_handle != null)
                 {
-                    Interop.Sys.Shutdown(_handle, SocketShutdown.Both);
-                    _handle.Close();
+                    _handle.Dispose();
                     _handle = null;
                 }
             }

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 8c239a2..4bd5e21 100644 (file)
--- 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
@@ -36,6 +36,8 @@ namespace System.IO.Ports
         // called when one character is received.
         internal event SerialDataReceivedEventHandler DataReceived;
 
+        private SafeFileHandle _handle = null;
+
         // members supporting properties exposed to SerialPort
         private byte _parityReplace = (byte)'?';
         private bool _isAsync = true;

diff --git a/src/libraries/System.IO.Ports/src/System/IO/Ports/SerialStream.cs b/src/libraries/System.IO.Ports/src/System/IO/Ports/SerialStream.cs
index f0822a5..3d63e94 100644 (file)
--- a/src/libraries/System.IO.Ports/src/System/IO/Ports/SerialStream.cs
+++ b/src/libraries/System.IO.Ports/src/System/IO/Ports/SerialStream.cs
@@ -21,8 +21,6 @@ namespace System.IO.Ports
         private bool _inBreak = false;
         private Handshake _handshake;
 
-        private SafeFileHandle _handle = null;
-
 #pragma warning disable CS0067 // Events shared by Windows and Linux, on Linux we currently never call them
         // called when any of the pin/ring-related triggers occurs
         internal event SerialPinChangedEventHandler PinChanged;