From: Antoine Pitrou Date: Sun, 25 Apr 2010 22:26:08 +0000 (+0000) Subject: Merged revisions 80487,80489 via svnmerge from X-Git-Tag: v3.1.3rc1~865 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=e3123915297da3e87f6d62c0e0788a6b5421a5cf;p=python Merged revisions 80487,80489 via svnmerge from svn+ssh://pythondev@svn.python.org/python/branches/py3k ................ r80487 | antoine.pitrou | 2010-04-26 00:01:43 +0200 (lun., 26 avril 2010) | 12 lines Merged revisions 80484 via svnmerge from svn+ssh://pythondev@svn.python.org/python/trunk ........ r80484 | antoine.pitrou | 2010-04-25 23:40:32 +0200 (dim., 25 avril 2010) | 6 lines Issue #2302: Fix a race condition in SocketServer.BaseServer.shutdown, where the method could block indefinitely if called just before the event loop started running. This also fixes the occasional freezes witnessed in test_httpservers. ........ ................ r80489 | antoine.pitrou | 2010-04-26 00:19:43 +0200 (lun., 26 avril 2010) | 9 lines Merged revisions 80480 via svnmerge from svn+ssh://pythondev@svn.python.org/python/trunk ........ r80480 | antoine.pitrou | 2010-04-25 23:15:50 +0200 (dim., 25 avril 2010) | 3 lines Replace a Lock with a better suited Event. ........ ................ --- diff --git a/Lib/socketserver.py b/Lib/socketserver.py index 92adbcfaf5..3d32c3ea19 100644 --- a/Lib/socketserver.py +++ b/Lib/socketserver.py @@ -197,7 +197,7 @@ class BaseServer: self.server_address = server_address self.RequestHandlerClass = RequestHandlerClass self.__is_shut_down = threading.Event() - self.__serving = False + self.__shutdown_request = False def server_activate(self): """Called by constructor to activate the server. @@ -214,17 +214,19 @@ class BaseServer: self.timeout. If you need to do periodic tasks, do them in another thread. """ - self.__serving = True self.__is_shut_down.clear() - while self.__serving: - # XXX: Consider using another file descriptor or - # connecting to the socket to wake this up instead of - # polling. Polling reduces our responsiveness to a - # shutdown request and wastes cpu at all other times. - r, w, e = select.select([self], [], [], poll_interval) - if r: - self._handle_request_noblock() - self.__is_shut_down.set() + try: + while not self.__shutdown_request: + # XXX: Consider using another file descriptor or + # connecting to the socket to wake this up instead of + # polling. Polling reduces our responsiveness to a + # shutdown request and wastes cpu at all other times. + r, w, e = select.select([self], [], [], poll_interval) + if self in r: + self._handle_request_noblock() + finally: + self.__shutdown_request = False + self.__is_shut_down.set() def shutdown(self): """Stops the serve_forever loop. @@ -233,7 +235,7 @@ class BaseServer: serve_forever() is running in another thread, or it will deadlock. """ - self.__serving = False + self.__shutdown_request = True self.__is_shut_down.wait() # The distinction between handling, getting, processing and diff --git a/Lib/test/test_httpservers.py b/Lib/test/test_httpservers.py index 4653f4c458..aa1e48c741 100644 --- a/Lib/test/test_httpservers.py +++ b/Lib/test/test_httpservers.py @@ -34,14 +34,14 @@ class TestServerThread(threading.Thread): threading.Thread.__init__(self) self.request_handler = request_handler self.test_object = test_object - self.test_object.lock.acquire() def run(self): self.server = HTTPServer(('', 0), self.request_handler) self.test_object.PORT = self.server.socket.getsockname()[1] - self.test_object.lock.release() + self.test_object.server_started.set() + self.test_object = None try: - self.server.serve_forever() + self.server.serve_forever(0.05) finally: self.server.server_close() @@ -51,13 +51,12 @@ class TestServerThread(threading.Thread): class BaseTestCase(unittest.TestCase): def setUp(self): - self.lock = threading.Lock() + self.server_started = threading.Event() self.thread = TestServerThread(self, self.request_handler) self.thread.start() - self.lock.acquire() + self.server_started.wait() def tearDown(self): - self.lock.release() self.thread.stop() def request(self, uri, method='GET', body=None, headers={}): diff --git a/Lib/test/test_socketserver.py b/Lib/test/test_socketserver.py index 43ce55c37a..0ab95d18d3 100644 --- a/Lib/test/test_socketserver.py +++ b/Lib/test/test_socketserver.py @@ -243,6 +243,30 @@ class SocketServerTest(unittest.TestCase): # socketserver.DatagramRequestHandler, # self.dgram_examine) + def test_shutdown(self): + # Issue #2302: shutdown() should always succeed in making an + # other thread leave serve_forever(). + class MyServer(socketserver.TCPServer): + pass + + class MyHandler(socketserver.StreamRequestHandler): + pass + + threads = [] + for i in range(20): + s = MyServer((HOST, 0), MyHandler) + t = threading.Thread( + name='MyServer serving', + target=s.serve_forever, + kwargs={'poll_interval':0.01}) + t.daemon = True # In case this function raises. + threads.append((t, s)) + for t, s in threads: + t.start() + s.shutdown() + for t, s in threads: + t.join() + def test_main(): if imp.lock_held(): diff --git a/Misc/NEWS b/Misc/NEWS index dd02c3d1bd..ae87ebea38 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -40,6 +40,11 @@ Core and Builtins Library ------- +- Issue #2302: Fix a race condition in SocketServer.BaseServer.shutdown, + where the method could block indefinitely if called just before the + event loop started running. This also fixes the occasional freezes + witnessed in test_httpservers. + - Issue #8524: When creating an SSL socket, the timeout value of the original socket wasn't retained (instead, a socket with a positive timeout would be turned into a non-blocking SSL socket).