Repository: scoutapp/scout_apm_python
Pull request #448 (2020-01-03 16:56:17)

Extract SingletonThread class from SamplersThread
This allows the functionality to be tested independently and used elsewhere (in future CoreAgentSocket refactor).


Patch:
diff --git a/src/scout_apm/core/samplers/thread.py b/src/scout_apm/core/samplers/thread.py
index 8861389..f89421d 100644
--- a/src/scout_apm/core/samplers/thread.py
+++ b/src/scout_apm/core/samplers/thread.py
@@ -10,44 +10,15 @@ from scout_apm.core.commands import ApplicationEvent
 from scout_apm.core.samplers.cpu import Cpu
 from scout_apm.core.samplers.memory import Memory
 from scout_apm.core.socket import CoreAgentSocket
+from scout_apm.core.threading import SingletonThread
 
 logger = logging.getLogger(__name__)
 
 
-class SamplersThread(threading.Thread):
-    _instance = None
+class SamplersThread(SingletonThread):
     _instance_lock = threading.Lock()
     _stop_event = threading.Event()
 
-    @classmethod
-    def ensure_started(cls):
-        with cls._instance_lock:
-            if cls._instance is None:
-                cls._instance = cls()
-                cls._instance.start()
-
-    @classmethod
-    def ensure_stopped(cls):
-        with cls._instance_lock:
-            if cls._instance is None:
-                # Nothing to stop
-                return
-            elif not cls._instance.is_alive():
-                # Thread died
-                cls._instance = None
-                return
-
-            # Signal stopping
-            cls._stop_event.set()
-            cls._instance.join()
-
-            cls._instance = None
-            cls._stop_event.clear()
-
-    def __init__(self, *args, **kwargs):
-        super(SamplersThread, self).__init__(*args, **kwargs)
-        self.daemon = True
-
     def run(self):
         logger.debug("Starting Samplers.")
         instances = [Cpu(), Memory()]
diff --git a/src/scout_apm/core/threading.py b/src/scout_apm/core/threading.py
new file mode 100644
index 0000000..90b7ab0
--- /dev/null
+++ b/src/scout_apm/core/threading.py
@@ -0,0 +1,59 @@
+# coding=utf-8
+from __future__ import absolute_import, division, print_function, unicode_literals
+
+import threading
+
+
+class SingletonThread(threading.Thread):
+
+    _instance = None
+    # Copy these variables into subclasses to avoid sharing:
+    # (Would use __init_subclass__() but Python 2 doesn't support it and using
+    # metaclasses to achieve the same is a lot of faff.)
+    # _instance_lock = threading.Lock()
+    # _stop_event = threading.Event()
+
+    @classmethod
+    def ensure_started(cls):
+        instance = cls._instance
+        if instance is not None and instance.is_alive():
+            # No need to grab the lock
+            return
+        with cls._instance_lock:
+            if cls._instance is None or not cls._instance.is_alive():
+                cls._instance = cls()
+                cls._instance.start()
+            return
+
+    @classmethod
+    def ensure_stopped(cls):
+        if cls._instance is None:
+            # No need to grab the lock
+            return
+        with cls._instance_lock:
+            if cls._instance is None:
+                # Nothing to stop
+                return
+            elif not cls._instance.is_alive():
+                # Thread died
+                cls._instance = None
+                return
+
+            # Signal stopping
+            cls._stop_event.set()
+            cls._on_stop()
+            cls._instance.join()
+
+            cls._instance = None
+            cls._stop_event.clear()
+
+    @classmethod
+    def _on_stop(cls):
+        """
+        Hook to allow subclasses to add extra behaviour to stopping.
+        """
+        pass
+
+    def __init__(self, *args, **kwargs):
+        super(SingletonThread, self).__init__(*args, **kwargs)
+        self.daemon = True


Test patch:
diff --git a/tests/unit/core/samplers/test_thread.py b/tests/unit/core/samplers/test_thread.py
index 6cff924..59abab0 100644
--- a/tests/unit/core/samplers/test_thread.py
+++ b/tests/unit/core/samplers/test_thread.py
@@ -22,7 +22,6 @@ def test_ensure_started(caplog):
     SamplersThread.ensure_started()
     time.sleep(0.001)
 
-    assert SamplersThread._instance is not None
     assert caplog.record_tuples[0] == (
         "scout_apm.core.samplers.thread",
         10,
@@ -35,7 +34,6 @@ def test_ensure_stopped(caplog):
     time.sleep(0.001)
     SamplersThread.ensure_stopped()
 
-    assert SamplersThread._instance is None
     assert caplog.record_tuples[-1] == (
         "scout_apm.core.samplers.thread",
         10,
diff --git a/tests/unit/core/test_metadata.py b/tests/unit/core/test_metadata.py
index 4515105..3b69d99 100644
--- a/tests/unit/core/test_metadata.py
+++ b/tests/unit/core/test_metadata.py
@@ -1,6 +1,8 @@
 # coding=utf-8
 from __future__ import absolute_import, division, print_function, unicode_literals
 
+import sys
+
 import pytest
 
 from scout_apm.core.metadata import report_app_metadata
@@ -25,8 +27,12 @@ def test_report_app_metadata(send):
 
 
 @mock.patch("scout_apm.core.socket.CoreAgentSocket.send")
-def test_report_app_metadata_no_pkg_resources(send):
-    with pretend_package_unavailable("pkg_resources"):
+def test_report_app_metadata_no_importlib_metadata(send):
+    if sys.version_info >= (3, 8):
+        module_name = "importlib"
+    else:
+        module_name = "importlib_metadata"
+    with pretend_package_unavailable(module_name):
         report_app_metadata()
 
     assert send.call_count == 1
diff --git a/tests/unit/core/test_threading.py b/tests/unit/core/test_threading.py
new file mode 100644
index 0000000..faefec3
--- /dev/null
+++ b/tests/unit/core/test_threading.py
@@ -0,0 +1,89 @@
+# coding=utf-8
+from __future__ import absolute_import, division, print_function, unicode_literals
+
+import threading
+
+import pytest
+
+from scout_apm.core.threading import SingletonThread
+
+
+class ExampleThread(SingletonThread):
+    _instance_lock = threading.Lock()
+    _stop_event = threading.Event()
+
+    @classmethod
+    def _on_stop(cls):
+        cls._on_stop_called = True
+
+    def run(self):
+        while not self._stop_event.wait(timeout=1):
+            pass
+
+
+@pytest.fixture(autouse=True)
+def ensure_test_thread_stopped():
+    try:
+        yield
+    finally:
+        ExampleThread.ensure_stopped()
+
+
+def test_ensure_started():
+    ExampleThread.ensure_started()
+
+    assert isinstance(ExampleThread._instance, ExampleThread)
+
+
+def test_ensure_started_twice_idempotent():
+    ExampleThread.ensure_started()
+    instance_1 = ExampleThread._instance
+    ExampleThread.ensure_started()
+    instance_2 = ExampleThread._instance
+
+    assert isinstance(instance_1, ExampleThread)
+    assert isinstance(instance_2, ExampleThread)
+    assert instance_1 is instance_2
+
+
+def test_ensure_started_whilst_holding_lock():
+    ExampleThread.ensure_started()
+    instance_1 = ExampleThread._instance
+    with ExampleThread._instance_lock:
+        # Imitate another thread in the process of finishing starting -
+        # ensure_started() should still return due to its early check
+        ExampleThread.ensure_started()
+        instance_2 = ExampleThread._instance
+
+    assert instance_1 is instance_2
+
+
+def test_restart_makes_new_instance():
+    ExampleThread.ensure_started()
+    instance_1 = ExampleThread._instance
+    ExampleThread.ensure_stopped()
+    ExampleThread.ensure_started()
+    instance_2 = ExampleThread._instance
+
+    assert instance_1 is not instance_2
+
+
+def test_ensure_stopped():
+    ExampleThread.ensure_stopped()
+    assert ExampleThread._instance is None
+    assert not ExampleThread._stop_event.is_set()
+
+
+def test_ensure_stopped_calls_on_stop():
+    ExampleThread._on_stop_called = False
+    ExampleThread.ensure_started()
+    ExampleThread.ensure_stopped()
+
+    assert ExampleThread._on_stop_called
+
+
+def test_ensure_stopped_whilst_holding_lock():
+    with ExampleThread._instance_lock:
+        # Imitate another thread in the process of finishign stopping -
+        # ensure_stopped should still return due to its early check
+        ExampleThread.ensure_stopped()