[RFC] stopsignal stanza
Marc - A. Dahlhaus
mad at wol.de
Mon May 2 21:50:04 UTC 2011
Am 02.05.2011 20:33, schrieb Scott James Remnant:
> On Mon, May 2, 2011 at 11:31 AM, Marc - A. Dahlhaus <mad at wol.de
> <mailto:mad at wol.de>> wrote:
>
> Am 02.05.2011 19:58, schrieb Scott James Remnant:
> --8<--
> > Talked about the topic with SpamapS on irc and he suggested
> that we
> > choos another stanza for it as there is a bug open for sending
> another
> > signal than HUP on reload...
> >
> > Maybe we better use "signal {stop,reload} SIGNAL" for it?!
> >
> > I would prefer:
> >
> > kill signal HUP
> >
> > since that goes better with timeout, etc. There should already be the
> > code to translate signal names (normal exit uses it)
>
> Nice. For the messages that now mention "sending TERM signal", we need a
> intval to signalname translation, is there one in nih?
>
> Yup, in nih/signal.h - nih_signal_to_name
>
>
> >
> > Then we could also rename the system_kill function to
> system_signal_send
> > and reuse it for the reloading also...
>
> That renaming ok for you?
>
> system_send_signal () is less yoda ;-)
>
> Scott
This is the patch with a first added test that it does what it should.
The comments are also changed to reflect the changes made.
Will add some more tests to the testsuite tomorrow and get some sleep now.
Marc
--- a/init/errors.h
+++ b/init/errors.h
@@ -40,6 +40,7 @@ enum {
/* Errors while parsing configuration files */
PARSE_ILLEGAL_INTERVAL,
PARSE_ILLEGAL_EXIT,
+ PARSE_ILLEGAL_SIGNAL,
PARSE_ILLEGAL_UMASK,
PARSE_ILLEGAL_NICE,
PARSE_ILLEGAL_OOM,
@@ -60,6 +61,7 @@ enum {
#define ENVIRON_MISMATCHED_BRACES_STR N_("Mismatched braces")
#define PARSE_ILLEGAL_INTERVAL_STR N_("Illegal interval, expected
number of seconds")
#define PARSE_ILLEGAL_EXIT_STR N_("Illegal exit status, expected integer")
+#define PARSE_ILLEGAL_SIGNAL_STR N_("Illegal signal status, expected
integer")
#define PARSE_ILLEGAL_UMASK_STR N_("Illegal file creation mask,
expected octal integer")
#define PARSE_ILLEGAL_NICE_STR N_("Illegal nice value, expected -20 to
19")
#define PARSE_ILLEGAL_OOM_STR N_("Illegal oom adjustment, expected -16
to 15 or never")
--- a/init/job_class.c
+++ b/init/job_class.c
@@ -26,6 +26,7 @@
#include <errno.h>
#include <string.h>
+#include <signal.h>
#include <nih/macros.h>
#include <nih/alloc.h>
@@ -199,6 +200,7 @@ job_class_new (const void *parent,
class->task = FALSE;
class->kill_timeout = JOB_DEFAULT_KILL_TIMEOUT;
+ class->kill_signal = SIGTERM;
class->respawn = FALSE;
class->respawn_limit = JOB_DEFAULT_RESPAWN_LIMIT;
--- a/init/job_class.h
+++ b/init/job_class.h
@@ -129,6 +129,7 @@ typedef struct job_class {
int task;
time_t kill_timeout;
+ int kill_signal;
int respawn;
int respawn_limit;
--- a/init/job_process.c
+++ b/init/job_process.c
@@ -769,9 +769,9 @@ job_process_error_read (int fd)
* @process: process to be killed.
*
* This function forces a @job to leave its current state by sending
- * @process the TERM signal, and maybe later the KILL signal. The actual
- * state changes are performed by job_child_reaper when the process
- * has actually terminated.
+ * @process the "kill signal" defined signal (TERM by default), and maybe
+ * later the KILL signal. The actual state changes are performed by
+ * job_child_reaper when the process has actually terminated.
**/
void
job_process_kill (Job *job,
@@ -782,15 +782,17 @@ job_process_kill (Job *job,
nih_assert (job->kill_timer == NULL);
nih_assert (job->kill_process = -1);
- nih_info (_("Sending TERM signal to %s %s process (%d)"),
+ nih_info (_("Sending %s signal to %s %s process (%d)"),
+ nih_signal_to_name (job->class->kill_signal),
job_name (job), process_name (process), job->pid[process]);
- if (system_kill (job->pid[process], FALSE) < 0) {
+ if (system_send_signal (job->pid[process], job->class->kill_signal) < 0) {
NihError *err;
err = nih_error_get ();
if (err->number != ESRCH)
- nih_warn (_("Failed to send TERM signal to %s %s process (%d): %s"),
+ nih_warn (_("Failed to send %s signal to %s %s process (%d): %s"),
+ nih_signal_to_name (job->class->kill_signal),
job_name (job), process_name (process),
job->pid[process], err->message);
nih_free (err);
@@ -833,7 +835,7 @@ job_process_kill_timer (Job *job,
nih_info (_("Sending KILL signal to %s %s process (%d)"),
job_name (job), process_name (process), job->pid[process]);
- if (system_kill (job->pid[process], TRUE) < 0) {
+ if (system_send_signal (job->pid[process], SIGKILL) < 0) {
NihError *err;
err = nih_error_get ();
--- a/init/parse_job.c
+++ b/init/parse_job.c
@@ -1782,6 +1782,7 @@ stanza_kill (JobClass *class,
{
size_t a_pos, a_lineno;
int ret = -1;
+ char *endptr;
nih_local char *arg = NULL;
nih_assert (class != NULL);
@@ -1799,7 +1800,6 @@ stanza_kill (JobClass *class,
if (! strcmp (arg, "timeout")) {
nih_local char *timearg = NULL;
- char *endptr;
/* Update error position to the timeout value */
*pos = a_pos;
@@ -1816,14 +1816,40 @@ stanza_kill (JobClass *class,
if (errno || *endptr || (class->kill_timeout < 0))
nih_return_error (-1, PARSE_ILLEGAL_INTERVAL,
_(PARSE_ILLEGAL_INTERVAL_STR));
+ } else if (! strcmp (arg, "signal")) {
+ unsigned long status;
+ nih_local char *sigarg = NULL;
+ int signal;
- ret = nih_config_skip_comment (file, len, &a_pos, &a_lineno);
+ /* Update error position to the exit status */
+ *pos = a_pos;
+ if (lineno)
+ *lineno = a_lineno;
+
+ sigarg = nih_config_next_arg (NULL, file, len, &a_pos,
+ &a_lineno);
+ if (! sigarg)
+ goto finish;
+
+ signal = nih_signal_from_name (sigarg);
+ if (signal < 0) {
+ errno = 0;
+ status = strtoul (sigarg, &endptr, 10);
+ if (errno || *endptr || (status > INT_MAX))
+ nih_return_error (-1, PARSE_ILLEGAL_SIGNAL,
+ _(PARSE_ILLEGAL_SIGNAL_STR));
+ }
+
+ /* Set the signal */
+ class->kill_signal = signal;
} else {
nih_return_error (-1, NIH_CONFIG_UNKNOWN_STANZA,
_(NIH_CONFIG_UNKNOWN_STANZA_STR));
}
+ ret = nih_config_skip_comment (file, len, &a_pos, &a_lineno);
+
finish:
*pos = a_pos;
if (lineno)
--- a/init/system.c
+++ b/init/system.c
@@ -46,29 +46,23 @@
/**
- * system_kill:
+ * system_send_signal:
* @pid: process id of process,
- * @force: force the death.
+ * @signal: signal to send.
*
- * Kill all processes in the same process group as @pid, which may not
- * necessarily be the group leader.
- *
- * When @force is FALSE, the TERM signal is sent; when it is TRUE, KILL
- * is sent instead.
+ * Send all processes in the same process group as @pid, which may not
+ * necessarily be the group leader the @signal.
*
* Returns: zero on success, negative value on raised error.
**/
int
-system_kill (pid_t pid,
- int force)
+system_send_signal (pid_t pid,
+ int signal)
{
- int signal;
pid_t pgid;
nih_assert (pid > 0);
- signal = (force ? SIGKILL : SIGTERM);
-
pgid = getpgid (pid);
if (kill (pgid > 0 ? -pgid : pid, signal) < 0)
--- a/init/system.h
+++ b/init/system.h
@@ -29,7 +29,7 @@
NIH_BEGIN_EXTERN
-int system_kill (pid_t pid, int force)
+int system_send_signal (pid_t pid, int signal)
__attribute__ ((warn_unused_result));
int system_setup_console (ConsoleType type, int reset)
--- a/init/tests/test_parse_job.c
+++ b/init/tests/test_parse_job.c
@@ -4983,6 +4983,38 @@ test_stanza_kill (void)
TEST_EQ (pos, 16);
TEST_EQ (lineno, 1);
nih_free (err);
+
+ /* Check that a kill stanza with the signal argument and signal,
+ * sets the right signal on the jobs class.
+ */
+ TEST_FEATURE ("with signal and argument");
+ strcpy (buf, "kill signal INT\n");
+
+ TEST_ALLOC_FAIL {
+ pos = 0;
+ lineno = 1;
+ job = parse_job (NULL, "test", buf, strlen (buf),
+ &pos, &lineno);
+
+ if (test_alloc_failed) {
+ TEST_EQ_P (job, NULL);
+
+ err = nih_error_get ();
+ TEST_EQ (err->number, ENOMEM);
+ nih_free (err);
+
+ continue;
+ }
+
+ TEST_EQ (pos, strlen (buf));
+ TEST_EQ (lineno, 2);
+
+ TEST_ALLOC_SIZE (job, sizeof (JobClass));
+
+ TEST_EQ (job->kill_signal, SIGINT);
+
+ nih_free (job);
+ }
}
void
--- a/init/tests/test_system.c
+++ b/init/tests/test_system.c
@@ -35,7 +35,7 @@ test_kill (void)
pid_t pid1, pid2, pid3;
int ret, status;
- TEST_FUNCTION ("system_kill");
+ TEST_FUNCTION ("system_send_signal");
/* Check that when we normally kill the process, the TERM signal
* is sent to all processes in its process group.
@@ -51,7 +51,7 @@ test_kill (void)
setpgid (pid1, pid1);
setpgid (pid2, pid1);
- ret = system_kill (pid1, FALSE);
+ ret = system_send_signal (pid1, SIGTERM);
waitpid (pid1, &status, 0);
TEST_EQ (ret, 0);
@@ -79,7 +79,7 @@ test_kill (void)
setpgid (pid1, pid1);
setpgid (pid2, pid1);
- ret = system_kill (pid1, TRUE);
+ ret = system_send_signal (pid1, SIGKILL);
waitpid (pid1, &status, 0);
TEST_EQ (ret, 0);
@@ -114,7 +114,7 @@ test_kill (void)
kill (pid1, SIGTERM);
waitpid (pid1, &status, 0);
- ret = system_kill (pid2, FALSE);
+ ret = system_send_signal (pid2, SIGTERM);
waitpid (pid2, &status, 0);
TEST_EQ (ret, 0);
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: kill_signal_stanza.patch
URL: <https://lists.ubuntu.com/archives/upstart-devel/attachments/20110502/7d3a9c1d/attachment-0001.ksh>
More information about the upstart-devel
mailing list