Fix race condition and improve robustness during socket I/O

Fixes to make socket I/O more resilient during connection teardown.

1. BufferedWriter's write(): Added error handling to ignore common
   socket errors (e.g., ECONNRESET, EPIPE, ENOTCONN, EBADF) that occur
   when the underlying connection has been unexpectedly closed by the
   client or OS. This prevents a crash when attempting to write to a
   defunct socket.
2. BufferedWriters's close(): Made idempotent, allowing safe repeated
   calls without raising exceptions.
3. Needed to add explicit handling of WINDOWS environments as these are
   seen to throw Windows specific WSAENOTSOCK errors.

Includes new unit tests to cover the idempotency and graceful handling
of already closed underlying buffers.

Author: julianz-

cheroot/errors.py

@@ -1,8 +1,17 @@
 """Collection of exceptions raised and/or processed by Cheroot."""
 
 import errno
+import socket
 import sys
 
+from cheroot._compat import IS_WINDOWS
+
+
+try:
+    from OpenSSL.SSL import SysCallError as _OpenSSL_SysCallError
+except ImportError:
+    _OpenSSL_SysCallError = None
+
 
 class MaxSizeExceeded(Exception):
     """Exception raised when a client sends more data then allowed under limit.
@@ -64,13 +73,36 @@ def plat_specific_errors(*errnames):
     socket_errors_to_ignore.extend(plat_specific_errors('EPROTOTYPE'))
     socket_errors_nonblocking.extend(plat_specific_errors('EPROTOTYPE'))
 
+if IS_WINDOWS:
+    # On Windows, try to get the named constant from the socket module.
+    # If that fails, fall back to the known numeric value.
+    try:
+        _not_a_socket_err = socket.WSAENOTSOCK
+    except AttributeError:
+        _not_a_socket_err = 10038
+else:
+    """On other platforms, the relevant error is EBADF (Bad file descriptor)
+       EBADF is already incuded in acceptable_sock_shutdown_error_codes
+    """
 
 acceptable_sock_shutdown_error_codes = {
+    errno.EBADF,
     errno.ENOTCONN,
     errno.EPIPE,
     errno.ESHUTDOWN,  # corresponds to BrokenPipeError in Python 3
     errno.ECONNRESET,  # corresponds to ConnectionResetError in Python 3
 }
+
+if IS_WINDOWS:
+    acceptable_sock_shutdown_error_codes.add(socket.WSAENOTSOCK)
+
+acceptable_sock_shutdown_exceptions = (
+    BrokenPipeError,
+    ConnectionResetError,
+    # conditionally add _OpenSSL_SysCallError to the list
+    *(() if _OpenSSL_SysCallError is None else (_OpenSSL_SysCallError,)),
+)
+
 """Errors that may happen during the connection close sequence.
 
 * ENOTCONN — client is no longer connected
@@ -86,5 +118,3 @@ def plat_specific_errors(*errnames):
 * https://github.com/python/cpython/blob/c39b52f/Lib/poplib.py#L297-L302
 * https://docs.microsoft.com/windows/win32/api/winsock/nf-winsock-shutdown
 """
-
-acceptable_sock_shutdown_exceptions = (BrokenPipeError, ConnectionResetError)

cheroot/makefile.py

@@ -4,6 +4,10 @@
 import _pyio as io
 import socket
 
+from OpenSSL.SSL import SysCallError
+
+from cheroot.errors import acceptable_sock_shutdown_error_codes
+
 
 # Write only 16K at a time to sockets
 SOCK_WRITE_BLOCKSIZE = 16384
@@ -32,8 +36,53 @@ def _flush_unlocked(self):
                 n = self.raw.write(bytes(self._write_buf))
             except io.BlockingIOError as e:
                 n = e.characters_written
+            except OSError as e:
+                error_code = e.errno
+                if error_code in acceptable_sock_shutdown_error_codes:
+                    # The socket is gone, so just ignore this error.
+                    return
+                raise
+            except SysCallError as e:
+                error_code = e.args[0]
+                if error_code in acceptable_sock_shutdown_error_codes:
+                    # The socket is gone, so just ignore this error.
+                    return
+                raise
+            else:
+                # The 'try' block completed without an exception
+                if n is None:
+                    # This could happen with non-blocking write
+                    # when nothing was written
+                    break
+
             del self._write_buf[:n]
 
+    def close(self):
+        """
+        Close the stream and its underlying file object.
+
+        This method is designed to be idempotent (it can be called multiple
+        times without side effects). It gracefully handles a race condition
+        where the underlying socket may have already been closed by the remote
+        client or another thread.
+
+        A SysCallError or OSError with errno.EBADF or errno.ENOTCONN is caught
+        and ignored, as these indicate a normal, expected connection teardown.
+        Other exceptions are re-raised.
+        """
+        if self.closed:  # pylint: disable=W0125
+            return
+
+        try:
+            super().close()
+        except (OSError, SysCallError) as e:
+            error_code = e.errno if isinstance(e, OSError) else e.args[0]
+            if error_code in acceptable_sock_shutdown_error_codes:
+                # The socket is already closed, which is expected during
+                # a race condition.
+                return
+            raise
+
 
 class StreamReader(io.BufferedReader):
     """Socket stream reader."""

cheroot/makefile.pyi

@@ -1,5 +1,11 @@
 import io
 
+# this variable is only needed in Windows environments
+# and is an int in those cases
+# but mypy stubtest fails when adding a guard such as
+# if sys.platform == 'win32' or setting is as int
+WSAENOTSOCK: None
+
 SOCK_WRITE_BLOCKSIZE: int
 
 class BufferedWriter(io.BufferedWriter):

cheroot/server.py

@@ -67,6 +67,7 @@
 
 import contextlib
 import email.utils
+import errno
 import io
 import logging
 import os
@@ -81,6 +82,13 @@
 import urllib.parse
 from functools import lru_cache
 
+from OpenSSL.SSL import SysCallError
+
+from cheroot.errors import (
+    acceptable_sock_shutdown_error_codes,
+    acceptable_sock_shutdown_exceptions,
+)
+
 from . import __version__, connections, errors
 from ._compat import IS_PPC, bton
 from .makefile import MakeFile, StreamWriter
@@ -1189,9 +1197,21 @@ def write(self, chunk):
         if self.chunked_write and chunk:
             chunk_size_hex = hex(len(chunk))[2:].encode('ascii')
             buf = [chunk_size_hex, CRLF, chunk, CRLF]
-            self.conn.wfile.write(EMPTY.join(buf))
+            data = EMPTY.join(buf)
         else:
-            self.conn.wfile.write(chunk)
+            data = chunk
+
+        try:
+            self.conn.wfile.write(data)
+        except (SysCallError, ConnectionError, OSError) as write_error:
+            if isinstance(write_error, OSError):
+                error_code = write_error.errno
+            else:
+                error_code = write_error.args[0]
+            if error_code in acceptable_sock_shutdown_error_codes:
+                # The socket is gone, so just ignore this error.
+                return
+            raise
 
     def send_headers(self):  # noqa: C901  # FIXME
         """Assert, process, and send the HTTP response message-headers.
@@ -1285,7 +1305,27 @@ def send_headers(self):  # noqa: C901  # FIXME
         for k, v in self.outheaders:
             buf.append(k + COLON + SPACE + v + CRLF)
         buf.append(CRLF)
-        self.conn.wfile.write(EMPTY.join(buf))
+        try:
+            self.conn.wfile.write(EMPTY.join(buf))
+        except (SysCallError, ConnectionError, OSError) as e:
+            # We explicitly ignore these errors because they indicate the
+            # client has already closed the connection, which is a normal
+            # occurrence during a race condition.
+
+            # The .errno attribute is only available on OSError
+            # The .args[0] attribute is available on SysCallError
+            # Check for both cases to handle different exception types
+            error_code = e.errno if isinstance(e, OSError) else e.args[0]
+            if error_code in {
+                errno.ECONNRESET,
+                errno.EPIPE,
+                errno.ENOTCONN,
+                errno.EBADF,
+            }:
+                self.close_connection = True
+                self.conn.close()
+                return
+            raise
 
 
 class HTTPConnection:
@@ -1543,10 +1583,10 @@ def _close_kernel_socket(self):
 
         try:
             shutdown(socket.SHUT_RDWR)  # actually send a TCP FIN
-        except errors.acceptable_sock_shutdown_exceptions:
+        except acceptable_sock_shutdown_exceptions:  # pylint: disable=E0712
             pass
         except socket.error as e:
-            if e.errno not in errors.acceptable_sock_shutdown_error_codes:
+            if e.errno not in acceptable_sock_shutdown_error_codes:
                 raise
 
 

cheroot/test/test_makefile.py

@@ -1,5 +1,7 @@
 """Tests for :py:mod:`cheroot.makefile`."""
 
+import io
+
 from cheroot import makefile
 
 
@@ -51,3 +53,27 @@ def test_bytes_written():
     wfile = makefile.MakeFile(sock, 'w')
     wfile.write(b'bar')
     assert wfile.bytes_written == 3
+
+
+def test_close_is_idempotent():
+    """Test that close() can be called multiple times safely."""
+    raw_buffer = io.BytesIO()
+    buffered_writer = makefile.BufferedWriter(raw_buffer)
+
+    # Should not raise any exceptions
+    buffered_writer.close()
+    buffered_writer.close()  # Second call should be safe
+
+    assert buffered_writer.closed
+
+
+def test_close_handles_already_closed_buffer():
+    """Test that close() handles already closed underlying buffer."""
+    raw_buffer = io.BytesIO()
+    buffered_writer = makefile.BufferedWriter(raw_buffer)
+
+    # Close the underlying buffer first
+    raw_buffer.close()
+
+    # This should not raise an exception
+    buffered_writer.close()

docs/changelog-fragments.d/779.bugfix.rst

@@ -0,0 +1,3 @@
+Fixed race conditions to make socket I/O more resilient during connection teardown.
+
+-- by :user:`julianz-`

docs/spelling_wordlist.txt

@@ -57,6 +57,7 @@ signalling
 Sigstore
 ssl
 stdout
+stubtest
 subclasses
 submodules
 subpackages