Repository: pyserial/pyserial

Issue:
SerialBase does not call RawIOBase.close()
# TL;DR
Neither `SerialBase` nor any of its derived classes call the ultimate parent class' `io.RawIOBase.close()` method. This leads to an extraneous `close()` call from the finalizer.

# Problem
Version info:
```pyi
>>> sys.version
'3.10.7 (main, Sep  8 2022, 14:34:29) [GCC 12.2.0]'
>>> serial.__version__
'3.5'
```

Let's start with this surprising behavior:
```python
import serial

class MySerial(serial.Serial):
    def close(self):
        print('MySerial.close(): Enter')
        super().close()
        print('MySerial.close(): Exit')

s = MySerial()
print('end of script\n')
```

```console
$ python3 serialwtf_1.py 
end of script

MySerial.close(): Enter
MySerial.close(): Exit
```

:question: Surprisingly, there was a ghost call to `MySerial.close()` after the end of Python code execution.

That is a clue that some destructor/finalizer is running. Even more surprising, any exceptions thrown from `close()` during this ghost call are swallowed and ignored. Painfully, none of these inspection tools could show me the source of this ghost call:
- [`inspect.stack()`](https://docs.python.org/3/library/inspect.html#inspect.stack) -- Showed only the line where I called it
- [`traceback.print_stack()`](https://docs.python.org/3/library/traceback.html#traceback.print_stack) -- Showed only the line where I called it
- [`breakpoint()`](https://docs.python.org/3/library/functions.html#breakpoint) -- Ignored

## Explanation
Note that [`SerialBase`](https://github.com/pyserial/pyserial/blob/v3.5/serial/serialutil.py#L165) inherits from [`io.RawIOBase`](https://docs.python.org/3/library/io.html#io.RawIOBase).

It turns out that [`iobase_finalize()`](https://github.com/python/cpython/blob/v3.10.8/Modules/_io/iobase.c#L256-L307) (from the C module) will [call `.close()`](https://github.com/python/cpython/blob/v3.10.8/Modules/_io/iobase.c#L284) on the object if it has not already been marked as [`closed`](https://docs.python.org/3/library/io.html#io.IOBase.closed).

It's worth noting that the C code will [set a `_finalizing` attribute to `True`](https://github.com/python/cpython/blob/v3.10.8/Modules/_io/iobase.c#L282) to let `close()` know it is called from the finalizer. This lets us prove our understanding:

```python
import serial

class MySerial(serial.Serial):
    def close(self):
        print('MySerial.close(): Enter: closed={} finalizing={}'.format(
            self.closed, getattr(self, '_finalizing', False)))
        super().close()
        print('MySerial.close(): Exit')

s = MySerial()
with s:
    print('exiting `with`')
print('end of script\n')
```
```console
$ python3 serialwtf_2.py 
exiting `with`
MySerial.close(): Enter: closed=False finalizing=False
MySerial.close(): Exit
end of script

MySerial.close(): Enter: closed=False finalizing=True
MySerial.close(): Exit
```


## Root Cause
`IOBase.closed` needs to be set, but the only way to do so is to call `IOBase.close()` (via `super()`).

This indicates that `SerialBase` is not calling `IOBase.close()`, which is indeed the case. In fact, `SerialBase` does not implement `close()`, and `Serial` (from `serialposix.py` at least) does not call `super().close()`.



# Solution
The `Serial` classes should be good Python citizens and always call `super().close()` in their `close()` implementations.

This example shows that this will fix the problem:
```python
import serial
import io

class MySerial(serial.Serial):
    def close(self):
        print('MySerial.close(): Enter: closed={} finalizing={}'.format(
            self.closed, getattr(self, '_finalizing', False)))
        super().close()
        io.RawIOBase.close(self)        # <<<<<<<<
        print('MySerial.close(): Exit')

    def flush(self):
        print('MySerial.flush(): skip')

s = MySerial()
print(f'Before with: s.closed={s.closed}')
with s:
    print('exiting `with`')
print(f'After with: s.closed={s.closed}')
print('end of script\n')
```

```console
$ python3 serialwtf_3.py 
Before with: s.closed=False
exiting `with`
MySerial.close(): Enter: closed=False finalizing=False
MySerial.flush(): skip
MySerial.close(): Exit
After with: s.closed=True
end of script

```

Note that, for this example, I also had to neuter `flush()` because `IOBase.close()` essentially does this [(but in C)](https://github.com/python/cpython/blob/v3.10.8/Modules/_io/iobase.c#L212-L225):

```python
def close(self):
    self.flush()
    self.closed = True
```

...and [`serialposix.py:Serial.flush()`](https://github.com/pyserial/pyserial/blob/v3.5/serial/serialposix.py#L666-L673) will raise an exception if it is not open.

I'm not yet sure how to fix this aspect. It seems like `IOBase` doesn't like the idea of being constructed but *not* open (unlike `Serial`).

Discussion:

Patch:
diff --git a/serial/serialutil.py b/serial/serialutil.py
index f554472..87aaad9 100644
--- a/serial/serialutil.py
+++ b/serial/serialutil.py
@@ -557,6 +557,16 @@ class SerialBase(io.RawIOBase):
             b[:n] = array.array('b', data)
         return n
 
+    def close(self):
+        # Do not call RawIOBase.close() as that will try to flush().
+        pass
+
+    @property
+    def closed(self):
+        # Overrides RawIOBase.closed, as RawIOBase can only be closed once,
+        # but a Serial object can be opened/closed multiple times.
+        return not self.is_open
+
     #  -  -  -  -  -  -  -  -  -  -  -  -  -  -  -  -  -  -  -  -  -  -  -  -
     # context manager
 


Test patch:
diff --git a/test/test_close.py b/test/test_close.py
new file mode 100644
index 0000000..27b049e
--- /dev/null
+++ b/test/test_close.py
@@ -0,0 +1,58 @@
+#! /usr/bin/env python
+#
+# This file is part of pySerial - Cross platform serial port support for Python
+# (C) 2001-2015 Chris Liechti <cliechti@gmx.net>
+# (C) 2023 Google LLC
+#
+# SPDX-License-Identifier:    BSD-3-Clause
+import sys
+import unittest
+import serial
+
+# on which port should the tests be performed:
+PORT = 'loop://'
+
+class TestClose(unittest.TestCase):
+
+    def test_closed_true(self):
+        # closed is True if a Serial port is not open
+        s = serial.Serial()
+        self.assertFalse(s.is_open)
+        self.assertTrue(s.closed)
+
+    def test_closed_false(self):
+        # closed is False if a Serial port is open
+        s = serial.serial_for_url(PORT, timeout=1)
+        self.assertTrue(s.is_open)
+        self.assertFalse(s.closed)
+
+        s.close()
+        self.assertTrue(s.closed)
+
+    def test_close_not_called_by_finalize_if_closed(self):
+        close_calls = 0
+
+        class TestSerial(serial.Serial):
+            def close(self):
+                nonlocal close_calls
+                close_calls += 1
+
+        with TestSerial() as s:
+            pass
+            # close() should be called here
+
+        # Trigger RawIOBase finalization.
+        # Because we override .closed, close() should not be called
+        # if Serial says it is already closed.
+        del s
+
+        self.assertEqual(close_calls, 1)
+
+# - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+if __name__ == '__main__':
+    if len(sys.argv) > 1:
+        PORT = sys.argv[1]
+    sys.stdout.write("Testing port: {!r}\n".format(PORT))
+    sys.argv[1:] = ['-v']
+    # When this module is executed from the command-line, it runs all its tests
+    unittest.main()