Skip site navigation (1)Skip section navigation (2) 
Date:      Sun, 22 Nov 2009 18:23:31 +0000 (UTC)
From:      Jilles Tjoelker <jilles@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r199660 - head/bin/sh
Message-ID:  <200911221823.nAMINVL5025958@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: jilles
Date: Sun Nov 22 18:23:30 2009
New Revision: 199660
URL: http://svn.freebsd.org/changeset/base/199660

Log:
  Fix various things about SIGINT handling:
  * exception handlers are now run with interrupts disabled, which avoids
    many race conditions
  * fix some cases where SIGINT only aborts one command and continues the
    script, in particular if a SIGINT causes an EINTR error which trumped the
    interrupt.
  
  Example:
    sh -c 'echo < /some/fifo; echo This should not be printed'
  The fifo should not have writers. When pressing ctrl+c to abort the open,
  the shell used to continue with the next command.
  
  Example:
    sh -c '/bin/echo < /some/fifo; echo This should not be printed'
  Similar. Note, however, that this particular case did not and does not work
  in interactive mode with job control enabled.

Modified:
  head/bin/sh/error.c
  head/bin/sh/error.h
  head/bin/sh/eval.c
  head/bin/sh/parser.c
  head/bin/sh/redir.c
  head/bin/sh/var.c

Modified: head/bin/sh/error.c
==============================================================================
--- head/bin/sh/error.c	Sun Nov 22 17:25:11 2009	(r199659)
+++ head/bin/sh/error.c	Sun Nov 22 18:23:30 2009	(r199660)
@@ -73,11 +73,15 @@ static void exverror(int, const char *, 
  * Called to raise an exception.  Since C doesn't include exceptions, we
  * just do a longjmp to the exception handler.  The type of exception is
  * stored in the global variable "exception".
+ *
+ * Interrupts are disabled; they should be reenabled when the exception is
+ * caught.
  */
 
 void
 exraise(int e)
 {
+	INTOFF;
 	if (handler == NULL)
 		abort();
 	exception = e;
@@ -138,8 +142,15 @@ onint(void)
 static void
 exverror(int cond, const char *msg, va_list ap)
 {
-	CLEAR_PENDING_INT;
-	INTOFF;
+	/*
+	 * An interrupt trumps an error.  Certain places catch error
+	 * exceptions or transform them to a plain nonzero exit code
+	 * in child processes, and if an error exception can be handled,
+	 * an interrupt can be handled as well.
+	 *
+	 * exraise() will disable interrupts for the exception handler.
+	 */
+	FORCEINTON;
 
 #ifdef DEBUG
 	if (msg)

Modified: head/bin/sh/error.h
==============================================================================
--- head/bin/sh/error.h	Sun Nov 22 17:25:11 2009	(r199659)
+++ head/bin/sh/error.h	Sun Nov 22 18:23:30 2009	(r199660)
@@ -72,6 +72,8 @@ extern volatile sig_atomic_t intpending;
 
 #define INTOFF suppressint++
 #define INTON { if (--suppressint == 0 && intpending) onint(); }
+#define is_int_on() suppressint
+#define SETINTON(s) suppressint = (s)
 #define FORCEINTON {suppressint = 0; if (intpending) onint();}
 #define CLEAR_PENDING_INT intpending = 0
 #define int_pending() intpending

Modified: head/bin/sh/eval.c
==============================================================================
--- head/bin/sh/eval.c	Sun Nov 22 17:25:11 2009	(r199659)
+++ head/bin/sh/eval.c	Sun Nov 22 18:23:30 2009	(r199660)
@@ -782,7 +782,6 @@ evalcommand(union node *cmd, int flags, 
 		savelocalvars = localvars;
 		localvars = NULL;
 		reffunc(cmdentry.u.func);
-		INTON;
 		savehandler = handler;
 		if (setjmp(jmploc.loc)) {
 			if (exception == EXSHELLPROC)
@@ -798,6 +797,7 @@ evalcommand(union node *cmd, int flags, 
 			longjmp(handler->loc, 1);
 		}
 		handler = &jmploc;
+		INTON;
 		for (sp = varlist.list ; sp ; sp = sp->next)
 			mklocal(sp->text);
 		funcnest++;

Modified: head/bin/sh/parser.c
==============================================================================
--- head/bin/sh/parser.c	Sun Nov 22 17:25:11 2009	(r199659)
+++ head/bin/sh/parser.c	Sun Nov 22 18:23:30 2009	(r199660)
@@ -1312,6 +1312,7 @@ parsebackq: {
 	int saveprompt;
 	const int bq_startlinno = plinno;
 
+	str = NULL;
 	if (setjmp(jmploc.loc)) {
 		if (str)
 			ckfree(str);
@@ -1323,7 +1324,6 @@ parsebackq: {
 		longjmp(handler->loc, 1);
 	}
 	INTOFF;
-	str = NULL;
 	savelen = out - stackblock();
 	if (savelen > 0) {
 		str = ckmalloc(savelen);

Modified: head/bin/sh/redir.c
==============================================================================
--- head/bin/sh/redir.c	Sun Nov 22 17:25:11 2009	(r199659)
+++ head/bin/sh/redir.c	Sun Nov 22 18:23:30 2009	(r199660)
@@ -166,8 +166,11 @@ openredirect(union node *redir, char mem
 
 	/*
 	 * We suppress interrupts so that we won't leave open file
-	 * descriptors around.  This may not be such a good idea because
-	 * an open of a device or a fifo can block indefinitely.
+	 * descriptors around.  Because the signal handler remains
+	 * installed and we do not use system call restart, interrupts
+	 * will still abort blocking opens such as fifos (they will fail
+	 * with EINTR). There is, however, a race condition if an interrupt
+	 * arrives after INTOFF and before open blocks.
 	 */
 	INTOFF;
 	memory[fd] = 0;

Modified: head/bin/sh/var.c
==============================================================================
--- head/bin/sh/var.c	Sun Nov 22 17:25:11 2009	(r199659)
+++ head/bin/sh/var.c	Sun Nov 22 18:23:30 2009	(r199660)
@@ -195,7 +195,9 @@ setvarsafe(char *name, char *val, int fl
 	struct jmploc jmploc;
 	struct jmploc *const savehandler = handler;
 	int err = 0;
+	int inton;
 
+	inton = is_int_on();
 	if (setjmp(jmploc.loc))
 		err = 1;
 	else {
@@ -203,6 +205,7 @@ setvarsafe(char *name, char *val, int fl
 		setvar(name, val, flags);
 	}
 	handler = savehandler;
+	SETINTON(inton);
 	return err;
 }

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200911221823.nAMINVL5025958>