Fix bash coprocess usage races

3 files changed, 50 insertions, 2 deletions

diff --git a/libbutl/manifest-parser.bash.in b/libbutl/manifest-parser.bash.in
index 5b38eeb..df06138 100644
--- a/libbutl/manifest-parser.bash.in
+++ b/libbutl/manifest-parser.bash.in
@@ -67,9 +67,27 @@ function butl_manifest_parser_start () # [<file>]
   # multiple coprocesses at a time (see the BUGS section of bash(1) man page
   # for details).
   #
-  coproc { butl_parse_manifest; } <&"$butl_manifest_parser_ifd"
+  # An update: it turns out that we still can end up with an unset COPROC if
+  # the process finishes too early. To avoid that we suspend the subshell
+  # before executing the parser process and resume it after querying the
+  # COPROC value. Note that we need to be careful not to attempt to resume the
+  # process that hasn't suspended itself (see butl_resume_process() for
+  # details).
+  #
+  # Note that the suspend builtin doesn't work in subshells by default since
+  # there is no job control enabled for them. Also when it is enabled (via
+  # `set -m`), the builtin stops subshells recursively up to the command being
+  # run from the interactive shell, which is not what we want. That's why we
+  # use kill which is also a builtin and thus presumably is not slower than
+  # suspend.
+  #
+  coproc { kill -SIGSTOP $BASHPID; exec "$(butl_path)/manifest" parse; } \
+         <&"$butl_manifest_parser_ifd"
+
   exec {butl_manifest_parser_ofd}<&"${COPROC[0]}"
   butl_manifest_parser_pid="$COPROC_PID"
+
+  butl_resume_process "$butl_manifest_parser_pid"
 }
 
 # Finish the manifest parsing co-process.
diff --git a/libbutl/manifest-serializer.bash.in b/libbutl/manifest-serializer.bash.in
index fa6b94a..ce99779 100644
--- a/libbutl/manifest-serializer.bash.in
+++ b/libbutl/manifest-serializer.bash.in
@@ -64,9 +64,14 @@ function butl_manifest_serializer_start () # [--long-lines] [<file>]
 
   # See notes in butl_manifest_parser_start() on bash co-process issues.
   #
-  coproc { butl_serialize_manifest "${ops[@]}"; } >&"$butl_manifest_serializer_ofd"
+  coproc { kill -SIGSTOP $BASHPID; \
+           exec "$(butl_path)/manifest" "${ops[@]}" serialize; } \
+         >&"$butl_manifest_serializer_ofd"
+
   butl_manifest_serializer_ifd="${COPROC[1]}"
   butl_manifest_serializer_pid="$COPROC_PID"
+
+  butl_resume_process "$butl_manifest_serializer_pid"
 }
 
 # Finish the manifest serialization co-process.
diff --git a/libbutl/utility.bash.in b/libbutl/utility.bash.in
index 56bd3ab..5f51a2c 100644
--- a/libbutl/utility.bash.in
+++ b/libbutl/utility.bash.in
@@ -23,3 +23,28 @@ function butl_path ()
   #
   dirname "${BASH_SOURCE[0]}"
 }
+
+# Resume a stopped process execution by sending it the SIGCONT signal.
+#
+# Note that to avoid races the function waits until the process enters the
+# stopped state.
+#
+function butl_resume_process () # <pid>
+{
+  local pid="$1"
+
+  while true; do
+
+    # Note that while the ps's -o option 'state' value is not specified by
+    # POSIX, it is supported by all the major implementations with the leading
+    # 'T' character indicating the 'stopped' process run state.
+    #
+    local s
+    s="$(ps -p "$pid" -o state=)"
+
+    if [ "${s:0:1}" == "T" ]; then
+      kill -SIGCONT "$pid"
+      break
+    fi
+  done
+}