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 <sunil@medhas.org>
Reviewed-by: Veiko Aasa <veiko17@disroot.org>

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

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