From 733754e13b52bce6687d5a88c1075c963d034722 Mon Sep 17 00:00:00 2001 From: Alexey Popravka Date: Wed, 19 Dec 2018 15:50:12 +0200 Subject: Add failing tests to show difference between protocol parsers on_disconnect implementation/behavior (related to #1085). When hiredis is installed and HiredisParser is used (implicitly), connection can not be securily shared between process forks. --- tests/test_multiprocessing.py | 127 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 127 insertions(+) create mode 100644 tests/test_multiprocessing.py diff --git a/tests/test_multiprocessing.py b/tests/test_multiprocessing.py new file mode 100644 index 0000000..8af1459 --- /dev/null +++ b/tests/test_multiprocessing.py @@ -0,0 +1,127 @@ +import pytest +import multiprocessing +import contextlib + +from redis.connection import Connection, ConnectionPool + + +@contextlib.contextmanager +def exit_callback(callback, *args): + try: + yield + finally: + callback(*args) + + +class TestMultiprocessing(object): + # Test connection sharing between forks. + # See issue #1085 for details. + + def test_connection(self): + conn = Connection() + assert conn.send_command('ping') is None + assert conn.read_response() == b'PONG' + + def target(conn): + assert conn.send_command('ping') is None + assert conn.read_response() == b'PONG' + conn.disconnect() + + proc = multiprocessing.Process(target=target, args=(conn,)) + proc.start() + proc.join(3) + assert proc.exitcode is 0 + + # Check that connection is still alive after fork process has exited. + conn.send_command('ping') + assert conn.read_response() == b'PONG' + + def test_close_connection_in_main(self): + conn = Connection() + assert conn.send_command('ping') is None + assert conn.read_response() == b'PONG' + + def target(conn, ev): + ev.wait() + assert conn.send_command('ping') is None + assert conn.read_response() == b'PONG' + + ev = multiprocessing.Event() + proc = multiprocessing.Process(target=target, args=(conn, ev)) + proc.start() + + conn.disconnect() + ev.set() + + proc.join(3) + assert proc.exitcode is 0 + + @pytest.mark.parametrize('max_connections', [1, 2, None]) + def test_pool(self, max_connections): + pool = ConnectionPool.from_url('redis://localhost', + max_connections=max_connections) + + conn = pool.get_connection('ping') + with exit_callback(pool.release, conn): + assert conn.send_command('ping') is None + assert conn.read_response() == b'PONG' + + def target(pool): + with exit_callback(pool.disconnect): + conn = pool.get_connection('ping') + with exit_callback(pool.release, conn): + assert conn.send_command('ping') is None + assert conn.read_response() == b'PONG' + + proc = multiprocessing.Process(target=target, args=(pool,)) + proc.start() + proc.join(3) + assert proc.exitcode is 0 + + # Check that connection is still alive after fork process has exited. + conn = pool.get_connection('ping') + with exit_callback(pool.release, conn): + conn.send_command('ping') + assert conn.read_response() == b'PONG' + + @pytest.mark.parametrize('max_connections', [1, 2, None]) + def test_close_pool_in_main(self, max_connections): + pool = ConnectionPool.from_url('redis://localhost', + max_connections=max_connections) + + conn = pool.get_connection('ping') + assert conn.send_command('ping') is None + assert conn.read_response() == b'PONG' + + def target(pool, disconnect_event): + conn = pool.get_connection('ping') + with exit_callback(pool.release, conn): + assert conn.send_command('ping') is None + assert conn.read_response() == b'PONG' + disconnect_event.wait() + assert conn.send_command('ping') is None + assert conn.read_response() == b'PONG' + + ev = multiprocessing.Event() + + proc = multiprocessing.Process(target=target, args=(pool, ev)) + proc.start() + + pool.disconnect() + ev.set() + proc.join(3) + assert proc.exitcode is 0 + + def test_redis(self, r): + assert r.ping() is True + + def target(redis): + assert redis.ping() is True + del redis + + proc = multiprocessing.Process(target=target, args=(r,)) + proc.start() + proc.join(3) + assert proc.exitcode is 0 + + assert r.ping() is True -- cgit v1.2.1 From 6020f43264209b2430a01fc3098d74bb142942a7 Mon Sep 17 00:00:00 2001 From: Alexey Popravka Date: Thu, 3 Jan 2019 13:13:52 +0200 Subject: update test to expect errors --- tests/test_multiprocessing.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/tests/test_multiprocessing.py b/tests/test_multiprocessing.py index 8af1459..dae35bc 100644 --- a/tests/test_multiprocessing.py +++ b/tests/test_multiprocessing.py @@ -3,6 +3,7 @@ import multiprocessing import contextlib from redis.connection import Connection, ConnectionPool +from redis.exceptions import ConnectionError @contextlib.contextmanager @@ -33,8 +34,9 @@ class TestMultiprocessing(object): assert proc.exitcode is 0 # Check that connection is still alive after fork process has exited. - conn.send_command('ping') - assert conn.read_response() == b'PONG' + with pytest.raises(ConnectionError): + assert conn.send_command('ping') is None + assert conn.read_response() == b'PONG' def test_close_connection_in_main(self): conn = Connection() @@ -54,7 +56,7 @@ class TestMultiprocessing(object): ev.set() proc.join(3) - assert proc.exitcode is 0 + assert proc.exitcode is 1 @pytest.mark.parametrize('max_connections', [1, 2, None]) def test_pool(self, max_connections): @@ -81,8 +83,9 @@ class TestMultiprocessing(object): # Check that connection is still alive after fork process has exited. conn = pool.get_connection('ping') with exit_callback(pool.release, conn): - conn.send_command('ping') - assert conn.read_response() == b'PONG' + with pytest.raises(ConnectionError): + assert conn.send_command('ping') is None + assert conn.read_response() == b'PONG' @pytest.mark.parametrize('max_connections', [1, 2, None]) def test_close_pool_in_main(self, max_connections): -- cgit v1.2.1 From a8bf82fc9edc0040062e5b3ee4c3074f67caaea1 Mon Sep 17 00:00:00 2001 From: Alexey Popravka Date: Thu, 3 Jan 2019 13:15:36 +0200 Subject: =?UTF-8?q?Make=20PythonParser's=20on=5Fdisconnect=20consistent=20?= =?UTF-8?q?with=20Hiredisparser=20and=20Connection=20=E2=80=94=20do=20not?= =?UTF-8?q?=20close=20socket=20on=20disconnect.=20Resolves=20#1085?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- redis/connection.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/redis/connection.py b/redis/connection.py index ea06241..7575b76 100755 --- a/redis/connection.py +++ b/redis/connection.py @@ -276,9 +276,7 @@ class PythonParser(BaseParser): def on_disconnect(self): "Called when the socket disconnects" - if self._sock is not None: - self._sock.close() - self._sock = None + self._sock = None if self._buffer is not None: self._buffer.close() self._buffer = None -- cgit v1.2.1