diff --git a/docs/changelog.rst b/docs/changelog.rst index e6e3bcf4..f832716f 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -93,6 +93,10 @@ v0.20.0 (UNRELEASED) "database". If you insist on using a client that needs these commands change :confval:`mpd/command_blacklist`. +**HTTP frontend** + +- Prevent race condition in webservice broadcast from breaking the server. + **Audio** - Deprecated :meth:`mopidy.audio.Audio.emit_end_of_stream`. Pass a diff --git a/tests/http/test_handlers.py b/tests/http/test_handlers.py index 5803adaf..8bd82e11 100644 --- a/tests/http/test_handlers.py +++ b/tests/http/test_handlers.py @@ -40,45 +40,47 @@ class StaticFileHandlerTest(tornado.testing.AsyncHTTPTestCase): response.headers['Cache-Control'], 'no-cache') -class WebSocketHandlerTest(tornado.testing.AsyncHTTPTestCase): - def get_app(self): - self.core = mock.Mock() - return tornado.web.Application([ - (r'/ws/?', handlers.WebSocketHandler, {'core': self.core}) - ]) +# We aren't bothering with skipIf as then we would need to "backport" gen_test +if hasattr(tornado.websocket, 'websocket_connect'): + class WebSocketHandlerTest(tornado.testing.AsyncHTTPTestCase): + def get_app(self): + self.core = mock.Mock() + return tornado.web.Application([ + (r'/ws/?', handlers.WebSocketHandler, {'core': self.core}) + ]) - def connection(self): - url = self.get_url('/ws').replace('http', 'ws') - return tornado.websocket.websocket_connect(url, self.io_loop) + def connection(self): + url = self.get_url('/ws').replace('http', 'ws') + return tornado.websocket.websocket_connect(url, self.io_loop) - @tornado.testing.gen_test - def test_invalid_json_rpc_request_doesnt_crash_handler(self): - # An uncaught error would result in no message, so this is just a - # simplistic test to verify this. - conn = yield self.connection() - conn.write_message('invalid request') - message = yield conn.read_message() - self.assertTrue(message) + @tornado.testing.gen_test + def test_invalid_json_rpc_request_doesnt_crash_handler(self): + # An uncaught error would result in no message, so this is just a + # simplistic test to verify this. + conn = yield self.connection() + conn.write_message('invalid request') + message = yield conn.read_message() + self.assertTrue(message) - @tornado.testing.gen_test - def test_broadcast_makes_it_to_client(self): - conn = yield self.connection() - handlers.WebSocketHandler.broadcast('message') - message = yield conn.read_message() - self.assertEqual(message, 'message') + @tornado.testing.gen_test + def test_broadcast_makes_it_to_client(self): + conn = yield self.connection() + handlers.WebSocketHandler.broadcast('message') + message = yield conn.read_message() + self.assertEqual(message, 'message') - @tornado.testing.gen_test - def test_broadcast_to_client_that_just_closed_connection(self): - conn = yield self.connection() - conn.close() - handlers.WebSocketHandler.broadcast('message') + @tornado.testing.gen_test + def test_broadcast_to_client_that_just_closed_connection(self): + conn = yield self.connection() + conn.stream.close() + handlers.WebSocketHandler.broadcast('message') - @tornado.testing.gen_test - def test_broadcast_to_client_without_ws_connection_present(self): - yield self.connection() - # Tornado checks for ws_connection and raises WebSocketClosedError - # if it is missing, this test case simulates winning a race were - # this has happened but we have not yet been removed from clients. - for client in handlers.WebSocketHandler.clients: - client.ws_connection = None - handlers.WebSocketHandler.broadcast('message') + @tornado.testing.gen_test + def test_broadcast_to_client_without_ws_connection_present(self): + yield self.connection() + # Tornado checks for ws_connection and raises WebSocketClosedError + # if it is missing, this test case simulates winning a race were + # this has happened but we have not yet been removed from clients. + for client in handlers.WebSocketHandler.clients: + client.ws_connection = None + handlers.WebSocketHandler.broadcast('message')