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

optionally setsid and transparently use process group control #3

Closed
wants to merge 2 commits into from

Conversation

acg
Copy link
Contributor

@acg acg commented Jan 8, 2012

These changes make managing multiprocess services easier and safer. An example of such a service is one with a long-running shell pipeline in its ./run file. Such services were problematic for classic daemontools and are still somewhat problematic for daemontools-encore.

  • Only call setsid(2) if the s/setsid file exists, to preserve the classic daemontools behavior.
  • If s/setsid exists, make sending of signals to the service's process group the default. Callers of svc(8) don't have to change their invocation to control a service that elects to use a process group.
  • To explicitly send signals only to the process group leader of a multiprocess service, add the '=' option to svc(8). This is similar to the '+' option, which explicitly sends to the service's process group.
  • When supervise(8) receives SIGTERM or SIGINT, propagate this to the supervised process by behaving as if svc -dx had been called. Otherwise a supervised process running in a separate process group will be orphaned. The common problematic scenario is sending ^C to svscan(8) running in the terminal foreground.

These changes make managing multiprocess services easier and safer. An example of such a service is one with a long-running shell pipeline in its ./run file. Such services were problematic for classic daemontools and still somewhat problematic for daemontools-encore.

* Only call setsid(2) if the s/setsid file exists, to preserve the classic daemontools behavior.

* If s/setsid exists, make sending of signals to the service's process group the default. Callers of svc(8) don't have to change their invocation to control a service that elects to use a process group.

* To explicitly send signals only to the process group leader of a multiprocess service, add the '=' option to svc(8). This is similar to the '+' option, which explicitly sends to the service's process group.

* When supervise(8) receives SIGTERM or SIGINT, propagate this to the supervised process by behaving as if svc -dx had been called. Otherwise a supervised process running in a separate process group will be orphaned. The common problematic scenario is sending ^C to svscan(8) running in the terminal foreground.
…process, for proper foreground terminal behavior. otherwise on ^Z multiprocess services using s/setsid will continue to run
@bruceg
Copy link
Owner

bruceg commented Jan 9, 2012

On Sun, Jan 08, 2012 at 03:49:28PM -0800, Alan Grow wrote:

These changes make managing multiprocess services easier and safer. An example of such a service is one with a long-running shell pipeline in its ./run file. Such services were problematic for classic daemontools and are still somewhat problematic for daemontools-encore.

I'd appreciate if these could be broken down into individual changes
instead of one large patch introducing them all.

  • Only call setsid(2) if the s/setsid file exists, to preserve the classic daemontools behavior.

Is there a use case where doing setsid automatically causes problems?
I'd prefer the reverse -- call setsid unless s/no-setsid exists.

  • When supervise(8) receives SIGTERM or SIGINT, propagate this to the supervised process by behaving as if svc -dx had been called. Otherwise a supervised process running in a separate process group will be orphaned. The common problematic scenario is sending ^C to svscan(8) running in the terminal foreground.

This makes sense.

Bruce Guenter bruce@untroubled.org http://untroubled.org/

@acg
Copy link
Contributor Author

acg commented Jan 10, 2012

On Mon, Jan 09, 2012 at 08:15:47AM -0800, Bruce Guenter wrote:

I'd appreciate if these could be broken down into individual changes
instead of one large patch introducing them all.

No problem.

  • Only call setsid(2) if the s/setsid file exists, to preserve the classic daemontools behavior.

Is there a use case where doing setsid automatically causes problems?

Yes. It might break all sorts of things for daemontools users switching to daemontools-encore. Before the patch to relay SIGINT and SIGTERM, if svscan was running as the foreground terminal process and got ^C, supervise would orphan all running services. That and ^Z are mostly taken care of now, but some signals can't be relayed, and signal relaying in general is not reliable. For instance, a classic daemontools user accustomed to sending SIGSTOP to the svscan process group to pause everything can't do this anymore -- even if they don't need the setsid feature, all their services will continue to run. Similar problem for SIGKILL.

In short, "setsid by default" subtly breaks backwards compatibility with daemontools, and so should be a conscious opt-in rather than a surprise.

@timbunce
Copy link

Any news on this?

@acg
Copy link
Contributor Author

acg commented Jul 13, 2012

Going to refactor changes per Bruce's request. Will get around to it late July. I've been using the current patch on production boxes for some time with no problems.

Sent from my iPhone

On Jul 13, 2012, at 2:03 AM, Tim Buncereply@reply.github.com wrote:

Any news on this?


Reply to this email directly or view it on GitHub:
#3 (comment)

@bruceg
Copy link
Owner

bruceg commented Aug 27, 2013

Has there been any progress?

@josb-ebaysf
Copy link

This is a generally useful feature but it seems to have stalled. If someone else does the refactor, is there a chance this will be accepted?

@bruceg
Copy link
Owner

bruceg commented Mar 18, 2014

I am willing to take this if it is refactored.

@acg
Copy link
Contributor Author

acg commented Mar 18, 2014

And I'm still willing to refactor, there's just one point we need to resolve first. @bruceg do you still want setsid to be the default instead of opt-in? This is a behavior change that might take some users by surprise.

@bruceg
Copy link
Owner

bruceg commented Mar 18, 2014

I keep waffling, but I guess restoring the old behavior (no setsid) makes more sense.

FWIW I had never even considered sending signals to the entire svscan process tree at once. I still think it seems like a made up behavior, but if its going to be compatible I need to revert surprises.

@acg
Copy link
Contributor Author

acg commented Mar 24, 2014

Hello all, this change has been refactored and resubmitted as pull request #16 per @bruceg. Except for one thing, no behavior was changed: we're keeping the daemontools-encore behavior of setsid-by-default, because we think multiprocess safety should be opt-out instead of opt-in. You can optionally restore classic daemontools behavior by using a file ./no-setsid. A file ./setsid will have no effect in #16.

@bruceg bruceg closed this Mar 25, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants