Rev 6054: One race can hide another... the exception may pass from the connection thread to the server thread so both need to be checked, with care. in file:///home/vila/src/bzr/bugs/869366-test-server-race/
Vincent Ladeuil
v.ladeuil+lp at free.fr
Fri Oct 7 14:49:00 UTC 2011
At file:///home/vila/src/bzr/bugs/869366-test-server-race/
------------------------------------------------------------
revno: 6054
revision-id: v.ladeuil+lp at free.fr-20111007144859-oqoe6td0sc2gim43
parent: v.ladeuil+lp at free.fr-20111007134112-seo2r1u8y0pf97a7
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: 869366-test-server-race
timestamp: Fri 2011-10-07 16:48:59 +0200
message:
One race can hide another... the exception may pass from the connection thread to the server thread so both need to be checked, with care.
-------------- next part --------------
=== modified file 'bzrlib/cethread.py'
--- a/bzrlib/cethread.py 2011-05-19 09:32:38 +0000
+++ b/bzrlib/cethread.py 2011-10-07 14:48:59 +0000
@@ -126,6 +126,8 @@
super(CatchingExceptionThread, self).run()
except:
self.exception = sys.exc_info()
+# print '\nthread %r, sync: %r see %r' % (
+# self, self.sync_event, self.exception[0],)
finally:
# Make sure the calling thread is released
self.sync_event.set()
=== modified file 'bzrlib/tests/test_server.py'
--- a/bzrlib/tests/test_server.py 2011-10-07 13:41:12 +0000
+++ b/bzrlib/tests/test_server.py 2011-10-07 14:48:59 +0000
@@ -452,6 +452,8 @@
if debug_threads():
sys.stderr.write('Client thread %s started\n' % (t.name,))
# If an exception occured during the thread start, it will get raised.
+ # In rare cases, an exception raised during the request processing may
+ # also get caught here (see http://pad.lv/869366)
t.pending_exception()
# The following methods are called by the main thread
=== modified file 'bzrlib/tests/test_test_server.py'
--- a/bzrlib/tests/test_test_server.py 2011-10-07 13:41:12 +0000
+++ b/bzrlib/tests/test_test_server.py 2011-10-07 14:48:59 +0000
@@ -169,22 +169,27 @@
self.assertRaises(CantConnect, server.stop_server)
def test_server_crash_while_responding(self):
- sync = threading.Event()
- sync.clear()
+ # We need to ensure the exception will be raised
+ will_raise = threading.Event()
+ will_raise.clear()
+ # We need to ensure the exception has been caught
+ caught = threading.Event()
+ caught.clear()
+# print 'will_raise: %r, caught: %r\n' % (will_raise, caught)
# The thread that will serve the client
self.connection_thread = None
- test = self # So the handler can set connection_thread
class FailToRespond(Exception):
pass
class FailingDuringResponseHandler(TCPConnectionHandler):
- def handle_connection(self):
- req = self.rfile.readline()
- # Capture the thread and make it use 'sync' so we can wait on
+ def handle_connection(request):
+ req = request.rfile.readline()
+ # Capture the thread and make it use 'caught' so we can wait on
# the even set when the exception is caught
- test.connection_thread = threading.currentThread()
- test.connection_thread.set_sync_event(sync)
+ self.connection_thread = threading.currentThread()
+ self.connection_thread.set_sync_event(caught)
+ will_raise.set()
raise FailToRespond()
server = self.get_server(
@@ -192,32 +197,48 @@
client = self.get_client()
client.connect((server.host, server.port))
client.write('ping\n')
- sync.wait()
+ # Wait for the connection to succeed and be processed
+ will_raise.wait()
+ # Wait for the exception to be caught
+ caught.wait()
# Check that the connection thread did catch the exception,
# http://pad.lv/869366 was wrongly checking the server thread which
# works for TestingTCPServer where the connection is handled in the
# same thread than the server one but is racy for
# TestingThreadingTCPServer where the server thread may be in a
# blocking accept() call.
- self.assertRaises(FailToRespond,
- self.connection_thread.pending_exception)
+ try:
+ self.connection_thread.pending_exception()
+ except FailToRespond:
+ # Great, the test succeeded
+ pass
+ else:
+ # If the exception is not in the connection thread anymore, it's in
+ # the server.
+ server.server.stopped.wait()
+ # The exception is available now
+ self.assertRaises(FailToRespond, server.pending_exception)
def test_exception_swallowed_while_serving(self):
- sync = threading.Event()
- sync.clear()
+ # We need to ensure the exception will be raised
+ will_raise = threading.Event()
+ will_raise.clear()
+ # We need to ensure the exception has been caught
+ caught = threading.Event()
+ caught.clear()
# The thread that will serve the client
self.connection_thread = None
- test = self # So the handler can set connection_thread
class CantServe(Exception):
pass
class FailingWhileServingConnectionHandler(TCPConnectionHandler):
def handle(request):
- # Capture the thread and make it use 'sync' so we can wait on
+ # Capture the thread and make it use 'caught' so we can wait on
# the even set when the exception is caught
- test.connection_thread = threading.currentThread()
- test.connection_thread.set_sync_event(sync)
+ self.connection_thread = threading.currentThread()
+ self.connection_thread.set_sync_event(caught)
+ will_raise.set()
raise CantServe()
server = self.get_server(
@@ -227,10 +248,15 @@
client = self.get_client()
# Connect to the server so the exception is raised there
client.connect((server.host, server.port))
- # Wait for the exception to propagate.
- sync.wait()
+ # Wait for the connection to succeed and be processed
+ will_raise.wait()
+ # Wait for the exception to be caught
+ caught.wait()
# The connection wasn't served properly but the exception should have
# been swallowed (see test_server_crash_while_responding remark about
# http://pad.lv/869366 explaining why we can't check the server thread
- # here).
+ # here). More precisely, the exception *has* been caught and captured
+ # but it is cleared when joining the thread (or trying to acquire the
+ # exception) and as such won't propagate to the server thread.
self.connection_thread.pending_exception()
+ server.pending_exception()
More information about the bazaar-commits
mailing list