Browse Source

Fix signal blocking behaviour

Serj Kalichev 8 years ago
parent
commit
cca1d66a01
4 changed files with 53 additions and 59 deletions
  1. 1 1
      clish/shell.h
  2. 51 37
      clish/shell/shell_execute.c
  3. 1 1
      clish/shell/shell_var.c
  4. 0 20
      plugins/clish/sym_script.c

+ 1 - 1
clish/shell.h

@@ -109,7 +109,7 @@ clish_ptype_t *clish_shell_find_create_ptype(clish_shell_t * instance,
 clish_ptype_t *clish_shell_find_ptype(clish_shell_t *instance,
 	const char *name);
 void clish_shell_help(clish_shell_t * instance, const char *line);
-int clish_shell_exec_action(clish_context_t *context, char **out);
+int clish_shell_exec_action(clish_context_t *context, char **out, bool_t intr);
 int clish_shell_execute(clish_context_t *context, char **out);
 int clish_shell_forceline(clish_shell_t *instance, const char *line, char ** out);
 int clish_shell_readline(clish_shell_t *instance, char ** out);

+ 51 - 37
clish/shell/shell_execute.c

@@ -19,6 +19,13 @@
 #include <signal.h>
 #include <fcntl.h>
 
+/* Empty signal handler to ignore signal but don't use SIG_IGN. */
+static void sigignore(int signo)
+{
+	signo = signo; /* Happy compiler */
+	return;
+}
+
 /*-------------------------------------------------------- */
 static int clish_shell_lock(const char *lock_path)
 {
@@ -87,8 +94,6 @@ int clish_shell_execute(clish_context_t *context, char **out)
 	int result = 0;
 	char *lock_path = clish_shell__get_lockfile(this);
 	int lock_fd = -1;
-	sigset_t old_sigs;
-	struct sigaction old_sigint, old_sigquit, old_sighup;
 	clish_view_t *cur_view = clish_shell__get_view(this);
 	unsigned int saved_wdog_timeout = this->wdog_timeout;
 
@@ -116,42 +121,10 @@ int clish_shell_execute(clish_context_t *context, char **out)
 		}
 	}
 
-	/* Ignore and block SIGINT, SIGQUIT, SIGHUP */
-	if (!clish_command__get_interrupt(cmd)) {
-		struct sigaction sa;
-		sigset_t sigs;
-		sa.sa_flags = 0;
-		sigemptyset(&sa.sa_mask);
-		sa.sa_handler = SIG_IGN;
-		sigaction(SIGINT, &sa, &old_sigint);
-		sigaction(SIGQUIT, &sa, &old_sigquit);
-		sigaction(SIGHUP, &sa, &old_sighup);
-		sigemptyset(&sigs);
-		sigaddset(&sigs, SIGINT);
-		sigaddset(&sigs, SIGQUIT);
-		sigaddset(&sigs, SIGHUP);
-		sigprocmask(SIG_BLOCK, &sigs, &old_sigs);
-	}
-
 	/* Execute ACTION */
 	clish_context__set_action(context, clish_command__get_action(cmd));
-	result = clish_shell_exec_action(context, out);
-
-	/* Restore SIGINT, SIGQUIT, SIGHUP */
-	if (!clish_command__get_interrupt(cmd)) {
-		sigprocmask(SIG_SETMASK, &old_sigs, NULL);
-		/* Is the signals delivery guaranteed here (before
-		   sigaction restore) for previously blocked and
-		   pending signals? The simple test is working well.
-		   I don't want to use sigtimedwait() function bacause
-		   it needs a realtime extensions. The sigpending() with
-		   the sleep() is not nice too. Report bug if clish will
-		   get the SIGINT after non-interruptable action.
-		*/
-		sigaction(SIGINT, &old_sigint, NULL);
-		sigaction(SIGQUIT, &old_sigquit, NULL);
-		sigaction(SIGHUP, &old_sighup, NULL);
-	}
+	result = clish_shell_exec_action(context, out,
+		clish_command__get_interrupt(cmd));
 
 	/* Call config callback */
 	if (!result)
@@ -310,7 +283,7 @@ stdout_error:
 }
 
 /*----------------------------------------------------------- */
-int clish_shell_exec_action(clish_context_t *context, char **out)
+int clish_shell_exec_action(clish_context_t *context, char **out, bool_t intr)
 {
 	int result = -1;
 	clish_sym_t *sym;
@@ -318,6 +291,10 @@ int clish_shell_exec_action(clish_context_t *context, char **out)
 	void *func = NULL; /* We don't know the func API at this time */
 	const clish_action_t *action = clish_context__get_action(context);
 	clish_shell_t *shell = clish_context__get_shell(context);
+	/* Signal vars */
+	struct sigaction old_sigint, old_sigquit, old_sighup;
+	struct sigaction sa;
+	sigset_t old_sigs;
 
 	if (!(sym = clish_action__get_builtin(action)))
 		return 0;
@@ -329,6 +306,27 @@ int clish_shell_exec_action(clish_context_t *context, char **out)
 	}
 	script = clish_shell_expand(clish_action__get_script(action), SHELL_VAR_ACTION, context);
 
+	/* Ignore and block SIGINT, SIGQUIT, SIGHUP.
+	 * The SIG_IGN is not a case because it will be inherited
+	 * while a fork(). It's necessary to ignore signals because
+	 * the klish itself and ACTION script share the same terminal.
+	 */
+	sa.sa_flags = 0;
+	sigemptyset(&sa.sa_mask);
+	sa.sa_handler = sigignore; /* Empty signal handler */
+	sigaction(SIGINT, &sa, &old_sigint);
+	sigaction(SIGQUIT, &sa, &old_sigquit);
+	sigaction(SIGHUP, &sa, &old_sighup);
+	/* Block signals for children processes. The block state is inherited. */
+	if (!intr) {
+		sigset_t sigs;
+		sigemptyset(&sigs);
+		sigaddset(&sigs, SIGINT);
+		sigaddset(&sigs, SIGQUIT);
+		sigaddset(&sigs, SIGHUP);
+		sigprocmask(SIG_BLOCK, &sigs, &old_sigs);
+	}
+
 	/* Find out the function API */
 	/* CLISH_SYM_API_SIMPLE */
 	if (clish_sym__get_api(sym) == CLISH_SYM_API_SIMPLE) {
@@ -344,6 +342,22 @@ int clish_shell_exec_action(clish_context_t *context, char **out)
 			context, script, out);
 	}
 
+	/* Restore SIGINT, SIGQUIT, SIGHUP */
+	if (!intr) {
+		sigprocmask(SIG_SETMASK, &old_sigs, NULL);
+		/* Is the signals delivery guaranteed here (before
+		   sigaction restore) for previously blocked and
+		   pending signals? The simple test is working well.
+		   I don't want to use sigtimedwait() function because
+		   it needs a realtime extensions. The sigpending() with
+		   the sleep() is not nice too. Report bug if clish will
+		   get the SIGINT after non-interruptable action.
+		*/
+	}
+	sigaction(SIGINT, &old_sigint, NULL);
+	sigaction(SIGQUIT, &old_sigquit, NULL);
+	sigaction(SIGHUP, &old_sighup, NULL);
+
 	lub_string_free(script);
 
 	return result;

+ 1 - 1
clish/shell/shell_var.c

@@ -173,7 +173,7 @@ static char *find_var(const char *name, lub_bintree_t *tree, clish_context_t *co
 	if (!res) {
 		char *out = NULL;
 		clish_context__set_action(context, clish_var__get_action(var));
-		if (clish_shell_exec_action(context, &out)) {
+		if (clish_shell_exec_action(context, &out, BOOL_FALSE)) {
 			lub_string_free(out);
 			return NULL;
 		}

+ 0 - 20
plugins/clish/sym_script.c

@@ -31,10 +31,6 @@ CLISH_PLUGIN_OSYM(clish_script)
 	const char *fifo_name;
 	FILE *wpipe;
 	char *command = NULL;
-	struct sigaction sig_old_int;
-	struct sigaction sig_old_quit;
-	struct sigaction sig_new;
-	sigset_t sig_set;
 
 	assert(this);
 	if (!script) /* Nothing to do */
@@ -84,24 +80,8 @@ CLISH_PLUGIN_OSYM(clish_script)
 	lub_string_cat(&command, " ");
 	lub_string_cat(&command, fifo_name);
 
-	/* Ignore SIGINT and SIGQUIT */
-	/* Probably this code is necessary to don't get SIGINT
-	 * from executed script. Because the executed script
-	 * and klish have the same terminal.
-	 */
-	sigemptyset(&sig_set);
-	sig_new.sa_flags = 0;
-	sig_new.sa_mask = sig_set;
-	sig_new.sa_handler = SIG_IGN;
-	sigaction(SIGINT, &sig_new, &sig_old_int);
-	sigaction(SIGQUIT, &sig_new, &sig_old_quit);
-
 	res = system(command);
 
-	/* Restore SIGINT and SIGQUIT */
-	sigaction(SIGINT, &sig_old_int, NULL);
-	sigaction(SIGQUIT, &sig_old_quit, NULL);
-
 	/* Wait for the writing process */
 	kill(cpid, SIGTERM);
 	waitpid(cpid, NULL, 0);