From f559870d3eae5e37235c29c1cbf7236a04d3b059 Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Thu, 25 Sep 2025 12:27:51 -0700 Subject: [PATCH] actions: Fix lifetime of thread local storage - A local storage object must exist globally shared by all threads. Then object.__dict__ is the thread specific storage. Absent this, when multiple actions run in parallel, one will erase the thread local object of another. Tests: - When an error is raised in a privileged method, then the HTML error shown contains stdout and stderr of the involved processes. - Running functional tests on a lot of apps does not show this error anymore. Signed-off-by: Sunil Mohan Adapa Reviewed-by: Veiko Aasa --- plinth/action_utils.py | 4 ++-- plinth/actions.py | 18 ++++++------------ 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/plinth/action_utils.py b/plinth/action_utils.py index 8bed5ca0a..5e5e87f68 100644 --- a/plinth/action_utils.py +++ b/plinth/action_utils.py @@ -837,10 +837,10 @@ def run(command, **kwargs): kwargs['stderr'] = subprocess.PIPE process = subprocess.run(command, **kwargs) - if collect_stdout and actions.thread_storage: + if collect_stdout and hasattr(actions.thread_storage, 'stdout'): actions.thread_storage.stdout += process.stdout - if collect_stderr and actions.thread_storage: + if collect_stderr and hasattr(actions.thread_storage, 'stderr'): actions.thread_storage.stderr += process.stderr return process diff --git a/plinth/actions.py b/plinth/actions.py index 5762d3b99..d045c4000 100644 --- a/plinth/actions.py +++ b/plinth/actions.py @@ -20,7 +20,7 @@ logger = logging.getLogger(__name__) socket_path = '/run/freedombox/privileged.socket' -thread_storage = None +thread_storage = threading.local() # An alias for 'str' to mark some strings as sensitive. Sensitive strings are @@ -364,7 +364,6 @@ class JSONEncoder(json.JSONEncoder): def _setup_thread_storage(): """Setup collection of stdout/stderr from any process in this thread.""" global thread_storage - thread_storage = threading.local() thread_storage.stdout = b'' thread_storage.stderr = b'' @@ -376,14 +375,13 @@ def _clear_thread_storage(): cleaned up after a thread terminates. """ global thread_storage - if thread_storage: - thread_storage.stdout = None - thread_storage.stderr = None - thread_storage = None + thread_storage.stdout = None + thread_storage.stderr = None def get_return_value_from_exception(exception): """Return the value to return from server when an exception is raised.""" + global thread_storage return_value = { 'result': 'exception', 'exception': { @@ -391,14 +389,10 @@ def get_return_value_from_exception(exception): 'name': type(exception).__name__, 'args': exception.args, 'traceback': traceback.format_tb(exception.__traceback__), - 'stdout': '', - 'stderr': '' + 'stdout': getattr(thread_storage, 'stdout', b'').decode(), + 'stderr': getattr(thread_storage, 'stderr', b'').decode(), } } - if thread_storage: - return_value['exception']['stdout'] = thread_storage.stdout.decode() - return_value['exception']['stderr'] = thread_storage.stderr.decode() - return return_value