-
Notifications
You must be signed in to change notification settings - Fork 36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ctrl-C kills nyagos.exe itself when more
runs without redirect
#342
Comments
日本語ですみません。こちらまだ未解決ですか。 似た案件かどうか分かりませんが、時間がかかる処理を Ctrl+C で強制終了させようとしたら NYAGOS ごと終了してしまうようです。 具体的には Windows Terminal 上の NYAGOS で、拙作の depm の動作確認のため駆動中に意図的に Ctrl+C した時です。 ちなみに depm は内部で |
ありがとうございます。他にも https://twitter.com/tomato3713/status/1323614105702297600
https://twitter.com/tomato3713/status/1325457861955121154
という情報をいただいており、要対応と認識しています。 |
こちらなのですが、scoopを利用してnyagosをインストールしていたので ただし,コマンドを実行していない状態でCtrl+Cを連続して入力した際には,コード 2で終了する現象が発生するようになりました. |
@tomato3713 さんのご指摘、私のほうでも試してみましたが、確かに Ctrl+C で NYAGOS ごと死ぬ現象はなくなりました。 ただ、起動しているプロセスに SIGINT または SIGTERM が届いてないようで、外部(?)から強制的にプロセスが落とされている感じです。この辺は PowerShell から起動した場合と事象が異なっています。 一応、報告まで。 |
せっかくいろいろ情報を寄せていただいているのにレスポンスが遅くなり、申し訳ありません。 ~\scoop\shims\nyagos.exe より起動すると Ctrl-C で死にやすいというのは貴重な情報です。ありがとうございます。本体の話ではなくて恐縮ですが、とりいそぎ、自分が書いている scoop / nyagos 関連のドキュメントで ~\scoop\shims\nyagos.exe を使うよう記載しているものは ~\scoop\apps\nyagos\current\nyagos.exe の方を見るよう改訂しました。 調査の方はぼちぼちと開始していますが、まだ確定的な発見はなく、つらつらと zenn.dev のスクラップ に検討課題を綴っているような状況です。 |
今まで、readline とコマンド処理の間で、わずかながらシグナルをキャッチしていない隙間があったので、隙間が発生しないように、キャッチする範囲を広げてみました。
(context.WithCancel によるコマンドを停止させる channel と、signal.Notify による Ctrl-C をキャッチする channel で有効期限が一致しなくなってしまうため、うまい連携のさせ方がなかなか分からず、結構な難産でした) 結果:
と件名の問題については改善が見られました。 報告いただいている事象については、軽く試してみて、もしかして直ってる?という感じなのですが、十分な検証時間がとれていないので、もうちょっと試用を続けてみてから、4.4.9_1(予定)として出すかどうか考えます。 |
上で述べた改善を取り込んだバージョンを公開しました。しばらく様子見ですね |
以下の検証コードで簡単にチェックしてみました(自作パッケージの検証も兼ねてw) package main
import (
"context"
"fmt"
"os"
"time"
"github.com/spiegel-im-spiegel/gocli/signal"
)
func ticker(ctx context.Context) error {
t := time.NewTicker(1 * time.Second) // 1 second cycle
defer t.Stop()
for {
select {
case now := <-t.C: // ticker event
fmt.Println(now.Format(time.RFC3339))
case <-ctx.Done(): // cancel event from context
fmt.Println("Stop ticker")
return ctx.Err()
}
}
}
func main() {
if err := ticker(signal.Context(context.Background(), os.Interrupt)); err != nil {
fmt.Fprintf(os.Stderr, "Error Message: %#v\n", err)
return
}
} これを NYAGOS 上で実行すると
という感じになります。 あと |
検証報告ありがとうございます。
これは nyagos が子プロセス待機中に context の Cancel を受信すると、子プロセスを kill するように実装しているせいだと思います。この動作は手本にした (*"os/exec".Cmd).Start() あたりのコードを真似たためですが… kill までしなくてもいいかもしれませんねぇ。ターミナルがターミナル中のすべてのプロセスにシグナルを送ってるかもしれないので【要確認】 (なぜ、標準ライブラリをそのまま使わず、真似たコードになったかというと、二重引用符に関する扱いが CMD.EXE と "os/exec".Cmd で違っていて、コマンド引数に二重引用符をありのままに渡すことができないからです。そのまま渡せないと二重引用符の有無で引数の扱いを変える一部のコマンド(findなど)が意図どおり使えなくなってしまうという問題がありまして) |
kill すべきではないようですね。「kill しないように改造した nyagos」で以下の検証プログラムを走らせてみました。 package main
import (
"os"
"os/signal"
"time"
)
func main() {
if len(os.Args) >= 2 {
println("signal off")
// signal.Ignore(os.Interrupt)
sigCh := make(chan os.Signal, 100)
signal.Notify(sigCh, os.Interrupt)
defer signal.Stop(sigCh)
}
println("start")
time.Sleep(time.Second * 10)
println("end")
} プログラムは go build で EXE ファイルを作成して、nyagos の下から実行しました。
以下、結果を考察しました(若干、憶測も交じっているので、それは違うというと思われる点ありましたら、ご指摘ください)
おそらくは、kill を外すだけで(moreも含め)万事解決しそうです。 (どうも、自分の CommandContext の仕様の解釈の間違いがそもそもの問題だったようですね。たしかに "context" が登場するまで、こんな問題なかったもんなー) |
にて対応しました。しばらく様子見です。 このコードのカットの仕方だと無駄な goroutine が省けるので負荷は下がるものの、Ctrl-C 連打で os.Interuppt や context.WithCancel の channel のどれかが満タンになってとまったりしないか?とか心配があります。が、今のところ特にそういうハングとかはなく、一応動いているようです。
→ 落ちなくなりました。 |
上のバージョンで Ctrl+C での挙動が PowerShell と同じになりました。ばっちりです! |
@spiegel-im-spiegel さん・ @tomato3713 さん: ご確認ありがとうございました。わたくしの思い違いから発生してしまった本件、皆様のおかげで何とか収束できそうです。 今回は 4.4.9_0→1 の時と違って、別件に致命的な不具合を抱えているというわけでもないので、キリのいい時(例:Go のマイナーバージョンが一つあがった時)にでも、4.4.9_2 としてバイナリを公開したいと思います。 |
Go のマイナーバージョンはしばらくは出そうにないので、年もかわってしまったことですので 4.4.9_2 としてリリースしました。 自分でオープンした issue ですのにで自分でクローズしてしまいますが、何かおかしい動作が見られるようでしたら、本件を再オープンしていただいても、別途新規に issue を立てていただいても、どちらでも結構です。 どうも、ありがとうございました。 |
No description provided.
The text was updated successfully, but these errors were encountered: