usr.bin/make 2nd try: fix signal stuff
Max Okumoto
okumoto at ucsd.edu
Wed Dec 1 05:50:19 PST 2004
Another patch missed from mega patch.
Max
Fix a (very) long standing bug in make (this has been there probably
from the beginning). Make used to handle all its interrupt-time stuff
directly from the signal handler, including calls to printf, accessing
global data and so on. This is of course wrong and could provoke a core
dump when interrupting make. Just set a flag in the signal handler and
do everything else from the main thread.
PatchSet 370
Date: 2004/11/12 07:57:17
Author: harti
Log:
Fix a (very) long standing bug in make (this has been there probably
from the beginning). Make used to handle all its interrupt-time stuff
directly from the signal handler, including calls to printf, accessing
global data and so on. This is of course wrong and could provoke a core
dump when interrupting make. Just set a flag in the signal handler and
do everything else from the main thread.
PR: bin/29103
Members:
compat.c:1.38->1.39
job.c:1.55->1.56
Index: compat.c
===================================================================
RCS file: /usr/home/okumoto/Work/make/fbsd-cvs/src/usr.bin/make/compat.c,v
retrieving revision 1.38
retrieving revision 1.39
diff -u -r1.38 -r1.39
--- compat.c 23 Oct 2004 21:34:41 -0000 1.38
+++ compat.c 12 Nov 2004 07:57:17 -0000 1.39
@@ -79,6 +79,8 @@
static GNode *curTarg = NULL;
static GNode *ENDNode;
+static sig_atomic_t interrupted;
+
static void CompatInterrupt(int);
static int CompatMake(void *, void *);
static int shellneed(char *);
@@ -101,6 +103,16 @@
meta[0] = 1;
}
+/*
+ * Interrupt handler - set flag and defer handling to the main code
+ */
+static void
+CompatCatchSig(int signo)
+{
+
+ interrupted = signo;
+}
+
/*-
*-----------------------------------------------------------------------
* CompatInterrupt --
@@ -120,6 +132,17 @@
CompatInterrupt (int signo)
{
GNode *gn;
+ sigset_t nmask, omask;
+
+ sigemptyset(&nmask);
+ sigaddset(&nmask, SIGINT);
+ sigaddset(&nmask, SIGTERM);
+ sigaddset(&nmask, SIGHUP);
+ sigaddset(&nmask, SIGQUIT);
+ sigprocmask(SIG_SETMASK, &nmask, &omask);
+
+ /* prevent recursion in evaluation of .INTERRUPT */
+ interrupted = 0;
if ((curTarg != NULL) && !Targ_Precious (curTarg)) {
char *p1;
@@ -129,18 +152,20 @@
printf ("*** %s removed\n", file);
}
free(p1);
+ }
- /*
- * Run .INTERRUPT only if hit with interrupt signal
- */
- if (signo == SIGINT) {
- gn = Targ_FindNode(".INTERRUPT", TARG_NOCREATE);
- if (gn != NULL) {
- Lst_ForEach(gn->commands, Compat_RunCommand, (void *)gn);
- }
+ /*
+ * Run .INTERRUPT only if hit with interrupt signal
+ */
+ if (signo == SIGINT) {
+ gn = Targ_FindNode(".INTERRUPT", TARG_NOCREATE);
+ if (gn != NULL) {
+ Lst_ForEach(gn->commands, Compat_RunCommand, (void *)gn);
}
-
}
+
+ sigprocmask(SIG_SETMASK, &omask, NULL);
+
if (signo == SIGQUIT)
exit(signo);
(void) signal(signo, SIG_DFL);
@@ -373,10 +398,12 @@
while (1) {
while ((rstat = wait(&reason)) != cpid) {
- if (rstat == -1 && errno != EINTR) {
- break;
+ if (interrupted || (rstat == -1 && errno != EINTR)) {
+ break;
}
}
+ if (interrupted)
+ CompatInterrupt(interrupted);
if (rstat > -1) {
if (WIFSTOPPED(reason)) {
@@ -660,16 +687,16 @@
Shell_Init(); /* Set up shell. */
if (signal(SIGINT, SIG_IGN) != SIG_IGN) {
- signal(SIGINT, CompatInterrupt);
+ signal(SIGINT, CompatCatchSig);
}
if (signal(SIGTERM, SIG_IGN) != SIG_IGN) {
- signal(SIGTERM, CompatInterrupt);
+ signal(SIGTERM, CompatCatchSig);
}
if (signal(SIGHUP, SIG_IGN) != SIG_IGN) {
- signal(SIGHUP, CompatInterrupt);
+ signal(SIGHUP, CompatCatchSig);
}
if (signal(SIGQUIT, SIG_IGN) != SIG_IGN) {
- signal(SIGQUIT, CompatInterrupt);
+ signal(SIGQUIT, CompatCatchSig);
}
ENDNode = Targ_FindNode(".END", TARG_CREATE);
Index: job.c
===================================================================
RCS file: /usr/home/okumoto/Work/make/fbsd-cvs/src/usr.bin/make/job.c,v
retrieving revision 1.55
retrieving revision 1.56
diff -u -r1.55 -r1.56
--- job.c 11 Nov 2004 12:52:15 -0000 1.55
+++ job.c 12 Nov 2004 07:57:17 -0000 1.56
@@ -259,6 +259,9 @@
* limits or externally */
+static sig_atomic_t interrupted;
+
+
#if defined(USE_PGRP) && defined(SYSV)
# define KILL(pid, sig) killpg(-(pid), (sig))
#else
@@ -306,6 +309,19 @@
static void JobInterrupt(int, int);
static void JobRestartJobs(void);
+/*
+ * JobCatchSignal
+ *
+ * Got a signal. Set global variables and hope that someone will
+ * handle it.
+ */
+static void
+JobCatchSig(int signo)
+{
+
+ interrupted = signo;
+}
+
/*-
*-----------------------------------------------------------------------
* JobCondPassSig --
@@ -351,6 +367,10 @@
sigset_t nmask, omask;
struct sigaction act;
+ sigemptyset(&nmask);
+ sigaddset(&nmask, signo);
+ sigprocmask(SIG_SETMASK, &nmask, &omask);
+
DEBUGF(JOB, ("JobPassSig(%d) called.\n", signo));
Lst_ForEach(jobs, JobCondPassSig, (void *) &signo);
@@ -377,10 +397,8 @@
* Note we block everything else possible while we're getting the signal.
* This ensures that all our jobs get continued when we wake up before
* we take any other signal.
+ * XXX this comment seems wrong.
*/
- sigemptyset(&nmask);
- sigaddset(&nmask, signo);
- sigprocmask(SIG_SETMASK, &nmask, &omask);
act.sa_handler = SIG_DFL;
sigemptyset(&act.sa_mask);
act.sa_flags = 0;
@@ -1366,6 +1384,10 @@
Boolean noExec; /* Set true if we decide not to run the job */
int tfd; /* File descriptor for temp file */
+ if (interrupted) {
+ JobPassSig(interrupted);
+ return (JOB_ERROR);
+ }
if (previous != NULL) {
previous->flags &= ~(JOB_FIRST|JOB_IGNERR|JOB_SILENT);
job = previous;
@@ -1709,6 +1731,14 @@
nRead = read(job->inPipe, &job->outBuf[job->curPos],
JOB_BUFSIZE - job->curPos);
+ /*
+ * Check for interrupt here too, because the above read may block
+ * when the child process is stopped. In this case the interrupt
+ * will unblock it (we don't use SA_RESTART).
+ */
+ if (interrupted)
+ JobPassSig(interrupted);
+
if (nRead < 0) {
DEBUGF(JOB, ("JobDoOutput(piperead)"));
nr = 0;
@@ -1921,6 +1951,8 @@
JobFinish(job, &status);
}
+ if (interrupted)
+ JobPassSig(interrupted);
}
/*-
@@ -1961,6 +1993,8 @@
if ((nfds = kevent(kqfd, NULL, 0, kev, KEV_SIZE, NULL)) == -1) {
if (errno != EINTR)
Punt("kevent: %s", strerror(errno));
+ if (interrupted)
+ JobPassSig(interrupted);
} else {
for (i = 0; i < nfds; i++) {
if (kev[i].flags & EV_ERROR) {
@@ -1984,9 +2018,11 @@
timeout.tv_usec = SEL_USEC;
if ((nfds = select(FD_SETSIZE, &readfds, (fd_set *) 0,
- (fd_set *) 0, &timeout)) <= 0)
+ (fd_set *) 0, &timeout)) <= 0) {
+ if (interrupted)
+ JobPassSig(interrupted);
return;
- else {
+ } else {
if (Lst_Open(jobs) == FAILURE) {
Punt("Cannot open job table");
}
@@ -2055,6 +2091,7 @@
Job_Init(int maxproc)
{
GNode *begin; /* node for commands to do at the very start */
+ struct sigaction sa;
jobs = Lst_Init(FALSE);
stoppedJobs = Lst_Init(FALSE);
@@ -2087,19 +2124,24 @@
/*
* Catch the four signals that POSIX specifies if they aren't ignored.
- * JobPassSig will take care of calling JobInterrupt if appropriate.
+ * JobCatchSignal will just set global variables and hope someone
+ * else is going to handle the interrupt.
*/
+ sa.sa_handler = JobCatchSig;
+ sigemptyset(&sa.sa_mask);
+ sa.sa_flags = 0;
+
if (signal(SIGINT, SIG_IGN) != SIG_IGN) {
- (void) signal(SIGINT, JobPassSig);
+ (void) sigaction(SIGINT, &sa, NULL);
}
if (signal(SIGHUP, SIG_IGN) != SIG_IGN) {
- (void) signal(SIGHUP, JobPassSig);
+ (void) sigaction(SIGHUP, &sa, NULL);
}
if (signal(SIGQUIT, SIG_IGN) != SIG_IGN) {
- (void) signal(SIGQUIT, JobPassSig);
+ (void) sigaction(SIGQUIT, &sa, NULL);
}
if (signal(SIGTERM, SIG_IGN) != SIG_IGN) {
- (void) signal(SIGTERM, JobPassSig);
+ (void) sigaction(SIGTERM, &sa, NULL);
}
/*
* There are additional signals that need to be caught and passed if
@@ -2109,16 +2151,16 @@
*/
#if defined(USE_PGRP)
if (signal(SIGTSTP, SIG_IGN) != SIG_IGN) {
- (void) signal(SIGTSTP, JobPassSig);
+ (void) sigaction(SIGTSTP, &sa, NULL);
}
if (signal(SIGTTOU, SIG_IGN) != SIG_IGN) {
- (void) signal(SIGTTOU, JobPassSig);
+ (void) sigaction(SIGTTOU, &sa, NULL);
}
if (signal(SIGTTIN, SIG_IGN) != SIG_IGN) {
- (void) signal(SIGTTIN, JobPassSig);
+ (void) sigaction(SIGTTIN, &sa, NULL);
}
if (signal(SIGWINCH, SIG_IGN) != SIG_IGN) {
- (void) signal(SIGWINCH, JobPassSig);
+ (void) sigaction(SIGWINCH, &sa, NULL);
}
#endif
@@ -2447,6 +2489,10 @@
}
if (runINTERRUPT && !touchFlag) {
+ /* clear the interrupted flag because we would get an
+ * infinite loop otherwise */
+ interrupted = 0;
+
interrupt = Targ_FindNode(".INTERRUPT", TARG_NOCREATE);
if (interrupt != NULL) {
ignoreErrors = FALSE;
More information about the Submit
mailing list