mattn / goemon

五右衛門
182 stars 14 forks source link

Fix wait condition after sending signal in terminate #9

Closed soh335 closed 7 years ago

soh335 commented 7 years ago

日本語で失礼します

起きてること

goemon.terminate() を呼ぶと必ず 5 秒間待ってしまう(windows では確認出来ていません) cmd.Process.Signal を送り cmd が終了した場合 ProcessState.Exited() は false を返すこことが原因のようです。

下のログはどちらも起動直後に ctrl-c を送ったものです。

before

 goemon git:(master) time goemon sleep 10
GOEMON 2017/01/26 13:47:19 goemon.go:273: open /Users/soh335/dev/src/github.com/mattn/goemon/goemon.yml: no such file or directory
GOEMON 2017/01/26 13:47:19 goemon.go:306: starting command [sleep 10]
GOEMON 2017/01/26 13:47:19 goemon.go:277: loading /Users/soh335/dev/src/github.com/mattn/goemon/goemon.yml
GOEMON 2017/01/26 13:47:19 goemon.go:294: starting livereload
GOEMON 2017/01/26 13:47:19 goemon.go:210: goemon loaded /Users/soh335/dev/src/github.com/mattn/goemon/goemon.yml
^Cgoemon sleep 10  0.02s user 0.04s system 1% cpu 5.684 total

after

 goemon git:(fix/terminate) time goemon sleep 10
GOEMON 2017/01/26 13:47:35 goemon.go:273: open /Users/soh335/dev/src/github.com/mattn/goemon/goemon.yml: no such file or directory
GOEMON 2017/01/26 13:47:35 goemon.go:306: starting command [sleep 10]
GOEMON 2017/01/26 13:47:35 goemon.go:277: loading /Users/soh335/dev/src/github.com/mattn/goemon/goemon.yml
GOEMON 2017/01/26 13:47:35 goemon.go:294: starting livereload
GOEMON 2017/01/26 13:47:35 goemon.go:210: goemon loaded /Users/soh335/dev/src/github.com/mattn/goemon/goemon.yml
^Cgoemon sleep 10  0.02s user 0.02s system 2% cpu 1.703 total

何か勘違いしてたらすいません!

環境

$ go version
go version go1.7.4 darwin/amd64
mattn commented 7 years ago

あざます

mattn commented 7 years ago

いまさらなのですが、この処理は

  1. シグナルを送る
  2. 死ぬまで待つ
  3. 待って死ななかったら Kill で殺す

という処理で、Exited がしばらく false を返すのはいいのかなーと思っています。おそらく CTRL-C した時点で sleep が死んでいたという事ですか?(すいません、先に言えって話ですね)

soh335 commented 7 years ago

シグナルを送って死んだ場合は、Exited はずっと false になってしまうみたいなんです。自分から死んだ場合のみ Exited() が true のようです。 なのでプロセスが終わったかどうかは cmd.Wait が抜ける時に設定する cmd.ProcessState があるかどうかで判断するのが良いのかなという感じです。

mattn commented 7 years ago

お手数なのですが、これ試してもらってもいいでしょうか。

diff --git a/proc_posix.go b/proc_posix.go
index ce7a4fa..3e462b4 100644
--- a/proc_posix.go
+++ b/proc_posix.go
@@ -19,19 +19,13 @@ func (g *goemon) terminate() error {
    if g.cmd != nil && g.cmd.Process != nil {
        if err := g.cmd.Process.Signal(os.Interrupt); err != nil {
            g.Logger.Println(err)
-       } else {
-           cd := 5
-           for cd > 0 {
-               if g.cmd.ProcessState != nil {
-                   break
-               }
-               time.Sleep(time.Second)
-               cd--
-           }
+           return g.cmd.Process.Kill()
        }
-       if g.cmd.ProcessState == nil {
+       t := time.AfterFunc(5*time.Second, func() {
            g.cmd.Process.Kill()
-       }
+       })
+       defer t.Stop()
+       return g.cmd.Wait()
    }
    return nil
 }
soh335 commented 7 years ago

問題なく動いてる気がします。 signal 直後で sleep を一回するのもなくなっているようです。

mattn commented 7 years ago

やたー! thx