Issue #23430: Stop socketserver from catching SystemExit etc from handlers

Also make handle_error() consistently output to stderr, and fix the
documentation.

Lib/socketserver.py

@@ -132,6 +132,7 @@ import socket
 import selectors
 import os
 import errno
+import sys
 try:
     import threading
 except ImportError:
@@ -316,9 +317,12 @@ class BaseServer:
         if self.verify_request(request, client_address):
             try:
                 self.process_request(request, client_address)
-            except:
+            except Exception:
                 self.handle_error(request, client_address)
                 self.shutdown_request(request)
+            except:
+                self.shutdown_request(request)
+                raise
         else:
             self.shutdown_request(request)
 
@@ -372,12 +376,12 @@ class BaseServer:
         The default is to print a traceback and continue.
 
         """
-        print('-'*40)
-        print('Exception happened during processing of request from', end=' ')
-        print(client_address)
+        print('-'*40, file=sys.stderr)
+        print('Exception happened during processing of request from',
+            client_address, file=sys.stderr)
         import traceback
-        traceback.print_exc() # XXX But this goes to stderr!
-        print('-'*40)
+        traceback.print_exc()
+        print('-'*40, file=sys.stderr)
 
 
 class TCPServer(BaseServer):
@@ -601,16 +605,17 @@ class ForkingMixIn:
         else:
             # Child process.
             # This must never return, hence os._exit()!
+            status = 1
             try:
                 self.finish_request(request, client_address)
-                self.shutdown_request(request)
-                os._exit(0)
-            except:
+                status = 0
+            except Exception:
+                self.handle_error(request, client_address)
+            finally:
                 try:
-                    self.handle_error(request, client_address)
                     self.shutdown_request(request)
                 finally:
-                    os._exit(1)
+                    os._exit(status)
 
 
 class ThreadingMixIn:
@@ -628,9 +633,9 @@ class ThreadingMixIn:
         """
         try:
             self.finish_request(request, client_address)
-            self.shutdown_request(request)
-        except:
+        except Exception:
             self.handle_error(request, client_address)
+        finally:
             self.shutdown_request(request)
 
     def process_request(self, request, client_address):

Lib/test/test_socketserver.py

@@ -58,6 +58,7 @@ if HAVE_UNIX_SOCKETS:
 
 @contextlib.contextmanager
 def simple_subprocess(testcase):
+    """Tests that a custom child process is not waited on (Issue 1540386)"""
     pid = os.fork()
     if pid == 0:
         # Don't raise an exception; it would be caught by the test harness.
@@ -281,6 +282,97 @@ class SocketServerTest(unittest.TestCase):
                                        socketserver.StreamRequestHandler)
 
 
+class ErrorHandlerTest(unittest.TestCase):
+    """Test that the servers pass normal exceptions from the handler to
+    handle_error(), and that exiting exceptions like SystemExit and
+    KeyboardInterrupt are not passed."""
+
+    def tearDown(self):
+        test.support.unlink(test.support.TESTFN)
+
+    def test_sync_handled(self):
+        BaseErrorTestServer(ValueError)
+        self.check_result(handled=True)
+
+    def test_sync_not_handled(self):
+        with self.assertRaises(SystemExit):
+            BaseErrorTestServer(SystemExit)
+        self.check_result(handled=False)
+
+    @unittest.skipUnless(threading, 'Threading required for this test.')
+    def test_threading_handled(self):
+        ThreadingErrorTestServer(ValueError)
+        self.check_result(handled=True)
+
+    @unittest.skipUnless(threading, 'Threading required for this test.')
+    def test_threading_not_handled(self):
+        ThreadingErrorTestServer(SystemExit)
+        self.check_result(handled=False)
+
+    @requires_forking
+    def test_forking_handled(self):
+        ForkingErrorTestServer(ValueError)
+        self.check_result(handled=True)
+
+    @requires_forking
+    def test_forking_not_handled(self):
+        ForkingErrorTestServer(SystemExit)
+        self.check_result(handled=False)
+
+    def check_result(self, handled):
+        with open(test.support.TESTFN) as log:
+            expected = 'Handler called\n' + 'Error handled\n' * handled
+            self.assertEqual(log.read(), expected)
+
+
+class BaseErrorTestServer(socketserver.TCPServer):
+    def __init__(self, exception):
+        self.exception = exception
+        super().__init__((HOST, 0), BadHandler)
+        with socket.create_connection(self.server_address):
+            pass
+        try:
+            self.handle_request()
+        finally:
+            self.server_close()
+        self.wait_done()
+
+    def handle_error(self, request, client_address):
+        with open(test.support.TESTFN, 'a') as log:
+            log.write('Error handled\n')
+
+    def wait_done(self):
+        pass
+
+
+class BadHandler(socketserver.BaseRequestHandler):
+    def handle(self):
+        with open(test.support.TESTFN, 'a') as log:
+            log.write('Handler called\n')
+        raise self.server.exception('Test error')
+
+
+class ThreadingErrorTestServer(socketserver.ThreadingMixIn,
+        BaseErrorTestServer):
+    def __init__(self, *pos, **kw):
+        self.done = threading.Event()
+        super().__init__(*pos, **kw)
+
+    def shutdown_request(self, *pos, **kw):
+        super().shutdown_request(*pos, **kw)
+        self.done.set()
+
+    def wait_done(self):
+        self.done.wait()
+
+
+class ForkingErrorTestServer(socketserver.ForkingMixIn, BaseErrorTestServer):
+    def wait_done(self):
+        [child] = self.active_children
+        os.waitpid(child, 0)
+        self.active_children.clear()
+
+
 class MiscTestCase(unittest.TestCase):
 
     def test_all(self):