From 8d623c1927c2c4e381bd484bcc51def3213890f6 Mon Sep 17 00:00:00 2001
From: Koichi Murase <myoga.murase@gmail.com>
Date: Mon, 3 Apr 2023 06:03:54 +0900
Subject: [PATCH] util.bgproc: increase frequency of bgproc termination check

---
 docs/ChangeLog.md  |  1 +
 lib/util.bgproc.sh | 18 +++-----------
 note.txt           | 59 ++++++++++++++++++++++++++++++++++++++++++++++
 src/util.sh        | 30 +++++++++++++++++++----
 4 files changed, 88 insertions(+), 20 deletions(-)

diff --git a/docs/ChangeLog.md b/docs/ChangeLog.md
index b87b6c49..d36adacd 100644
--- a/docs/ChangeLog.md
+++ b/docs/ChangeLog.md
@@ -141,6 +141,7 @@
 - complete: add `bleopt complete_source_sabbrev_{opts,ignore}` (motivated by mozirilla213) `#D2013` f95eb0cc `#D2016` 45c76746
 - util.bgproc: separate `ble/util/bgproc` from `histdb` (motivated by bkerin) `#D2017` 7803305f
   - util.bgproc: fix use of `ble/util/idle` in bash-3 `#D2026` xxxxxxxx
+  - util.bgproc: increase frequency of bgproc termination check (motivated by bkerin) `#D2027` xxxxxxxx
 - menu-complete: support selection by index (requested by bkerin) `#D2023` b91b8bc8
 
 ## Changes
diff --git a/lib/util.bgproc.sh b/lib/util.bgproc.sh
index 69745c7e..2392142a 100644
--- a/lib/util.bgproc.sh
+++ b/lib/util.bgproc.sh
@@ -274,23 +274,11 @@ function ble/util/bgproc#start {
   return "$_ble_local_ext"
 }
 
-function ble/util/bgproc#stop/.wait {
-  local pid=$1 msec=$2
-  while
-    kill -0 "$pid" 2>/dev/null || return 0
-    ((msec>0))
-  do
-    ble/util/msleep "$((msec>1000?1000:msec))"
-    ((msec-=1000))
-  done
-  return 1
-}
 function ble/util/bgproc#stop/.kill {
   local pid=$1 i close_timeout=$2
-  ble/util/bgproc#stop/.wait "$pid" "$close_timeout" && return 0
-  kill -- "$pid"
-  ble/util/bgproc#stop/.wait "$pid" 10000 && return 0
-  kill -9 "$pid"
+  ble/util/conditional-sync '' '((1))' 1000 progressive-weight:pid="$pid":no-wait-pid:timeout="$close_timeout"
+  kill -0 "$pid" || return 0
+  ble/util/conditional-sync '' '((1))' 1000 progressive-weight:pid="$pid":no-wait-pid:SIGKILL:timeout=10000
 }
 
 ## @fn ble/util/bgproc#stop prefix
diff --git a/note.txt b/note.txt
index 84aa0249..6863116f 100644
--- a/note.txt
+++ b/note.txt
@@ -6726,6 +6726,65 @@ bash_tips
 
 2023-04-02
 
+  * bgproc: シェルの終了時に即座に強制終了するオプションをつける? (motivated by bkerin) [#D2028]
+    https://github.com/akinomyoga/ble.sh/discussions/309#discussioncomment-5498921
+
+    然し実際にそれが良い事なのか分からない。というか現状の実装ではそうはなって
+    いないのだったか? → child session はちゃんとすぐに終了する。然し端末が閉じ
+    ない。これは stderr, stdout, stdin の何れかが未だ開いたままになっているのが
+    原因と思われる。
+
+    .kill ではちゃんと全部閉じている筈と思ったがよく考えたらスタートしたプロセ
+    ス自体が stderr を保持しているのが原因の気がする。
+
+    そのまま強制終了するというのもどうかという気がするが、それならばそれで良い。
+
+    * done: 或いは終了チェックの interval をより短くする。これは
+      ble/util/bgproc#stop/.wait で使っている 1000 という値を設定できる様にすれ
+      ば良い。然し、これを変更したとして実際に終了にかかる時間がそんなに速くな
+      るのだろうか。
+
+      うーん。或いは conditional-sync の progressive-weight を使う? と思ったが
+      conditional-sync は新しいサブシェルを待つ物なので単に或る条件を待つという
+      事には使えない。conditional-sync を拡張して任意の条件を待つ事ができるよう
+      にする可能性も考えたが、
+
+      a うーん。或いはコマンドを開始するのではなくて既存のプロセスを待つという
+        機能を持たせる? この時に conditional-sync に足りていないのは kill を二
+        段階で行う機能である。SIGKILL を送る様にするオプションは実装しているが、
+        kill で駄目だった時に段階的に SIGKILL を送るという機能は実装していない。
+        そういう機能も追加して良いかもしれない。
+
+        うーん。取り敢えず conditional-sync を2回呼び出して二回目で SIGKILL を
+        指定すれば良いのだと気づいたので conditional-sync 自体に段階的に kill
+        を実行する機能は実装しなくて良かった。
+
+      b 或いはコマンドが空の時には指定された条件だけを待つという振る舞いにする。
+        この場合にはコマンドの kill までは実行しない。
+
+      然しもし原因が .kill の interval による物なのだとしても、
+      stdout/stderr/stdin を封じている限りは生きているというのは .kill が原因な
+      のではないのでは? 然し、プロセスが瞬間的に終了する事ができなかった場合に
+      は .kill による自動 kill が起動する迄は bgproc が保持している stderr が開
+      いたままである。然し、その場合には timeout になるまで相当待たなければなら
+      ない (既定では 10sec である。或いは SIGKILL まで更に 10sec ある)。
+
+      * ok: と思ったがもしかすると .kill 自体が fd を保持しているから bg
+        process が終了しない可能性? と思ったがそれも変だ。.kill 自体は fd を閉
+        じてから呼び出している筈 → 実際に確認した。
+
+    ? ok: ところで set -m して起動した bgproc のなかで更にパイプやサブシェルな
+      どを立ち上げた時に更に別の PGID になっているとその別の PGID までは kill
+      しきれないのでは。なので set +m を中で実行しなければならないのでは。
+
+      →と思ったが、実験してみた限りでは set -m が実際に効果を持つのはそれを呼
+      び出したサブシェルだけの様である。なので更に入れ子で呼び出された物に関し
+      ては敢えて set -m を実行しない限りはちゃんと元の pgid に属する様になって
+      いる。
+
+    即座に終了する方法について考えたがこれは単に kill-timeout を 0 に設定すれば
+    良いのでは。結局向こうで起こっている事が何か分からないのでどうしようもない。
+
   * make: インストール時に元のコメントなどの情報への pointer をつける (suggested by bkerin) [#D2027]
     https://github.com/akinomyoga/ble.sh/discussions/309#discussioncomment-5498921
 
diff --git a/src/util.sh b/src/util.sh
index 8bd7d094..879357f4 100644
--- a/src/util.sh
+++ b/src/util.sh
@@ -3638,7 +3638,10 @@ function ble/util/conditional-sync/.kill {
 ##   COMMAND ends.
 ##
 ##   @param[in] command
-##     The command that is evaluated in a subshell
+##     The command that is evaluated in a subshell.  If an empty string is
+##     specified, only the CONDITION is checked for the synchronization and any
+##     background subshell is not started.
+##
 ##   @param[in,opt] condition
 ##     The command to test the condition to continue to run the command.  The
 ##     default condition is "! ble/decode/has-input".  The following local
@@ -3667,6 +3670,15 @@ function ble/util/conditional-sync/.kill {
 ##       The processes are killed by SIGKILL.  When this is unspecified, the
 ##       processes are killed by SIGTERM.
 ##
+##     @opt pid=PID
+##       When COMMAND is empty, the function waits for the exit of the process
+##       specified by PID.  If a negative integer is specified, it is treated
+##       as PGID.  When the condition is unsatisfied or the timeout has been
+##       reached, the specified process will be killed.
+##
+##     @opt no-wait-pid
+##       Do not wait for the exit status of the background process
+##
 function ble/util/conditional-sync {
   local __ble_command=$1
   local __ble_continue=${2:-'! ble/decode/has-input'}
@@ -3680,10 +3692,18 @@ function ble/util/conditional-sync {
     local __ble_weight_max=$__ble_weight __ble_weight=1
 
   local sync_elapsed=0
-  [[ $__ble_timeout ]] && ((__ble_timeout<=0)) && return 142
+  if [[ $__ble_timeout ]] && ((__ble_timeout<=0)); then return 142; fi
   builtin eval -- "$__ble_continue" || return 148
   (
-    builtin eval -- "$__ble_command" & local __ble_pid=$!
+    local __ble_pid=
+    if [[ $__ble_command ]]; then
+      builtin eval -- "$__ble_command" & __ble_pid=$!
+    else
+      local ret
+      ble/opts#extract-last-optarg "$__ble_opts" pid
+      __ble_pid=$ret
+      ble/util/unlocal ret
+    fi
     while
       # check timeout
       if [[ $__ble_timeout ]]; then
@@ -3699,14 +3719,14 @@ function ble/util/conditional-sync {
       ((sync_elapsed+=__ble_weight))
       [[ :$__ble_opts: == *:progressive-weight:* ]] &&
         ((__ble_weight<<=1,__ble_weight>__ble_weight_max&&(__ble_weight=__ble_weight_max)))
-      builtin kill -0 "$__ble_pid" &>/dev/null
+      [[ ! $__ble_pid ]] || builtin kill -0 "$__ble_pid" &>/dev/null
     do
       if ! builtin eval -- "$__ble_continue"; then
         ble/util/conditional-sync/.kill
         return 148
       fi
     done
-    wait "$__ble_pid"
+    [[ ! $__ble_pid || :$__ble_opts: == *:no-wait-pid:* ]] || wait "$__ble_pid"
   )
 }