Re: Deadlock when receiving kill-signal from child process

[Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>]

On Aug 9,  4:42pm, Bart Schaefer wrote:
}
} Nevertheless, it looks like glob.c needs to protect some places where
} it manipulates global state.

I couldn't really find anything definitive -- all the memory management
routines (halloc, zhalloc, zfree, etc.) already do signal queuing, so
we only get into trouble with e.g. stdio using the library malloc/free
directly.

The hunks in patcompile() below are paranoia because the static globals
patcode, patsize, et al. are being frobbed -- but even with this I get
worried that a signal trap that uses pattern matching called in the
midst of some other pattern matching operation could leave those in an
inconsistent state, because they're not saved/restored like globbing
state in zglob().

Similarly if you were in the middle of a pattern that wanted to set the
MATCH / MBEGIN / MEND / match / mbegin / mend parameters and then a
trap handler also did so, it sure looks to me as if the state of those
parameters becomes indeterminate.

Patch below follows on to 36022 but doesn't overlap with it in any way.


diff --git a/Src/glob.c b/Src/glob.c
index eff34a2..f82c3bd 100644
--- a/Src/glob.c
+++ b/Src/glob.c
@@ -216,22 +216,26 @@ static struct globdata curglobdata;
 
 #define save_globstate(N) \
   do { \
+    queue_signals(); \
     memcpy(&(N), &curglobdata, sizeof(struct globdata)); \
     (N).gd_pathpos = pathpos; \
     (N).gd_pathbuf = pathbuf; \
     (N).gd_glob_pre = glob_pre; \
     (N).gd_glob_suf = glob_suf; \
     pathbuf = NULL; \
+    unqueue_signals(); \
   } while (0)
 
 #define restore_globstate(N) \
   do { \
+    queue_signals(); \
     zfree(pathbuf, pathbufsz); \
     memcpy(&curglobdata, &(N), sizeof(struct globdata)); \
     pathpos = (N).gd_pathpos; \
     pathbuf = (N).gd_pathbuf; \
     glob_pre = (N).gd_glob_pre; \
     glob_suf = (N).gd_glob_suf; \
+    unqueue_signals(); \
   } while (0)
 
 /* pathname component in filename patterns */
diff --git a/Src/pattern.c b/Src/pattern.c
index 8fa1a72..7d38988 100644
--- a/Src/pattern.c
+++ b/Src/pattern.c
@@ -520,6 +520,8 @@ patcompile(char *exp, int inflags, char **endexp)
     char *lng, *strp = NULL;
     Patprog p;
 
+    queue_signals();
+
     startoff = sizeof(struct patprog);
     /* Ensure alignment of start of program string */
     startoff = (startoff + sizeof(union upat) - 1) & ~(sizeof(union upat) - 1);
@@ -582,8 +584,10 @@ patcompile(char *exp, int inflags, char **endexp)
 	if (!strp || (*strp && *strp != '/')) {
 	    /* No, do normal compilation. */
 	    strp = NULL;
-	    if (patcompswitch(0, &flags) == 0)
+	    if (patcompswitch(0, &flags) == 0) {
+		unqueue_signals();
 		return NULL;
+	    }
 	} else {
 	    /*
 	     * Yes, copy the string, and skip compilation altogether.
@@ -715,6 +719,8 @@ patcompile(char *exp, int inflags, char **endexp)
 
     if (endexp)
 	*endexp = patparse;
+
+    unqueue_signals();
     return p;
 }
 

-- 
Barton E. Schaefer