plushu / plushu-build-cedarish

Plugin that builds apps in a Cedar-ish environment
MIT License
0 stars 0 forks source link

Is SIGTERM reaching the app? #5

Open stuartpb opened 9 years ago

stuartpb commented 9 years ago

Stopping the old container seems to always time out. Is SIGTERM not being forwarded to the app we start?

stuartpb commented 9 years ago

Looks like this is definitely the case due to the way we don't exec the app at runtime. Looking into fixing this...

Important note: no, we totally do exec the app at runtime, that's how bash -c works. Jump ahead to https://github.com/plushu/plushu-build-cedarish/issues/5#issuecomment-94965246.)

stuartpb commented 9 years ago

Since the command in the Procfile is itself a Bash command (ie. a script), I'm not certain there's a way around this (other than to have everybody go around and start adding "exec" to their Procfiles).

stuartpb commented 9 years ago

Actually, wait a tic, are Procfiles Bash scripts? I shouldn't think they are, actually. It seems more likely to me that they'd be processes and arguments (with maybe some shell expansion).

More about this problem in progrium/buildstep#114

stuartpb commented 9 years ago

So, from buildstep's comments, it sounds like the cleanest way to solve this is to vendor in the forego binary and use that instead of a Bash script.

stuartpb commented 9 years ago

Actually, you know what? We can do better, by piping the Ruby procfile read to xargs -x /exec & PID=$!; wait.

OTOH, does forego handle shell expansion? Is that a documented feature of Procfiles?

stuartpb commented 9 years ago

http://veithen.github.io/2014/11/16/sigterm-propagation.html seems like it might be necessary (if we want to keep bash around as a hypervisor).

stuartpb commented 9 years ago

Am I confusing SIGINT with SIGTERM? Does Bash actually forward SIGTERM to a job that is running and not backgrounded?

See http://www.vidarholen.net/contents/blog/?p=34, http://git.savannah.gnu.org/cgit/bash.git/tree/jobs.c#n2411

stuartpb commented 9 years ago

There might also be something to running the bash command inside another bash command that runs the second Bash command asynchronously and kills the inner Bash command's process group (kill -$pid rather than kill $pid) when receiving a signal.

http://stackoverflow.com/questions/2525855/how-to-propagate-a-signal-through-an-arborescence-of-scripts-bash

stuartpb commented 9 years ago

Per all the whining I just did in progrium/buildstep#102, running the command via bash -c should inherently be execing the command, as long as it's not something complex. This is only a behavior I'm observing in my Node apps (with no Procfile) - is there something broken in the default Node start? (If it's npm start, is there something broken in there?)

stuartpb commented 9 years ago

Okay, this is definitely not doing anything more complex than npm start: https://github.com/heroku/heroku-buildpack-nodejs/blob/master/lib/build.sh

So the question becomes: is npm start not forwarding signals?

stuartpb commented 9 years ago

THAT'S THE PROBLEM npm/npm#4603

stuartpb commented 9 years ago

Moving to rectify: heroku/heroku-buildpack-nodejs#217