diff --git a/docs/changelog.rst b/docs/changelog.rst index 6cf8ca15..ebc42c44 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -23,6 +23,8 @@ Bug fix release. where the scanner hangs on non-audio files like video. The scanner will now also let us know if we found any decodeable audio. (Fixes: :issue:`726`) +- HTTP: Fix threading bug that would cause duplicate delivery of WS messages. + v1.0.0 (2015-03-25) =================== diff --git a/mopidy/http/handlers.py b/mopidy/http/handlers.py index a5baf992..4f4b5988 100644 --- a/mopidy/http/handlers.py +++ b/mopidy/http/handlers.py @@ -5,6 +5,7 @@ import os import socket import tornado.escape +import tornado.ioloop import tornado.web import tornado.websocket @@ -65,6 +66,19 @@ def make_jsonrpc_wrapper(core_actor): ) +def _send_broadcast(client, msg): + # We could check for client.ws_connection, but we don't really + # care why the broadcast failed, we just want the rest of them + # to succeed, so catch everything. + try: + client.write_message(msg) + except Exception as e: + error_msg = encoding.locale_decode(e) + logger.debug('Broadcast of WebSocket message to %s failed: %s', + client.request.remote_ip, error_msg) + # TODO: should this do the same cleanup as the on_message code? + + class WebSocketHandler(tornado.websocket.WebSocketHandler): # XXX This set is shared by all WebSocketHandler objects. This isn't @@ -74,17 +88,12 @@ class WebSocketHandler(tornado.websocket.WebSocketHandler): @classmethod def broadcast(cls, msg): + # This can be called from outside the Tornado ioloop, so we need to + # safely cross the thread boundary by adding a callback to the loop. + loop = tornado.ioloop.IOLoop.current() for client in cls.clients: - # We could check for client.ws_connection, but we don't really - # care why the broadcast failed, we just want the rest of them - # to succeed, so catch everything. - try: - client.write_message(msg) - except Exception as e: - error_msg = encoding.locale_decode(e) - logger.debug('Broadcast of WebSocket message to %s failed: %s', - client.request.remote_ip, error_msg) - # TODO: should this do the same cleanup as the on_message code? + # One callback per client to keep time we hold up the loop short + loop.add_callback(_send_broadcast, client, msg) def initialize(self, core): self.jsonrpc = make_jsonrpc_wrapper(core)