Fix deadlock on large output from basilisp.process/exec (#1210)

Fixes #1202

Author: chrisrink10

CHANGELOG.md

@@ -5,9 +5,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
-
 ### Fixed 
- * Fixed a regression introduced in #1176 where the testrunner couldn't handle relative paths in `sys.path`, causing `basilisp test` to fail when no arugments were provided (#1204)
+ * Fix a regression introduced in #1176 where the testrunner couldn't handle relative paths in `sys.path`, causing `basilisp test` to fail when no arugments were provided (#1204)
+ * Fix a bug where `basilisp.process/exec` could deadlock reading process output if that output exceeded the buffer size (#1202)
 
 ## [v0.3.6]
 ### Added

src/basilisp/cli.py

@@ -398,7 +398,7 @@ def _subcommand(
     Callable[["argparse._SubParsersAction"], None],
 ]:
     def _wrap_add_subcommand(
-        f: Callable[[argparse.ArgumentParser], None]
+        f: Callable[[argparse.ArgumentParser], None],
     ) -> Callable[["argparse._SubParsersAction"], None]:
         def _wrapped_subcommand(subparsers: "argparse._SubParsersAction"):
             parser = subparsers.add_parser(

src/basilisp/lang/compiler/__init__.py

@@ -33,7 +33,10 @@
     GeneratorContext,
 )
 from basilisp.lang.compiler.generator import expressionize as _expressionize  # noqa
-from basilisp.lang.compiler.generator import gen_py_ast, py_module_preamble
+from basilisp.lang.compiler.generator import (
+    gen_py_ast,
+    py_module_preamble,
+)
 from basilisp.lang.compiler.generator import statementize as _statementize
 from basilisp.lang.compiler.optimizer import PythonASTOptimizer
 from basilisp.lang.interfaces import ISeq

src/basilisp/lang/compiler/analyzer.py

@@ -117,9 +117,21 @@
     PyTuple,
 )
 from basilisp.lang.compiler.nodes import Queue as QueueNode
-from basilisp.lang.compiler.nodes import Quote, Recur, Reify, Require, RequireAlias
+from basilisp.lang.compiler.nodes import (
+    Quote,
+    Recur,
+    Reify,
+    Require,
+    RequireAlias,
+)
 from basilisp.lang.compiler.nodes import Set as SetNode
-from basilisp.lang.compiler.nodes import SetBang, SpecialFormNode, Throw, Try, VarRef
+from basilisp.lang.compiler.nodes import (
+    SetBang,
+    SpecialFormNode,
+    Throw,
+    Try,
+    VarRef,
+)
 from basilisp.lang.compiler.nodes import Vector as VectorNode
 from basilisp.lang.compiler.nodes import (
     WithMeta,

src/basilisp/lang/compiler/generator.py

@@ -86,11 +86,25 @@
     PyTuple,
 )
 from basilisp.lang.compiler.nodes import Queue as QueueNode
-from basilisp.lang.compiler.nodes import Quote, Recur, Reify, Require
+from basilisp.lang.compiler.nodes import (
+    Quote,
+    Recur,
+    Reify,
+    Require,
+)
 from basilisp.lang.compiler.nodes import Set as SetNode
-from basilisp.lang.compiler.nodes import SetBang, T_withmeta, Throw, Try, VarRef
+from basilisp.lang.compiler.nodes import (
+    SetBang,
+    T_withmeta,
+    Throw,
+    Try,
+    VarRef,
+)
 from basilisp.lang.compiler.nodes import Vector as VectorNode
-from basilisp.lang.compiler.nodes import WithMeta, Yield
+from basilisp.lang.compiler.nodes import (
+    WithMeta,
+    Yield,
+)
 from basilisp.lang.compiler.utils import (
     ast_AsyncFunctionDef,
     ast_ClassDef,
@@ -392,7 +406,7 @@ def attr_node(node, idx):
 
 
 def _simple_ast_generator(
-    gen_ast: Callable[P_simplegen, T_pynode]
+    gen_ast: Callable[P_simplegen, T_pynode],
 ) -> Callable[P_simplegen, GeneratedPyAST[T_pynode]]:
     """Wrap simpler AST generators to return a GeneratedPyAST."""
 
@@ -1737,7 +1751,7 @@ def __fn_meta(
 
 
 def __kwargs_support_decorator(
-    node: Union[Fn, DefTypeMethodArity, DefTypeClassMethod, DefTypeStaticMethod]
+    node: Union[Fn, DefTypeMethodArity, DefTypeClassMethod, DefTypeStaticMethod],
 ) -> Iterable[ast.expr]:
     if node.kwarg_support is None:
         return
@@ -3533,8 +3547,8 @@ def _py_tuple_to_py_ast(
 def _with_meta_to_py_ast(
     ctx: GeneratorContext,
     node: WithMeta[T_withmeta],
-    *args: P_generator.args,
-    **kwargs: P_generator.kwargs,
+    *args,
+    **kwargs,
 ) -> GeneratedPyAST[ast.expr]:
     """Generate a Python AST node for Python interop method calls."""
     assert node.op == NodeOp.WITH_META

src/basilisp/lang/runtime.py

@@ -2098,7 +2098,7 @@ def wrapped_f(*args, **kwargs):
 
 
 def _basilisp_fn(
-    arities: tuple[Union[int, kw.Keyword], ...]
+    arities: tuple[Union[int, kw.Keyword], ...],
 ) -> Callable[..., BasilispFunction]:
     """Create a Basilisp function, setting meta and supplying a with_meta
     method implementation."""

src/basilisp/process.lpy

@@ -265,20 +265,22 @@
 
 (defn exec
   "Execute a command as by :lpy:fn:`start` and, upon successful return, return the
-  captured value of the process ``stdout`` as by :lpy:fn:`basilisp.core/slurp`.
+  captured value of the process ``stdout`` as if by :lpy:fn:`basilisp.core/slurp`.
 
   If ``opts`` are specified, they should be provided as a map in the first argument
   position. ``opts`` are exactly the same as those in :lpy:fn:`start`.
 
   If the return code is non-zero, throw
   :external:py:exc:`subprocess.CalledProcessError`."
   [& opts+args]
-  (let [process (apply start opts+args)
-        retcode (.wait process)]
+  (let [process    (apply start opts+args)
+        [stdout _] (.communicate process)
+        retcode    (.-returncode process)]
     (if (zero? retcode)
-      (if-let [out (.-stdout process)]
-        (slurp out)
-        "")
+      (cond
+        (byte-string? stdout) (.decode stdout "utf-8")
+        stdout                stdout
+        :else                 "")
       (throw
        (subprocess/CalledProcessError retcode
                                       (.-args process)

tests/basilisp/test_process.lpy

@@ -1,5 +1,6 @@
 (ns tests.basilisp.test-process
-  (:import pathlib
+  (:import os
+           pathlib
            subprocess
            sys)
   (:require
@@ -141,10 +142,12 @@
   (is (= "" (process/exec sys/executable "-c" "pass")))
   (is (= "" (process/exec {:out :inherit} sys/executable "-c" "print(\"hi\")")))
   (is (= "" (process/exec sys/executable "-c" "import sys; print(\"hi\", file=sys.stderr)")))
-  (is (= "hi\n" (process/exec sys/executable "-c" "print(\"hi\")")))
+  (is (= (str "hi" os/linesep)
+         (process/exec sys/executable "-c" "print(\"hi\")")))
   (is (thrown? subprocess/CalledProcessError
                (process/exec sys/executable "-c" "import sys; sys.exit(2)")))
-  (is (= "BASILISP\n" (process/exec {:env {"PYTHON_HOSTED_LANG" "BASILISP"}}
-                                    sys/executable
-                                    "-c"
-                                    "import os; print(os.environ[\"PYTHON_HOSTED_LANG\"])"))))
+  (is (= (str "BASILISP" os/linesep)
+         (process/exec {:env {"PYTHON_HOSTED_LANG" "BASILISP"}}
+                       sys/executable
+                       "-c"
+                       "import os; print(os.environ[\"PYTHON_HOSTED_LANG\"])"))))