diff options
author | Matus Valo <matusvalo@users.noreply.github.com> | 2018-12-29 19:42:21 +0100 |
---|---|---|
committer | Asif Saif Uddin <auvipy@gmail.com> | 2018-12-30 00:42:21 +0600 |
commit | 938f8a34bc6608d9b8889958828ea2d96ebdf8ed (patch) | |
tree | 3a2d19b12243bb857d203b85a15c2e0e761b99bd | |
parent | a7dd9d4e3d8e1a9db8f13f6feffeabd903988513 (diff) | |
download | py-amqp-938f8a34bc6608d9b8889958828ea2d96ebdf8ed.tar.gz |
Don't revive channel when connection is closing (#220)
* Don't revive channel when connection is closing
* When connection is closing dont raise error when Channel.Close method is received
* Added unitests
* Fix flake8
* Fix typo
-rw-r--r-- | amqp/abstract_channel.py | 1 | ||||
-rw-r--r-- | amqp/channel.py | 11 | ||||
-rw-r--r-- | amqp/connection.py | 2 | ||||
-rw-r--r-- | t/integration/test_integration.py | 29 | ||||
-rw-r--r-- | t/unit/test_channel.py | 1 |
5 files changed, 40 insertions, 4 deletions
diff --git a/amqp/abstract_channel.py b/amqp/abstract_channel.py index 138d472..5fe6611 100644 --- a/amqp/abstract_channel.py +++ b/amqp/abstract_channel.py @@ -22,6 +22,7 @@ class AbstractChannel(object): """ def __init__(self, connection, channel_id): + self.is_closing = False self.connection = connection self.channel_id = channel_id connection.channels[channel_id] = self diff --git a/amqp/channel.py b/amqp/channel.py index c829a2c..a0c686f 100644 --- a/amqp/channel.py +++ b/amqp/channel.py @@ -219,12 +219,14 @@ class Channel(AbstractChannel): if is_closed: return + self.is_closing = True return self.send_method( spec.Channel.Close, argsig, (reply_code, reply_text, method_sig[0], method_sig[1]), wait=spec.Channel.CloseOk, ) finally: + self.is_closing = False self.connection = None def _on_close(self, reply_code, reply_text, class_id, method_id): @@ -274,10 +276,11 @@ class Channel(AbstractChannel): is the ID of the method. """ self.send_method(spec.Channel.CloseOk) - self._do_revive() - raise error_for_code( - reply_code, reply_text, (class_id, method_id), ChannelError, - ) + if not self.connection.is_closing: + self._do_revive() + raise error_for_code( + reply_code, reply_text, (class_id, method_id), ChannelError, + ) def _on_close_ok(self): """Confirm a channel close. diff --git a/amqp/connection.py b/amqp/connection.py index 6e47798..5af06e1 100644 --- a/amqp/connection.py +++ b/amqp/connection.py @@ -569,12 +569,14 @@ class Connection(AbstractChannel): return try: + self.is_closing = True return self.send_method( spec.Connection.Close, argsig, (reply_code, reply_text, method_sig[0], method_sig[1]), wait=spec.Connection.CloseOk, ) except (OSError, IOError, SSLError): + self.is_closing = False # close connection self.collect() raise diff --git a/t/integration/test_integration.py b/t/integration/test_integration.py index 2969d2b..2957656 100644 --- a/t/integration/test_integration.py +++ b/t/integration/test_integration.py @@ -281,6 +281,35 @@ class test_channel: ) assert ch.is_open is False + def test_received_channel_Close_during_connection_close(self): + # This test verifies that library handles correctly closing channel + # during closing of connection: + # 1. User requests closing connection - client sends Connection.Close + # 2. Broker requests closing Channel - client receives Channel.Close + # 3. Broker sends Connection.CloseOk + # see GitHub issue #218 + conn = Connection() + with patch.object(conn, 'Transport') as transport_mock: + handshake(conn, transport_mock) + channel_id = 1 + create_channel(channel_id, conn, transport_mock) + # Replies sent by broker + transport_mock().read_frame.side_effect = [ + # Inject close methods + ret_factory( + spec.Channel.Close, + channel=channel_id, + args=(1, False), + arg_format='Lb' + ), + ret_factory( + spec.Connection.CloseOk, + args=(1, False), + arg_format='Lb' + ) + ] + conn.close() + @pytest.mark.parametrize("method, callback", channel_testdata) def test_channel_methods(self, method, callback): # Test verifying that proper Channel callback is called when diff --git a/t/unit/test_channel.py b/t/unit/test_channel.py index 173e0d7..7e7edc6 100644 --- a/t/unit/test_channel.py +++ b/t/unit/test_channel.py @@ -17,6 +17,7 @@ class test_Channel: @pytest.fixture(autouse=True) def setup_conn(self): self.conn = MagicMock(name='connection') + self.conn.is_closing = False self.conn.channels = {} self.conn._get_free_channel_id.return_value = 2 self.c = Channel(self.conn, 1) |