fix: do not run sync commands via event loop

The asyncio event loop is not reentrant safe. By that, we must not
interact with the event loop directly from any async (coro) context.
This is especially problematic, as we don't know if a non-async command
will interact with the eventloop somewhere down the stack.
This became visible when reading the repo.dirty property from an async
context, as it internally uses run_cmd which internally calls
ev.run_until_complete(). This property further was cached, which
sometimes hid the bug.

To fix this, the async and the non-async code paths need to be strictly
separated. This is possible by porting over the run_cmd from event loop
based waiting to subprocess.run. By that, we loose the liveoutput
command, which is anyways only used by a single invocation in the build
plugin. This now explicitly interacts with the event loop to restore the
behavior.

Reported-by: Konrad Weihmann <konrad.weihmann@avnet.eu>
Signed-off-by: Felix Moessbauer <felix.moessbauer@siemens.com>
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

kas/libkas.py

@@ -33,7 +33,7 @@
 import errno
 import pathlib
 import signal
-from subprocess import Popen, PIPE
+from subprocess import Popen, PIPE, run as subprocess_run
 from .context import get_context
 from .kasusererror import KasUserError, CommandExecError
 
@@ -177,17 +177,18 @@ async def run_cmd_async(cmd, cwd, env=None, fail=True, liveupdate=False):
     return (ret, ''.join(logo.stdout))
 
 
-def run_cmd(cmd, cwd, env=None, fail=True, liveupdate=False):
+def run_cmd(cmd, cwd, env=None, fail=True):
     """
         Runs a command synchronously.
     """
+    env = env or get_context().environ
+    cmdstr = ' '.join(cmd)
+    logging.debug('%s$ %s', cwd, cmdstr)
 
-    loop = asyncio.get_event_loop()
-    (ret, output) = loop.run_until_complete(
-        run_cmd_async(cmd, cwd, env, fail, liveupdate))
-    if ret and fail:
-        raise CommandExecError(cmd, ret)
-    return (ret, output)
+    ret = subprocess_run(cmd, env=env, cwd=cwd, stdout=PIPE)
+    if ret.returncode and fail:
+        raise CommandExecError(cmd, ret.returncode)
+    return (ret.returncode, ret.stdout.decode('utf-8'))
 
 
 def find_program(paths, name):
@@ -204,6 +205,8 @@ def find_program(paths, name):
 def repos_fetch(repos):
     """
         Fetches the list of repositories to the kas_work_dir.
+
+        .. note:: termination point of the asyncio event loop.
     """
     if len(repos) == 0:
         return
@@ -222,6 +225,8 @@ def repos_fetch(repos):
 def repos_apply_patches(repos):
     """
         Applies the patches to the repositories.
+
+        .. note:: termination point of the asyncio event loop.
     """
     if len(repos) == 0:
         return

kas/plugins/build.py

@@ -40,11 +40,12 @@
 import subprocess
 import sys
 import json
+import asyncio
 from pathlib import Path
 from datetime import datetime
 from kas.context import create_global_context
 from kas.config import Config
-from kas.libkas import find_program, run_cmd
+from kas.libkas import find_program, run_cmd_async
 from kas.libkas import setup_parser_keep_config_unchanged_arg
 from kas.libcmds import Macro, Command
 from kas.libkas import setup_parser_common_args
@@ -148,7 +149,9 @@ def execute(self, ctx):
             if ret != 0:
                 raise CommandExecError(cmd, ret)
         else:
-            run_cmd(cmd, cwd=ctx.build_dir, liveupdate=True)
+            loop = asyncio.get_event_loop()
+            loop.run_until_complete(run_cmd_async(cmd, cwd=ctx.build_dir,
+                                                  liveupdate=True))
         time_finished = datetime.now()
 
         if ctx.args.provenance: