Merge branch 'testing/341-conn-reset-on-sock-shutdown-regression'

This change adds artificial tests for TCP connection becoming broken
simultaneously with the server-side initiating the client connection
shutdown sequence with TCP FIN packet.

Ref #344

cheroot/errors.py

@@ -69,9 +69,15 @@ def plat_specific_errors(*errnames):
 * ENOTCONN — client is no longer connected
 * EPIPE — write on a pipe while the other end has been closed
 * ESHUTDOWN — write on a socket which has been shutdown for writing
-* ECONNRESET — connection is reset by the peer
-
-Ref: https://github.com/cherrypy/cheroot/issues/341#issuecomment-735884889
+* ECONNRESET — connection is reset by the peer, we received a TCP RST packet
+
+Refs:
+* https://github.com/cherrypy/cheroot/issues/341#issuecomment-735884889
+* https://bugs.python.org/issue30319
+* https://bugs.python.org/issue30329
+* https://github.com/python/cpython/commit/83a2c28
+* https://github.com/python/cpython/blob/c39b52f/Lib/poplib.py#L297-L302
+* https://docs.microsoft.com/windows/win32/api/winsock/nf-winsock-shutdown
 """
 
 try:  # py3

cheroot/test/test_conn.py

@@ -3,6 +3,7 @@
 from __future__ import absolute_import, division, print_function
 __metaclass__ = type
 
+import errno
 import socket
 import time
 import logging
@@ -16,7 +17,7 @@
 from jaraco.text import trim, unwrap
 
 from cheroot.test import helper, webtest
-from cheroot._compat import IS_CI, IS_PYPY
+from cheroot._compat import IS_CI, IS_PYPY, IS_WINDOWS
 import cheroot.server
 
 
@@ -561,6 +562,128 @@ def check_server_idle_conn_count(count, timeout=1.0):
     test_client.server_instance.timeout = timeout
 
 
+@pytest.mark.parametrize(
+    ('simulated_exception', 'error_number', 'exception_leaks'),
+    (
+        pytest.param(
+            socket.error, errno.ECONNRESET, False,
+            id='socket.error(ECONNRESET)',
+        ),
+        pytest.param(
+            socket.error, errno.EPIPE, False,
+            id='socket.error(EPIPE)',
+        ),
+        pytest.param(
+            socket.error, errno.ENOTCONN, False,
+            id='simulated socket.error(ENOTCONN)',
+        ),
+        pytest.param(
+            None,  # <-- don't raise an artificial exception
+            errno.ENOTCONN, False,
+            id='real socket.error(ENOTCONN)',
+            marks=pytest.mark.xfail(
+                IS_WINDOWS,
+                reason='Now reproducible this way on Windows',
+            ),
+        ),
+        pytest.param(
+            socket.error, errno.ESHUTDOWN, False,
+            id='socket.error(ESHUTDOWN)',
+        ),
+        pytest.param(RuntimeError, 666, True, id='RuntimeError(666)'),
+        pytest.param(socket.error, -1, True, id='socket.error(-1)'),
+    ) + (
+        () if six.PY2 else (
+            pytest.param(
+                ConnectionResetError, errno.ECONNRESET, False,
+                id='ConnectionResetError(ECONNRESET)',
+            ),
+            pytest.param(
+                BrokenPipeError, errno.EPIPE, False,
+                id='BrokenPipeError(EPIPE)',
+            ),
+            pytest.param(
+                BrokenPipeError, errno.ESHUTDOWN, False,
+                id='BrokenPipeError(ESHUTDOWN)',
+            ),
+        )
+    ),
+)
+def test_broken_connection_during_tcp_fin(
+        error_number, exception_leaks,
+        mocker, monkeypatch,
+        simulated_exception, test_client,
+):
+    """Test there's no traceback on broken connection during close.
+
+    It artificially causes :py:data:`~errno.ECONNRESET` /
+    :py:data:`~errno.EPIPE` / :py:data:`~errno.ESHUTDOWN` /
+    :py:data:`~errno.ENOTCONN` as well as unrelated :py:exc:`RuntimeError`
+    and :py:exc:`socket.error(-1) <socket.error>` on the server socket when
+    :py:meth:`socket.shutdown() <socket.socket.shutdown>` is called. It's
+    triggered by closing the client socket before the server had a chance
+    to respond.
+
+    The expectation is that only :py:exc:`RuntimeError` and a
+    :py:exc:`socket.error` with an unusual error code would leak.
+
+    With the :py:data:`None`-parameter, a real non-simulated
+    :py:exc:`OSError(107, 'Transport endpoint is not connected')
+    <OSError>` happens.
+    """
+    exc_instance = (
+        None if simulated_exception is None
+        else simulated_exception(error_number, 'Simulated socket error')
+    )
+    old_close_kernel_socket = (
+        test_client.server_instance.
+        ConnectionClass._close_kernel_socket
+    )
+
+    def _close_kernel_socket(self):
+        monkeypatch.setattr(  # `socket.shutdown` is read-only otherwise
+            self, 'socket',
+            mocker.mock_module.Mock(wraps=self.socket),
+        )
+        if exc_instance is not None:
+            monkeypatch.setattr(
+                self.socket, 'shutdown',
+                mocker.mock_module.Mock(side_effect=exc_instance),
+            )
+        _close_kernel_socket.fin_spy = mocker.spy(self.socket, 'shutdown')
+        _close_kernel_socket.exception_leaked = True
+        old_close_kernel_socket(self)
+        _close_kernel_socket.exception_leaked = False
+
+    monkeypatch.setattr(
+        test_client.server_instance.ConnectionClass,
+        '_close_kernel_socket',
+        _close_kernel_socket,
+    )
+
+    conn = test_client.get_connection()
+    conn.auto_open = False
+    conn.connect()
+    conn.send(b'GET /hello HTTP/1.1')
+    conn.send(('Host: %s' % conn.host).encode('ascii'))
+    conn.close()
+
+    for _ in range(10):  # Let the server attempt TCP shutdown
+        time.sleep(0.1)
+        if hasattr(_close_kernel_socket, 'exception_leaked'):
+            break
+
+    if exc_instance is not None:  # simulated by us
+        assert _close_kernel_socket.fin_spy.spy_exception is exc_instance
+    else:  # real
+        assert isinstance(
+            _close_kernel_socket.fin_spy.spy_exception, socket.error,
+        )
+        assert _close_kernel_socket.fin_spy.spy_exception.errno == error_number
+
+    assert _close_kernel_socket.exception_leaked is exception_leaks
+
+
 @pytest.mark.parametrize(
     'timeout_before_headers',
     (

docs/conf.py

@@ -107,6 +107,10 @@
     # Requires a more liberal 'Accept: ' HTTP request header:
     # Ref: https://github.com/sphinx-doc/sphinx/issues/7247
     r'https://github\.com/cherrypy/cheroot/workflows/[^/]+/badge\.svg',
+
+    # Has an ephemeral anchor (line-range) but actual HTML has separate per-
+    # line anchors.
+    r'https://github.com/python/cpython/blob/c39b52f/Lib/poplib.py#L297-L302',
 ]
 linkcheck_workers = 25