From 265951bf0082bfa52073381a5fd9b5ab4c429bef Mon Sep 17 00:00:00 2001 From: Trygve Aaberge Date: Thu, 14 Aug 2014 01:55:51 +0200 Subject: [PATCH] network: disable_recv before telling actor to close connection As of d62ad96, when the connection can't receive more data from the client, it tells the actor to stop the connection and calls disable_recv(). The actor operates in it's own thread and when it stops the connection, disable_recv is being called again from a different thread. Since the actor is told to stop the connection before disable_recv is called, the two calls to disable_recv may happen simultaneously. This causes a race condition issue where both threads can reach past the check that recv_id is not None before either of them set it to None. If one of them set it to None before the other one tries to use it, an error is raised. This commit calls disable_recv before telling the actor to stop the connection. Since disable_recv is a blocking call, this ensures that recv_id is being set to None before the actor thread begins to stop the connection. Fixes #781 --- docs/changelog.rst | 3 +++ mopidy/utils/network.py | 2 +- tests/utils/network/test_connection.py | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index ec881a92..e242b989 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -21,6 +21,9 @@ Bug fix release. - Configuration: :option:`mopidy --config` now supports directories. +- Network: Fix a race condition where two threads could try to free the same + data simultaneously. (Fixes: :issue:`781`) + v0.19.3 (2014-08-03) ==================== diff --git a/mopidy/utils/network.py b/mopidy/utils/network.py index 11469b47..4ea25026 100644 --- a/mopidy/utils/network.py +++ b/mopidy/utils/network.py @@ -268,8 +268,8 @@ class Connection(object): return True if not data: - self.actor_ref.tell({'close': True}) self.disable_recv() + self.actor_ref.tell({'close': True}) return True try: diff --git a/tests/utils/network/test_connection.py b/tests/utils/network/test_connection.py index 7a25643f..7435353a 100644 --- a/tests/utils/network/test_connection.py +++ b/tests/utils/network/test_connection.py @@ -418,8 +418,8 @@ class ConnectionTest(unittest.TestCase): self.assertTrue(network.Connection.recv_callback( self.mock, sentinel.fd, gobject.IO_IN)) - self.mock.actor_ref.tell.assert_called_once_with({'close': True}) self.mock.disable_recv.assert_called_once_with() + self.mock.actor_ref.tell.assert_called_once_with({'close': True}) def test_recv_callback_recoverable_error(self): self.mock.sock = Mock(spec=socket.SocketType)