From 9e31c9ee7c28afa349baa6125a953d6dba3320a9 Mon Sep 17 00:00:00 2001 From: Thomas BESSOU Date: Sun, 16 Feb 2025 18:15:42 +0100 Subject: [PATCH 1/4] Allow app to track callback change This allows apps to answer the question "was I already called as part of handling this particular event?" from anywhere in the code, by tracking the callback counter. In particular this is useful to simplify https://github.com/Ten0/homeassistant_python_typer/blob/1f8075ae6655a65aab093f20fa4624d6587a201b/README.md#what-to-be-careful-about --- appdaemon/adbase.py | 1 + appdaemon/threading.py | 15 +++++++++++++++ 2 files changed, 16 insertions(+) diff --git a/appdaemon/adbase.py b/appdaemon/adbase.py index 0ddf9215c..40a378076 100644 --- a/appdaemon/adbase.py +++ b/appdaemon/adbase.py @@ -76,6 +76,7 @@ def __init__(self, ad: AppDaemon, name, logging, args, config, app_config, globa self.app_dir = self.AD.app_dir self.config_dir = self.AD.config_dir self.dashboard_dir = None + self.callback_counter = 0 if self.AD.http is not None: self.dashboard_dir = self.AD.http.dashboard_dir diff --git a/appdaemon/threading.py b/appdaemon/threading.py index 4ec941bb7..cab3b1eef 100644 --- a/appdaemon/threading.py +++ b/appdaemon/threading.py @@ -887,6 +887,8 @@ async def async_worker(self, args): # noqa: C901 app = await self.AD.app_management.get_app_instance(name, objectid) if app is not None: try: + self.increment_callback_counter(app, name) + if _type == "scheduler": try: await self.update_thread_info("async", callback, name, _type, _id, silent) @@ -1010,6 +1012,8 @@ def worker(self): # noqa: C901 app = utils.run_coroutine_threadsafe(self, self.AD.app_management.get_app_instance(name, objectid)) if app is not None: try: + self.increment_callback_counter(app, name) + if _type == "scheduler": try: utils.run_coroutine_threadsafe( @@ -1193,3 +1197,14 @@ def report_callback_sig(self, name, type, funcref, args): "Logged an error to %s", self.AD.logging.get_filename("error_log"), ) + + def increment_callback_counter(self, app, name): + try: + app.callback_counter += 1 + except AttributeError: + error_logger = logging.getLogger("Error.{}".format(name)) + error_logger.warning("-" * 60) + error_logger.warning("Unexpected error in worker for App %s:", name) + error_logger.warning("-" * 60) + error_logger.warning(traceback.format_exc()) + error_logger.warning("-" * 60) From a05357cbcbcfc6e61d2b2fb000247d8d5d9263c4 Mon Sep 17 00:00:00 2001 From: Thomas BESSOU Date: Wed, 23 Apr 2025 21:20:44 +0200 Subject: [PATCH 2/4] Move counter update to a place where it can't race before funcref is called --- appdaemon/threads.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/appdaemon/threads.py b/appdaemon/threads.py index 6fb345980..91db4a853 100644 --- a/appdaemon/threads.py +++ b/appdaemon/threads.py @@ -927,8 +927,6 @@ async def async_worker(self, args): # noqa: C901 app = self.AD.app_management.get_app_instance(name, objectid) if app is not None: try: - self.increment_callback_counter(app, name) - pos_args = tuple() kwargs = dict() match _type: @@ -979,6 +977,7 @@ async def safe_callback(): """Wraps actually calling the function for the callback with logic to transform exceptions based on the callback type""" try: + self.increment_callback_counter(app, name) await funcref() except Exception as exc: # positional arguments common to all the AppCallbackFail exceptions @@ -1022,8 +1021,6 @@ def worker(self): # noqa: C901 app = self.AD.app_management.get_app_instance(name, objectid) if app is not None: try: - self.increment_callback_counter(app, name) - pos_args = tuple() kwargs = dict() match args["type"]: @@ -1074,6 +1071,7 @@ def safe_callback(): """Wraps actually calling the function for the callback with logic to transform exceptions based on the callback type""" try: + self.increment_callback_counter(app, name) funcref() except Exception as exc: # positional arguments common to all the AppCallbackFail exceptions From edf39ba81c4ed06fac2ee08e76e8b72eb798efc5 Mon Sep 17 00:00:00 2001 From: Thomas BESSOU Date: Wed, 23 Apr 2025 23:21:31 +0200 Subject: [PATCH 3/4] Fix potential race during increment_callback_counter --- appdaemon/threads.py | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/appdaemon/threads.py b/appdaemon/threads.py index 91db4a853..8da80b619 100644 --- a/appdaemon/threads.py +++ b/appdaemon/threads.py @@ -976,8 +976,8 @@ async def async_worker(self, args): # noqa: C901 async def safe_callback(): """Wraps actually calling the function for the callback with logic to transform exceptions based on the callback type""" + increment_callback_counter(app, name) try: - self.increment_callback_counter(app, name) await funcref() except Exception as exc: # positional arguments common to all the AppCallbackFail exceptions @@ -1070,8 +1070,8 @@ def worker(self): # noqa: C901 def safe_callback(): """Wraps actually calling the function for the callback with logic to transform exceptions based on the callback type""" + increment_callback_counter(app, name) try: - self.increment_callback_counter(app, name) funcref() except Exception as exc: # positional arguments common to all the AppCallbackFail exceptions @@ -1172,13 +1172,16 @@ def report_callback_sig(self, name, type, funcref, args): self.AD.logging.get_filename("error_log"), ) - def increment_callback_counter(self, app, name): - try: +def increment_callback_counter(app, name): + try: + # This function may be called concurrently and the GIL won't protect us against + # races during app.callback_counter += 1 so we need to use a lock. We'll just use that of the app. + with app.lock: app.callback_counter += 1 - except AttributeError: - error_logger = logging.getLogger("Error.{}".format(name)) - error_logger.warning("-" * 60) - error_logger.warning("Unexpected error in worker for App %s:", name) - error_logger.warning("-" * 60) - error_logger.warning(traceback.format_exc()) - error_logger.warning("-" * 60) + except: + error_logger = logging.getLogger("Error.{}".format(name)) + error_logger.warning("-" * 60) + error_logger.warning("Unexpected error in worker for App %s:", name) + error_logger.warning("-" * 60) + error_logger.warning(traceback.format_exc()) + error_logger.warning("-" * 60) From 2d1817529aad0810e61ba50e287a68a518cda4cd Mon Sep 17 00:00:00 2001 From: Thomas BESSOU Date: Thu, 8 May 2025 21:23:07 +0200 Subject: [PATCH 4/4] Lint fix --- appdaemon/threads.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/appdaemon/threads.py b/appdaemon/threads.py index 8da80b619..b1c6a67dd 100644 --- a/appdaemon/threads.py +++ b/appdaemon/threads.py @@ -1178,7 +1178,7 @@ def increment_callback_counter(app, name): # races during app.callback_counter += 1 so we need to use a lock. We'll just use that of the app. with app.lock: app.callback_counter += 1 - except: + except Exception: error_logger = logging.getLogger("Error.{}".format(name)) error_logger.warning("-" * 60) error_logger.warning("Unexpected error in worker for App %s:", name)