fix: Don't overwrite base logging Handler class lock var

Author: Stephen Hawkins

logdna/logdna.py

@@ -74,7 +74,7 @@ def __init__(self, key, options={}):
             max_workers=self.max_concurrent_requests)
 
         self.setLevel(logging.DEBUG)
-        self.lock = threading.RLock()
+        self._lock = threading.RLock()
 
         self.flusher = None
 
@@ -100,7 +100,7 @@ def buffer_log(self, message):
 
     def buffer_log_sync(self, message):
         # Attempt to acquire lock to write to buffer
-        if self.lock.acquire(blocking=True):
+        if self._lock.acquire(blocking=True):
             try:
                 msglen = len(message['line'])
                 if self.buf_size + msglen < self.buf_retention_limit:
@@ -119,7 +119,7 @@ def buffer_log_sync(self, message):
             except Exception as e:
                 self.internalLogger.exception(f'Error in buffer_log_sync: {e}')
             finally:
-                self.lock.release()
+                self._lock.release()
 
     def flush(self):
         self.schedule_flush_sync()
@@ -137,18 +137,18 @@ def schedule_flush_sync(self, should_block=False):
 
     def try_lock_and_do_flush_request(self, should_block=False):
         local_buf = []
-        if self.lock.acquire(blocking=should_block):
+        if self._lock.acquire(blocking=should_block):
             if not self.buf:
                 self.close_flusher()
-                self.lock.release()
+                self._lock.release()
                 return
 
             local_buf = self.buf.copy()
             self.buf.clear()
             self.buf_size = 0
             if local_buf:
                 self.close_flusher()
-            self.lock.release()
+            self._lock.release()
 
         if local_buf:
             self.try_request(local_buf)
@@ -351,4 +351,5 @@ def close(self):
         if self.request_thread_pool:
             self.request_thread_pool.shutdown(wait=True)
             self.request_thread_pool = None
+
         logging.Handler.close(self)

tests/test_logger.py

@@ -255,15 +255,27 @@ def test_close(self):
         handler = LogDNAHandler(LOGDNA_API_KEY, sample_options)
         close_flusher_mock = unittest.mock.Mock()
         close_flusher_mock.side_effect = handler.close_flusher
-        handler.close_flusher = close_flusher_mock
         handler.schedule_flush_sync = unittest.mock.Mock()
+        handler.close_flusher = close_flusher_mock
         handler.close()
         handler.close_flusher.assert_called_once_with()
         handler.schedule_flush_sync.assert_called_once_with(
             should_block=True)
         self.assertIsNone(handler.worker_thread_pool)
         self.assertIsNone(handler.request_thread_pool)
 
+    # These should be separate objects, since there is already
+    # a variable in the base class named self.lock. We want
+    # to make sure that a separate lock is created for the
+    # locking semantics of the LogDNA Handler
+    def test_lock_var_separate_from_local_lock_var(self):
+        handler = LogDNAHandler(LOGDNA_API_KEY, sample_options)
+        self.assertIsNotNone(handler)
+
+        # Test that we did not replace the base class' instance var.
+        self.assertIsNotNone(handler._lock)
+        self.assertIsNotNone(handler.lock)
+
     def test_flush(self):
         handler = LogDNAHandler(LOGDNA_API_KEY, sample_options)
         handler.worker_thread_pool = MockThreadPoolExecutor()