[Python-checkins] cpython: Issue #23430: Stop socketserver from catching SystemExit etc from handlers
martin.panter
python-checkins at python.org
Sun Feb 21 06:01:40 EST 2016
https://hg.python.org/cpython/rev/d500d1a9615f
changeset: 100285:d500d1a9615f
user: Martin Panter <vadmium+py at gmail.com>
date: Sun Feb 21 08:49:56 2016 +0000
summary:
Issue #23430: Stop socketserver from catching SystemExit etc from handlers
Also make handle_error() consistently output to stderr, and fix the
documentation.
files:
Doc/library/socketserver.rst | 6 +-
Doc/whatsnew/3.6.rst | 11 ++-
Lib/socketserver.py | 31 ++++---
Lib/test/test_socketserver.py | 92 +++++++++++++++++++++++
Misc/NEWS | 6 +
5 files changed, 131 insertions(+), 15 deletions(-)
diff --git a/Doc/library/socketserver.rst b/Doc/library/socketserver.rst
--- a/Doc/library/socketserver.rst
+++ b/Doc/library/socketserver.rst
@@ -304,7 +304,11 @@
This function is called if the :meth:`~BaseRequestHandler.handle`
method of a :attr:`RequestHandlerClass` instance raises
an exception. The default action is to print the traceback to
- standard output and continue handling further requests.
+ standard error and continue handling further requests.
+
+ .. versionchanged:: 3.6
+ Now only called for exceptions derived from the :exc:`Exception`
+ class.
.. method:: handle_timeout()
diff --git a/Doc/whatsnew/3.6.rst b/Doc/whatsnew/3.6.rst
--- a/Doc/whatsnew/3.6.rst
+++ b/Doc/whatsnew/3.6.rst
@@ -334,7 +334,16 @@
* When a relative import is performed and no parent package is known, then
:exc:`ImportError` will be raised. Previously, :exc:`SystemError` could be
- raised. (Contribute by Brett Cannon in :issue:`18018`.)
+ raised. (Contributed by Brett Cannon in :issue:`18018`.)
+
+* Servers based on the :mod:`socketserver` module, including those
+ defined in :mod:`http.server`, :mod:`xmlrpc.server` and
+ :mod:`wsgiref.simple_server`, now only catch exceptions derived
+ from :exc:`Exception`. Therefore if a request handler raises
+ an exception like :exc:`SystemExit` or :exc:`KeyboardInterrupt`,
+ :meth:`~socketserver.BaseServer.handle_error` is no longer called, and
+ the exception will stop a single-threaded server. (Contributed by
+ Martin Panter in :issue:`23430`.)
Changes in the C API
diff --git a/Lib/socketserver.py b/Lib/socketserver.py
--- a/Lib/socketserver.py
+++ b/Lib/socketserver.py
@@ -132,6 +132,7 @@
import selectors
import os
import errno
+import sys
try:
import threading
except ImportError:
@@ -316,9 +317,12 @@
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 @@
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 @@
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 @@
"""
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):
diff --git a/Lib/test/test_socketserver.py b/Lib/test/test_socketserver.py
--- a/Lib/test/test_socketserver.py
+++ b/Lib/test/test_socketserver.py
@@ -58,6 +58,7 @@
@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 @@
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):
diff --git a/Misc/NEWS b/Misc/NEWS
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -196,6 +196,12 @@
the connected socket) when verify_request() returns false. Patch by Aviv
Palivoda.
+- Issue #23430: Change the socketserver module to only catch exceptions
+ raised from a request handler that are derived from Exception (instead of
+ BaseException). Therefore SystemExit and KeyboardInterrupt no longer
+ trigger the handle_error() method, and will now to stop a single-threaded
+ server.
+
- Issue #25939: On Windows open the cert store readonly in ssl.enum_certificates.
- Issue #25995: os.walk() no longer uses FDs proportional to the tree depth.
--
Repository URL: https://hg.python.org/cpython
More information about the Python-checkins
mailing list