From b7a37322e159a643c33cd21a8a06f627b7204234 Mon Sep 17 00:00:00 2001 From: Walter Boring Date: Tue, 24 Mar 2026 11:54:29 -0400 Subject: [PATCH] refactor(threads): add daemon, period, Event-based shutdown to APRSDThread - Add daemon=True class attribute (subclasses override to False) - Add period=1 class attribute for wait interval - Replace thread_stop boolean with _shutdown_event (threading.Event) - Add wait() method for interruptible sleeps - Update tests for new Event-based API BREAKING: thread_stop boolean replaced with _shutdown_event. Code checking thread.thread_stop directly must use thread._shutdown_event.is_set() --- aprsd/threads/aprsd.py | 35 ++++++++--- tests/threads/test_aprsd_thread.py | 98 +++++++++++++++++++++++++----- 2 files changed, 110 insertions(+), 23 deletions(-) diff --git a/aprsd/threads/aprsd.py b/aprsd/threads/aprsd.py index 77e60cf..9eebbae 100644 --- a/aprsd/threads/aprsd.py +++ b/aprsd/threads/aprsd.py @@ -2,7 +2,6 @@ import abc import datetime import logging import threading -import time from typing import List import wrapt @@ -13,21 +12,26 @@ LOG = logging.getLogger('APRSD') class APRSDThread(threading.Thread, metaclass=abc.ABCMeta): """Base class for all threads in APRSD.""" + # Class attributes - subclasses override as needed + daemon = True # Most threads are daemon threads + period = 1 # Default wait period in seconds loop_count = 1 _pause = False - thread_stop = False def __init__(self, name): super().__init__(name=name) - self.thread_stop = False + # Set daemon from class attribute + self.daemon = self.__class__.daemon + # Set period from class attribute (can be overridden in __init__) + self.period = self.__class__.period + self._shutdown_event = threading.Event() + self.loop_count = 0 APRSDThreadList().add(self) self._last_loop = datetime.datetime.now() def _should_quit(self): - """see if we have a quit message from the global queue.""" - if self.thread_stop: - return True - return False + """Check if thread should exit.""" + return self._shutdown_event.is_set() def pause(self): """Logically pause the processing of the main loop.""" @@ -40,8 +44,21 @@ class APRSDThread(threading.Thread, metaclass=abc.ABCMeta): self._pause = False def stop(self): + """Signal thread to stop. Returns immediately.""" LOG.debug(f"Stopping thread '{self.name}'") - self.thread_stop = True + self._shutdown_event.set() + + def wait(self, timeout: float | None = None) -> bool: + """Wait for shutdown signal or timeout. + + Args: + timeout: Seconds to wait. Defaults to self.period. + + Returns: + True if shutdown was signaled, False if timeout expired. + """ + wait_time = timeout if timeout is not None else self.period + return self._shutdown_event.wait(timeout=wait_time) @abc.abstractmethod def loop(self): @@ -64,7 +81,7 @@ class APRSDThread(threading.Thread, metaclass=abc.ABCMeta): LOG.debug('Starting') while not self._should_quit(): if self._pause: - time.sleep(1) + self.wait(timeout=1) else: self.loop_count += 1 can_loop = self.loop() diff --git a/tests/threads/test_aprsd_thread.py b/tests/threads/test_aprsd_thread.py index b18cfb5..5224735 100644 --- a/tests/threads/test_aprsd_thread.py +++ b/tests/threads/test_aprsd_thread.py @@ -1,3 +1,4 @@ +import datetime import threading import time import unittest @@ -42,11 +43,9 @@ class TestAPRSDThread(unittest.TestCase): """Test thread initialization.""" thread = TestThread('TestThread1') self.assertEqual(thread.name, 'TestThread1') - self.assertFalse(thread.thread_stop) + self.assertFalse(thread._shutdown_event.is_set()) self.assertFalse(thread._pause) - self.assertEqual(thread.loop_count, 1) - - # Should be registered in thread list + self.assertEqual(thread.loop_count, 0) # Was 1, now starts at 0 thread_list = APRSDThreadList() self.assertIn(thread, thread_list.threads_list) @@ -54,8 +53,7 @@ class TestAPRSDThread(unittest.TestCase): """Test _should_quit() method.""" thread = TestThread('TestThread2') self.assertFalse(thread._should_quit()) - - thread.thread_stop = True + thread._shutdown_event.set() self.assertTrue(thread._should_quit()) def test_pause_unpause(self): @@ -72,20 +70,93 @@ class TestAPRSDThread(unittest.TestCase): def test_stop(self): """Test stop() method.""" thread = TestThread('TestThread4') - self.assertFalse(thread.thread_stop) - + self.assertFalse(thread._shutdown_event.is_set()) thread.stop() - self.assertTrue(thread.thread_stop) + self.assertTrue(thread._shutdown_event.is_set()) def test_loop_age(self): """Test loop_age() method.""" - import datetime - thread = TestThread('TestThread5') age = thread.loop_age() self.assertIsInstance(age, datetime.timedelta) self.assertGreaterEqual(age.total_seconds(), 0) + def test_daemon_attribute_default(self): + """Test that daemon attribute defaults to True.""" + thread = TestThread('DaemonTest') + self.assertTrue(thread.daemon) + + def test_daemon_attribute_override(self): + """Test that daemon attribute can be overridden via class attribute.""" + + class NonDaemonThread(APRSDThread): + daemon = False + + def loop(self): + return False + + thread = NonDaemonThread('NonDaemonTest') + self.assertFalse(thread.daemon) + + def test_period_attribute_default(self): + """Test that period attribute defaults to 1.""" + thread = TestThread('PeriodTest') + self.assertEqual(thread.period, 1) + + def test_period_attribute_override(self): + """Test that period attribute can be overridden via class attribute.""" + + class LongPeriodThread(APRSDThread): + period = 60 + + def loop(self): + return False + + thread = LongPeriodThread('LongPeriodTest') + self.assertEqual(thread.period, 60) + + def test_shutdown_event_exists(self): + """Test that _shutdown_event is created.""" + thread = TestThread('EventTest') + self.assertIsInstance(thread._shutdown_event, threading.Event) + self.assertFalse(thread._shutdown_event.is_set()) + + def test_wait_returns_false_on_timeout(self): + """Test that wait() returns False when timeout expires.""" + thread = TestThread('WaitTimeoutTest') + start = time.time() + result = thread.wait(timeout=0.1) + elapsed = time.time() - start + self.assertFalse(result) + self.assertGreaterEqual(elapsed, 0.1) + + def test_wait_returns_true_when_stopped(self): + """Test that wait() returns True immediately when stop() was called.""" + thread = TestThread('WaitStopTest') + thread.stop() + start = time.time() + result = thread.wait(timeout=10) + elapsed = time.time() - start + self.assertTrue(result) + self.assertLess(elapsed, 1) + + def test_wait_uses_period_by_default(self): + """Test that wait() uses self.period when no timeout specified.""" + + class ShortPeriodThread(APRSDThread): + period = 0.1 + + def loop(self): + return False + + thread = ShortPeriodThread('ShortPeriodTest') + start = time.time() + result = thread.wait() + elapsed = time.time() - start + self.assertFalse(result) + self.assertGreaterEqual(elapsed, 0.1) + self.assertLess(elapsed, 0.5) + def test_str(self): """Test __str__() method.""" thread = TestThread('TestThread6') @@ -253,10 +324,9 @@ class TestAPRSDThreadList(unittest.TestCase): thread2 = TestThread('TestThread9') thread_list.add(thread1) thread_list.add(thread2) - thread_list.stop_all() - self.assertTrue(thread1.thread_stop) - self.assertTrue(thread2.thread_stop) + self.assertTrue(thread1._shutdown_event.is_set()) + self.assertTrue(thread2._shutdown_event.is_set()) def test_pause_all(self): """Test pause_all() method."""