Skip to content
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

Is SIGTERM reaching the app? #5

Open
stuartpb opened this issue Jan 18, 2015 · 12 comments
Open

Is SIGTERM reaching the app? #5

stuartpb opened this issue Jan 18, 2015 · 12 comments

Comments

@stuartpb
Copy link
Member

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

@stuartpb
Copy link
Member Author

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 #5 (comment).)

@stuartpb
Copy link
Member Author

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
Copy link
Member Author

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
Copy link
Member Author

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
Copy link
Member Author

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
Copy link
Member Author

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
Copy link
Member Author

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
Copy link
Member Author

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
Copy link
Member Author

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
Copy link
Member Author

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
Copy link
Member Author

THAT'S THE PROBLEM npm/npm#4603

@stuartpb
Copy link
Member Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant