bra closes stdout/stderr before finishing stopping child process

[Dieterbe]

when bra runs a program, in this case grafana (https://github.com/raintank/grafana)
and then you kill bra with sigterm, then grafana dies like so:

epoll_wait(10, {}, 128, 0)              = 0
futex(0xc2081ea7d8, FUTEX_WAKE, 1)      = 1
write(2, "2015/08/16 21:50:10 \33[1;32m[I] s"..., 69) = -1 EPIPE (Broken pipe)
--- SIGPIPE {si_signo=SIGPIPE, si_code=SI_USER, si_pid=919, si_uid=0} ---
rt_sigreturn({mask=[]})                 = -1 EPIPE (Broken pipe)
rt_sigaction(SIGPIPE, {SIG_DFL, ~[], SA_RESTORER|SA_STACK|SA_SIGINFO, 0x4d3810}, NULL, 8) = 0
gettid()                                = 919
tkill(919, SIGPIPE)                     = 0
--- SIGPIPE {si_signo=SIGPIPE, si_code=SI_TKILL, si_pid=919, si_uid=0} ---
+++ killed by SIGPIPE +++

a child process (such as grafana here) when asked to terminate (via sigterm, sigint) should be able to write some final messages to stdout/stderr before it calls exit()
but bra closes the file descriptors prematurely, resulting in the sigpipe error.

[Dieterbe]

I think what should probably happen is,
bra should use something like

signal.Notify(c, os.Interrupt)
signal.Notify(c, os.Kill)
signal.Notify(c, syscall.SIGTERM)

when the signal is received, it should kill the child process (using sigterm), but wait until it exits.
perhaps with a timeout of 60 seconds after which it can issue a sigkill.
only after the child process exits, should we close its stdout/stderr.

[unknwon]

Thanks your feedback!

Just to confirm that you are running as bra run and no sudo command is involved, right?

[unknwon]

when the signal is received, it should kill the child process (using sigterm), but wait until it exits.

Here, do you mean send a os.Interrupt and wait for a timeout, then do send os.Kill?

[Dieterbe]

Just to confirm that you are running as bra run and no sudo command is involved, right?

right.

Here, do you mean send a os.Interrupt and wait for a timeout, then do send os.Kill?

either one of sigint or sigterm should work fine to issue a termination request to the child process. they are very similar.
and yes, if the process doesn't terminate itself within a given timeout duration, then send sigkill, it looks like (*Process) Kill from the os package does this for you.

[unknwon]

Thanks for following up!

I got it now, so I'll change bra to send a os.Interrupt first and give child process a chance to exit itself.

[unknwon]

Hi @Dieterbe , I've pushed code for graceful shutdown which first sends os.Interrupt signal and with for timeout(default is 15 seconds but configurable), then send kill signal.

[Dieterbe]

weird. i couldn't reproduce it.
killing bra with bra 0.2 now resulted in the child grafana process just keeping running.
after go get -u github.com/Unknwon/bra, i.e. Bra version 0.3.3.0903 i have the same behavior. child process just keeps running :?

[unknwon]

child process just keeps running :?

Hi, I've met this issue before, so if your child process handles kill signal itself, bra can't control it, and of course child process captures your ctrl+c and handles it as well.

[Dieterbe]

no i sent sigterm (via kill) to bra, bra stopped, i didn't send anything to the child process (though bra should have)

[unknwon]

no i sent sigterm (via kill) to bra, bra stopped, i didn't send anything to the child process (though bra should have)

Yes, but if your child process handles sigterm, bra still exits as it should and can't do anything to child process anymore.

[Dieterbe]

it looks like child process is not getting a signal at all from bra. because if i send it sigterm (kill) or sigint (kill -int), it successfully exits in both cases within the second. when running under bra, i was stracing the child process and also looking at its log output. when i killed bra i couldn't find evidence of the child process receiving a signal from bra.

[unknwon]

bra does not send sigterm to child process.

But from what I understood and experienced, your sigterm is passed by something in the middle to your child process, and your child process should exit itself.

Not pro on this topic, can't really tell what's in the black box...

But, I can let Bra captures your sigterm and pass it to child process.

[Dieterbe]

there is nothing "in the middle". i have bra just run the grafana process, which i monitor.
but I think i see what's wrong. looking at the bra code, i see that it only sends a signal (sigint) when files change, not when bra itself is exiting. gracefulKill should also be invoked when bra exits.

[unknwon]

Yeah, that's good idea.

[unknwon]

Hi @Dieterbe , I've pushed code to master, please help test!

[Dieterbe]

i can confirm using the latest bra, when bra is asked to exit and shuts down, grafana does not suffer sigpipe anymore, it is given the chance to write some stuff to stdout and exit cleanly.

nice! thank you.

[unknwon]

Thanks your confirmation!