From 384e6d035b45e687aacfcd39b2fe6c8e8ab92f62 Mon Sep 17 00:00:00 2001
From: Sviatoslav Sydorenko <wk@sydorenko.org.ua>
Date: Mon, 30 Nov 2020 18:51:58 +0100
Subject: [PATCH 1/2] =?UTF-8?q?=F0=9F=90=9B=20Suppress=20legit=20OS=20erro?=
 =?UTF-8?q?rs=20on=20socket=20shutdown?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When closing a socket we normally want to terminate the transport-
level connection by sending a TCP FIN packet over the wire. This works
great if the client is willing to perform a 3-way handshake but
sometimes the client-side just drops the connection by sending an RST
instead of a FIN. In this case, our server-side `socket.shutdown()`
call will raise an OSError (socket.error on Python 2) or its
derivatives. Which is perfectly fine and it's alright for us to ignore
those.

The kernel socket close helper rewrite introduced a behavior of only
suppressing ENOTCONN in v8.4.8 (the case when the client is no longer
with us) but it left own a few other cases that may be happening too.

This change fixes that by extending the list of errors that are to be
suppressed. Here's what's handled from now on:
  * 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

Fixes #341

Refs:
  * https://en.wikipedia.org/wiki/Transmission_Control_Protocol#Connection_termination
  * http://cs.baylor.edu/~donahoo/practical/CSockets/TCPRST.pdf
  * https://github.com/cherrypy/cheroot/issues/341#issuecomment-735884889

Co-Authored-By: Liam Staskawicz <liam@stask.net>
---
 cheroot/errors.py | 22 ++++++++++++++++++++++
 cheroot/server.py |  6 +++---
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/cheroot/errors.py b/cheroot/errors.py
index 4395e56528..4101fe3e4a 100644
--- a/cheroot/errors.py
+++ b/cheroot/errors.py
@@ -1,3 +1,4 @@
+# -*- coding: utf-8 -*-
 """Collection of exceptions raised and/or processed by Cheroot."""
 
 from __future__ import absolute_import, division, print_function
@@ -56,3 +57,24 @@ def plat_specific_errors(*errnames):
 if sys.platform == 'darwin':
     socket_errors_to_ignore.extend(plat_specific_errors('EPROTOTYPE'))
     socket_errors_nonblocking.extend(plat_specific_errors('EPROTOTYPE'))
+
+
+acceptable_sock_shutdown_error_codes = {
+    errno.ENOTCONN,
+    errno.EPIPE, errno.ESHUTDOWN,  # corresponds to BrokenPipeError in Python 3
+    errno.ECONNRESET,  # corresponds to ConnectionResetError in Python 3
+}
+"""Errors that may happen during the connection close sequence.
+
+* 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
+"""
+
+try:  # py3
+    acceptable_sock_shutdown_exceptions = (BrokenPipeError, ConnectionResetError)
+except NameError:  # py2
+    acceptable_sock_shutdown_exceptions = ()
diff --git a/cheroot/server.py b/cheroot/server.py
index a5c31b2e65..2a9c95310b 100644
--- a/cheroot/server.py
+++ b/cheroot/server.py
@@ -75,7 +75,6 @@
 import platform
 import contextlib
 import threading
-import errno
 
 try:
     from functools import lru_cache
@@ -1478,9 +1477,10 @@ def _close_kernel_socket(self):
 
         try:
             shutdown(socket.SHUT_RDWR)  # actually send a TCP FIN
+        except errors.acceptable_sock_shutdown_exceptions:
+            pass
         except socket.error as e:
-            # Suppress "client is no longer connected"
-            if e.errno != errno.ENOTCONN:
+            if e.errno not in errors.acceptable_sock_shutdown_error_codes:
                 raise
 
 

From 48d33d48825789d3b524bb71bf2116d5525cc291 Mon Sep 17 00:00:00 2001
From: "Jason R. Coombs" <jaraco@jaraco.com>
Date: Sat, 5 Dec 2020 11:30:23 -0500
Subject: [PATCH 2/2] Update changelog. Ref #342.

---
 CHANGES.rst | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/CHANGES.rst b/CHANGES.rst
index 134e3ff27f..189e79a7be 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -6,6 +6,9 @@
   waiting for a ``tick`` event, addressing performance
   degradation introduced in v8.1.0.
 
+- :issue:`341` via :pr:`342`: Suppress legitimate OS errors
+  expected on shutdown.
+
 .. scm-version-title:: v8.4.8
 
 - :issue:`317` via :pr:`337`: Fixed a regression in