Relay SIGTERM from docker stop to the child processes of /start

[jdoconnor]

I'm not sure if anyone else wants something like this, but I needed to make it to ensure that the child processes that were spawned by the start script were able to receive the SIGTERM that docker stop sends.

idea (and most of the code) from https://github.com/statianzo/dokku-shoreman/blob/master/post-release

[progrium]

This is a good idea but we might have to work on the implementation. Let's revisit after we get #109 in

[progrium]

Is this issue still open after merging #109 in? Either way, if we want to pull this in, it'll have to rebase.

[stuartpb]

This is most likely still an issue (unless /start has been swapped out for forego). See also plushu/plushu-build-cedarish#5.

It would seem this is an unsolveable problem due to #114, which is inherently diametrically opposed to this (as noted in #114 (comment)).

[stuartpb]

Okay, so I just looked at the way forego does it (traceback below), and what they do is basically construct a Bash script (passed to Bash with either -c or -ic depending on whether the -i flag is set) that sources all scripts in .profile, then executes whatever the command in the procfile is. This is what @ddollar wrote, and if anybody knows what a process runner has to do to have feature parity with Heroku, it's him.

• in https://github.com/ddollar/forego/blob/master/unix.go#L13
• from https://github.com/ddollar/forego/blob/master/process.go#L18
• from https://github.com/ddollar/forego/blob/master/start.go#L123

That said, having all Procfile entries implicitly be shell commands seems a bit wrong, since you should be able to have a 1:1 process correspondence, to be run with a hypervisor like, y'know, Docker. If Heroku's using shell scripts like this, then how are they stopping apps? Are they just sending them SIGKILL instead of stopping them (since it's basically impossible to forward a signal to a shell command)?

[stuartpb]

One possibility occurs to me: do other shells forward signals to their subprocesses (is dropping signals on the floor while waiting a Bashism quirk)? If so, you can just exec the command with that shell and be safe.

[stuartpb]

Also, Bash appears to have different signal handling when set -m (monitor mode) is activated, and all processes run in a separate process group. Maybe the secret of proper signal propagation lies somewhere in there?

[stuartpb]

Maybe docker kill (and/or docker stop) needs a flag to send the signal to the entire process group and not just the top-level process?

[stuartpb]

The inescapable conclusion, for me, is that any Procfile command that uses shell features needs to be rewritten to be wrapped in bash -c "exec <...>" - the inherent shell is a misfeature, mostly because of the way it eats signals.

In other words, any Procfile entry should be usable as a list of arguments to docker run.

Yes, this may break some apps, or even some buildpacks, but re-introducing a shell wrapper is easy, and it lets the user insert an exec where necessary.

[stuartpb]

Well, on the subject of Procfile jobs supporting Bash scripts, on the, uh... hand that isn't the hand that supports that, https://github.com/heroku/hsup uses proc descriptions that are explicitly a binary and a list of arguments, which would imply that Procfiles are not meant to hold Bash scripts. (The way hsup converts the "commands" given by the API to this array appears to be by coercing the "commands" string returned by the API to an array, which I presume splits by fields: https://github.com/heroku/hsup/blob/master/api_poller.go#L20)

[stuartpb]

Wait a second, that's an ordinary array constructor, taking a string... it just drops the command directly into an array of one item? WTF?

[stuartpb]

And apparently it used to do this by converting the command to a Bash command as well, and this is a change that was made a couple months ago- and now hsup start just drops all of its args into a Bash command and crosses its fingers. AAAARGH heroku/hsup@58189a2

[stuartpb]

Actually, the commit message to heroku/hsup@edbe4a1 has reminded me: bash will (effectively) automatically insert exec in front of anything that is given to bash -c when it's a single command. So it's really just a matter of execing the bash call.

... Hey, wait a minute, I'm doing that already! What gives?

[stuartpb]

Okay, final update: all my confusion around here was really just misdirected blame toward npm/npm#4603. Plushu (and probably whatever Dokku does now) is fine at relaying signals to child processes, because they exec through to that child process. The trouble comes when that child process doesn't exec through.