Repository: pika/pika

Issue:
BlockingConnection heartbeat attribute can be undefined
I've been testing connection recovery docs and managed to run into the following exceptions several times:

```
Traceback (most recent call last):
  File "./blocking_consumer_recovery2.py", line 31, in <module>
    channel.start_consuming()
  File "/Users/antares/Development/RabbitMQ/pika.git/pika/adapters/blocking_connection.py", line 1878, in start_consuming
    self._process_data_events(time_limit=None)
  File "/Users/antares/Development/RabbitMQ/pika.git/pika/adapters/blocking_connection.py", line 2040, in _process_data_events
    self.connection.process_data_events(time_limit=time_limit)
  File "/Users/antares/Development/RabbitMQ/pika.git/pika/adapters/blocking_connection.py", line 814, in process_data_events
    self._flush_output(common_terminator)
  File "/Users/antares/Development/RabbitMQ/pika.git/pika/adapters/blocking_connection.py", line 525, in _flush_output
    self._impl.ioloop.process_timeouts()
  File "/Users/antares/Development/RabbitMQ/pika.git/pika/adapters/select_connection.py", line 462, in process_timeouts
    self._timer.process_timeouts()
  File "/Users/antares/Development/RabbitMQ/pika.git/pika/adapters/select_connection.py", line 297, in process_timeouts
    timeout.callback()
  File "/Users/antares/Development/RabbitMQ/pika.git/pika/heartbeat.py", line 104, in send_and_check
    self._start_timer()
  File "/Users/antares/Development/RabbitMQ/pika.git/pika/heartbeat.py", line 167, in _start_timer
    if self.active:
  File "/Users/antares/Development/RabbitMQ/pika.git/pika/heartbeat.py", line 53, in active
    return self._connection.heartbeat is self
AttributeError: 'SelectConnection' object has no attribute 'heartbeat'
```

unfortunately I don't have a 100% clear idea of how to reproduce this but all I did was restarting down nodes cleanly. At some point during recovery attempt the above exception would make the process exit. Perhaps the code that checks for `self._connection.heartbeat` should be more defensive.

Discussion:

Patch:
diff --git a/README.rst b/README.rst
index a7a52be..1fcb7c7 100644
--- a/README.rst
+++ b/README.rst
@@ -12,9 +12,9 @@ extensions.
 - Python 2.7 and 3.4+ are supported.
 
 - Since threads aren't appropriate to every situation, it doesn't
-  require threads. It takes care not to forbid them, either. The same
-  goes for greenlets, callbacks, continuations and generators. It is
-  not necessarily thread-safe however, and your mileage will vary.
+  require threads. Pika core takes care not to forbid them, either. The same
+  goes for greenlets, callbacks, continuations, and generators. An instance of
+  Pika's built-in connection adapters is not thread-safe, however.
 
 - People may be using direct sockets, plain old `select()`,
   or any of the wide variety of ways of getting network events to and from a
diff --git a/pika/heartbeat.py b/pika/heartbeat.py
index af6d93c..a3822a4 100644
--- a/pika/heartbeat.py
+++ b/pika/heartbeat.py
@@ -50,7 +50,7 @@ class HeartbeatChecker(object):
         :rtype True
 
         """
-        return self._connection.heartbeat is self
+        return self._connection._heartbeat_checker is self
 
     @property
     def bytes_received_on_connection(self):


Test patch:
diff --git a/tests/unit/heartbeat_tests.py b/tests/unit/heartbeat_tests.py
index c9e8b63..2ff98a7 100644
--- a/tests/unit/heartbeat_tests.py
+++ b/tests/unit/heartbeat_tests.py
@@ -20,15 +20,39 @@ import pika.exceptions
 # pylint: disable=C0103
 
 
+class ConstructableConnection(connection.Connection):
+    """Adds dummy overrides for `Connection`'s abstract methods so
+    that we can instantiate and test it.
+
+    """
+    def _adapter_connect_stream(self):
+        pass
+
+    def _adapter_disconnect_stream(self):
+        raise NotImplementedError
+
+    def add_timeout(self, deadline, callback):
+        raise NotImplementedError
+
+    def remove_timeout(self, timeout_id):
+        raise NotImplementedError
+
+    def _adapter_emit_data(self, data):
+        raise NotImplementedError
+
+    def _adapter_get_write_buffer_size(self):
+        raise NotImplementedError
+
+
 class HeartbeatTests(unittest.TestCase):
 
     INTERVAL = 5
 
     def setUp(self):
-        self.mock_conn = mock.Mock(spec=connection.Connection)
+        self.mock_conn = mock.Mock(spec_set=ConstructableConnection())
         self.mock_conn.bytes_received = 100
         self.mock_conn.bytes_sent = 100
-        self.mock_conn.heartbeat = mock.Mock(spec=heartbeat.HeartbeatChecker)
+        self.mock_conn._heartbeat_checker = mock.Mock(spec=heartbeat.HeartbeatChecker)
         self.obj = heartbeat.HeartbeatChecker(self.mock_conn, self.INTERVAL)
 
     def tearDown(self):
@@ -65,11 +89,11 @@ class HeartbeatTests(unittest.TestCase):
         timer.assert_called_once_with()
 
     def test_active_true(self):
-        self.mock_conn.heartbeat = self.obj
+        self.mock_conn._heartbeat_checker = self.obj
         self.assertTrue(self.obj.active)
 
     def test_active_false(self):
-        self.mock_conn.heartbeat = mock.Mock()
+        self.mock_conn._heartbeat_checker = mock.Mock()
         self.assertFalse(self.obj.active)
 
     def test_bytes_received_on_connection(self):
@@ -178,7 +202,7 @@ class HeartbeatTests(unittest.TestCase):
 
     @mock.patch('pika.heartbeat.HeartbeatChecker._setup_timer')
     def test_start_timer_active(self, setup_timer):
-        self.mock_conn.heartbeat = self.obj
+        self.mock_conn._heartbeat_checker = self.obj
         self.obj._start_timer()
         self.assertTrue(setup_timer.called)