Merge pull request #1920 from brauner/2017-11-12/console_exit_clean_on_sigterm

src/lxc/console.c

@@ -59,25 +59,39 @@ static struct lxc_list lxc_ttys;
 
 typedef void (*sighandler_t)(int);
 
-__attribute__((constructor))
-void lxc_console_init(void)
+__attribute__((constructor)) void lxc_console_init(void)
 {
 	lxc_list_init(&lxc_ttys);
 }
 
 void lxc_console_winsz(int srcfd, int dstfd)
 {
+	int ret;
 	struct winsize wsz;
-	if (isatty(srcfd) && ioctl(srcfd, TIOCGWINSZ, &wsz) == 0) {
-		DEBUG("set winsz dstfd:%d cols:%d rows:%d", dstfd,
-		      wsz.ws_col, wsz.ws_row);
-		ioctl(dstfd, TIOCSWINSZ, &wsz);
+
+	if (!isatty(srcfd))
+		return;
+
+	ret = ioctl(srcfd, TIOCGWINSZ, &wsz);
+	if (ret < 0) {
+		WARN("Failed to get window size");
+		return;
 	}
+
+	ret = ioctl(dstfd, TIOCSWINSZ, &wsz);
+	if (ret < 0)
+		WARN("Failed to set window size");
+	else
+		DEBUG("Set window size to %d columns and %d rows", wsz.ws_col,
+		      wsz.ws_row);
+
+	return;
 }
 
 static void lxc_console_winch(struct lxc_tty_state *ts)
 {
 	lxc_console_winsz(ts->stdinfd, ts->masterfd);
+
 	if (ts->winch_proxy)
 		lxc_cmd_console_winch(ts->winch_proxy, ts->winch_proxy_lxcpath);
 }
@@ -93,24 +107,34 @@ void lxc_console_sigwinch(int sig)
 	}
 }
 
-int lxc_console_cb_sigwinch_fd(int fd, uint32_t events, void *cbdata,
-		struct lxc_epoll_descr *descr)
+int lxc_console_cb_signal_fd(int fd, uint32_t events, void *cbdata,
+			       struct lxc_epoll_descr *descr)
 {
+	ssize_t ret;
 	struct signalfd_siginfo siginfo;
 	struct lxc_tty_state *ts = cbdata;
 
-	ssize_t ret = read(fd, &siginfo, sizeof(siginfo));
+	ret = read(fd, &siginfo, sizeof(siginfo));
 	if (ret < 0 || (size_t)ret < sizeof(siginfo)) {
-		ERROR("failed to read signal info");
+		ERROR("Failed to read signal info");
 		return -1;
 	}
 
-	lxc_console_winch(ts);
+	if (siginfo.ssi_signo == SIGTERM) {
+		DEBUG("Received SIGTERM. Detaching from the console");
+		return 1;
+	}
+
+	if (siginfo.ssi_signo == SIGWINCH)
+		lxc_console_winch(ts);
+
 	return 0;
 }
 
-struct lxc_tty_state *lxc_console_sigwinch_init(int srcfd, int dstfd)
+struct lxc_tty_state *lxc_console_signal_init(int srcfd, int dstfd)
 {
+	int ret;
+	bool istty;
 	sigset_t mask;
 	struct lxc_tty_state *ts;
 
@@ -123,45 +147,60 @@ struct lxc_tty_state *lxc_console_sigwinch_init(int srcfd, int dstfd)
 	ts->masterfd = dstfd;
 	ts->sigfd = -1;
 
-	if (!isatty(srcfd)) {
+	sigemptyset(&mask);
+
+	istty = isatty(srcfd) == 1;
+	if (!istty) {
 		INFO("fd %d does not refer to a tty device", srcfd);
-		return ts;
+	} else {
+		/* Add tty to list to be scanned at SIGWINCH time. */
+		lxc_list_add_elem(&ts->node, ts);
+		lxc_list_add_tail(&lxc_ttys, &ts->node);
+		sigaddset(&mask, SIGWINCH);
 	}
 
-	/* add tty to list to be scanned at SIGWINCH time */
-	lxc_list_add_elem(&ts->node, ts);
-	lxc_list_add_tail(&lxc_ttys, &ts->node);
+	/* Exit the mainloop cleanly on SIGTERM. */
+	sigaddset(&mask, SIGTERM);
 
-	sigemptyset(&mask);
-	sigaddset(&mask, SIGWINCH);
-	if (sigprocmask(SIG_BLOCK, &mask, &ts->oldmask)) {
-		SYSERROR("failed to block SIGWINCH");
-		ts->sigfd = -1;
-		lxc_list_del(&ts->node);
-		return ts;
+	ret = sigprocmask(SIG_BLOCK, &mask, &ts->oldmask);
+	if (ret < 0) {
+		WARN("Failed to block signals");
+		goto on_error;
 	}
 
 	ts->sigfd = signalfd(-1, &mask, 0);
 	if (ts->sigfd < 0) {
-		SYSERROR("failed to create signal fd");
+		WARN("Failed to create signal fd");
 		sigprocmask(SIG_SETMASK, &ts->oldmask, NULL);
-		ts->sigfd = -1;
-		lxc_list_del(&ts->node);
-		return ts;
+		goto on_error;
 	}
 
-	DEBUG("process %d created signal fd %d to handle SIGWINCH events", getpid(), ts->sigfd);
+	DEBUG("Created signal fd %d", ts->sigfd);
+	return ts;
+
+on_error:
+	ERROR("Failed to create signal fd");
+	if (ts->sigfd >= 0) {
+		close(ts->sigfd);
+		ts->sigfd = -1;
+	}
+	if (istty)
+		lxc_list_del(&ts->node);
 	return ts;
 }
 
-void lxc_console_sigwinch_fini(struct lxc_tty_state *ts)
+void lxc_console_signal_fini(struct lxc_tty_state *ts)
 {
 	if (ts->sigfd >= 0) {
 		close(ts->sigfd);
-		lxc_list_del(&ts->node);
-		sigprocmask(SIG_SETMASK, &ts->oldmask, NULL);
+
+		if (sigprocmask(SIG_SETMASK, &ts->oldmask, NULL) < 0)
+			WARN("%s - Failed to restore signal mask", strerror(errno));
 	}
 
+	if (isatty(ts->stdinfd))
+		lxc_list_del(&ts->node);
+
 	free(ts);
 }
 
@@ -178,7 +217,7 @@ static int lxc_console_cb_con(int fd, uint32_t events, void *data,
 		lxc_mainloop_del_handler(descr, fd);
 		if (fd == console->peer) {
 			if (console->tty_state) {
-				lxc_console_sigwinch_fini(console->tty_state);
+				lxc_console_signal_fini(console->tty_state);
 				console->tty_state = NULL;
 			}
 			console->peer = -1;
@@ -225,16 +264,15 @@ static void lxc_console_mainloop_add_peer(struct lxc_console *console)
 	if (console->peer >= 0) {
 		if (lxc_mainloop_add_handler(console->descr, console->peer,
 					     lxc_console_cb_con, console))
-			WARN("console peer not added to mainloop");
+			WARN("Failed to add console peer handler to mainloop");
 	}
 
 	if (console->tty_state && console->tty_state->sigfd != -1) {
 		if (lxc_mainloop_add_handler(console->descr,
 					     console->tty_state->sigfd,
-					     lxc_console_cb_sigwinch_fd,
+					     lxc_console_cb_signal_fd,
 					     console->tty_state)) {
-			WARN("failed to add to mainloop SIGWINCH handler for '%d'",
-			     console->tty_state->sigfd);
+			WARN("Failed to add signal handler to mainloop");
 		}
 	}
 }
@@ -321,7 +359,7 @@ int lxc_setup_tios(int fd, struct termios *oldtios)
 static void lxc_console_peer_proxy_free(struct lxc_console *console)
 {
 	if (console->tty_state) {
-		lxc_console_sigwinch_fini(console->tty_state);
+		lxc_console_signal_fini(console->tty_state);
 		console->tty_state = NULL;
 	}
 	close(console->peerpty.master);
@@ -367,7 +405,7 @@ static int lxc_console_peer_proxy_alloc(struct lxc_console *console, int sockfd)
 	if (lxc_setup_tios(console->peerpty.slave, &oldtermio) < 0)
 		goto err1;
 
-	ts = lxc_console_sigwinch_init(console->peerpty.master, console->master);
+	ts = lxc_console_signal_init(console->peerpty.master, console->master);
 	if (!ts)
 		goto err1;
 
@@ -479,10 +517,10 @@ static int lxc_console_peer_default(struct lxc_console *console)
 		goto on_error1;
 	}
 
-	ts = lxc_console_sigwinch_init(console->peer, console->master);
+	ts = lxc_console_signal_init(console->peer, console->master);
 	console->tty_state = ts;
 	if (!ts) {
-		WARN("unable to install SIGWINCH handler");
+		WARN("Failed to install signal handler");
 		goto on_error1;
 	}
 
@@ -785,7 +823,7 @@ int lxc_console(struct lxc_container *c, int ttynum,
 	if (ret < 0)
 		TRACE("Process is already group leader");
 
-	ts = lxc_console_sigwinch_init(stdinfd, masterfd);
+	ts = lxc_console_signal_init(stdinfd, masterfd);
 	if (!ts) {
 		ret = -1;
 		goto close_fds;
@@ -811,9 +849,9 @@ int lxc_console(struct lxc_container *c, int ttynum,
 
 	if (ts->sigfd != -1) {
 		ret = lxc_mainloop_add_handler(&descr, ts->sigfd,
-					       lxc_console_cb_sigwinch_fd, ts);
+					       lxc_console_cb_signal_fd, ts);
 		if (ret < 0) {
-			ERROR("Failed to add SIGWINCH handler");
+			ERROR("Failed to add signal handler to mainloop");
 			goto close_mainloop;
 		}
 	}
@@ -867,7 +905,7 @@ close_mainloop:
 	lxc_mainloop_close(&descr);
 
 sigwinch_fini:
-	lxc_console_sigwinch_fini(ts);
+	lxc_console_signal_fini(ts);
 
 close_fds:
 	close(masterfd);

src/lxc/console.h

@@ -24,6 +24,9 @@
 #ifndef __LXC_CONSOLE_H
 #define __LXC_CONSOLE_H
 
+#include <signal.h>
+#include <stdio.h>
+
 #include "conf.h"
 #include "list.h"
 
@@ -38,18 +41,21 @@ struct lxc_tty_state
 	/* Escape sequence to use for exiting the pty. A single char can be
 	 * specified. The pty can then exited by doing: Ctrl + specified_char + q.
 	 * This field is checked by lxc_console_cb_tty_stdin(). Set to -1 to
-	 * disable exiting the pty via a escape sequence. */
+	 * disable exiting the pty via a escape sequence.
+	 */
 	int escape;
 	/* Used internally by lxc_console_cb_tty_stdin() to check whether an
-	 * escape sequence has been received. */
+	 * escape sequence has been received.
+	 */
 	int saw_escape;
 	/* Name of the container to forward the SIGWINCH event to. */
 	const char *winch_proxy;
 	/* Path of the container to forward the SIGWINCH event to. */
 	const char *winch_proxy_lxcpath;
-	/* File descriptor that accepts SIGWINCH signals. If set to -1 no
-	 * SIGWINCH handler could be installed. This also means that
-	 * the sigset_t oldmask member is meaningless. */
+	/* File descriptor that accepts signals. If set to -1 no signal handler
+	 * could be installed. This also means that the sigset_t oldmask member
+	 * is meaningless.
+	 */
 	int sigfd;
 	sigset_t oldmask;
 };
@@ -172,48 +178,48 @@ extern int lxc_setup_tios(int fd, struct termios *oldtios);
 extern void lxc_console_winsz(int srcfd, int dstfd);
 
 /*
- * lxc_console_sigwinch_init: install SIGWINCH handler
+ * lxc_console_signal_init: install signal handler
  *
  * @srcfd  : src for winsz in SIGWINCH handler
  * @dstfd  : dst for winsz in SIGWINCH handler
  *
  * Returns lxc_tty_state structure on success or NULL on failure. The sigfd
  * member of the returned lxc_tty_state can be select()/poll()ed/epoll()ed
- * on (ie added to a mainloop) for SIGWINCH.
+ * on (ie added to a mainloop) for signals.
  *
  * Must be called with process_lock held to protect the lxc_ttys list, or
  * from a non-threaded context.
  *
- * Note that SIGWINCH isn't installed as a classic asychronous handler,
- * rather signalfd(2) is used so that we can handle the signal when we're
- * ready for it. This avoids deadlocks since a signal handler
- * (ie lxc_console_sigwinch()) would need to take the thread mutex to
- * prevent lxc_ttys list corruption, but using the fd we can provide the
- * tty_state needed to the callback (lxc_console_cb_sigwinch_fd()).
+ * Note that the signal handler isn't installed as a classic asychronous
+ * handler, rather signalfd(2) is used so that we can handle the signal when
+ * we're ready for it. This avoids deadlocks since a signal handler (ie
+ * lxc_console_sigwinch()) would need to take the thread mutex to prevent
+ * lxc_ttys list corruption, but using the fd we can provide the tty_state
+ * needed to the callback (lxc_console_cb_signal_fd()).
  *
  * This function allocates memory. It is up to the caller to free it.
  */
-extern struct lxc_tty_state *lxc_console_sigwinch_init(int srcfd, int dstfd);
+extern struct lxc_tty_state *lxc_console_signal_init(int srcfd, int dstfd);
 
 /*
- * Handler for SIGWINCH events. To be registered via the corresponding functions
+ * Handler for signal events. To be registered via the corresponding functions
  * declared and defined in mainloop.{c,h} or lxc_console_mainloop_add().
  */
-extern int lxc_console_cb_sigwinch_fd(int fd, uint32_t events, void *cbdata,
-		struct lxc_epoll_descr *descr);
+extern int lxc_console_cb_signal_fd(int fd, uint32_t events, void *cbdata,
+				    struct lxc_epoll_descr *descr);
 
 /*
- * lxc_console_sigwinch_fini: uninstall SIGWINCH handler
+ * lxc_console_signal_fini: uninstall signal handler
  *
- * @ts  : the lxc_tty_state returned by lxc_console_sigwinch_init
+ * @ts  : the lxc_tty_state returned by lxc_console_signal_init
  *
  * Restore the saved signal handler that was in effect at the time
- * lxc_console_sigwinch_init() was called.
+ * lxc_console_signal_init() was called.
  *
  * Must be called with process_lock held to protect the lxc_ttys list, or
  * from a non-threaded context.
  */
-extern void lxc_console_sigwinch_fini(struct lxc_tty_state *ts);
+extern void lxc_console_signal_fini(struct lxc_tty_state *ts);
 
 extern int lxc_console_write_ringbuffer(struct lxc_console *console);
 

src/lxc/tools/lxc_attach.c

@@ -371,7 +371,7 @@ err3:
 	lxc_mainloop_close(&descr);
 err2:
 	if (ts && ts->sigfd != -1)
-		lxc_console_sigwinch_fini(ts);
+		lxc_console_signal_fini(ts);
 err1:
 	lxc_console_delete(&conf->console);