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
This commit is contained in:
Trygve Aaberge 2014-08-14 01:55:51 +02:00
parent f0b66bdfcb
commit 265951bf00
3 changed files with 5 additions and 2 deletions

View File

@ -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)
====================

View File

@ -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:

View File

@ -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)