Re: Deadlock when receiving kill-signal from child process

[Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>]

On Aug 9,  4:53pm, Mathias Fredriksson wrote:
}
} At first I thought there were absolutely no more deadlocks, but I
} managed to repeatedly produce one (when _not_ disowning the child
} processes).

Reviewing the code again has led me to realize / recall that although
we consistently use our signal-safe wrappers around malloc(), the
similar wrapper around free() is enabled only when the configuration
has included --enable-zsh-mem.  Further, we're inconsistent about using
the signal-safe wrapper for realloc() [a better one gets compiled in
with --enable-zsh-mem].  The last few stack traces you've sent [except
for the most recent one mentioning fork()] indicate clashes between
realloc() and free(), even though other parts of the stack look wonky.

Even with --enable-zsh-mem the realloc() wrapper sometimes calls
malloc() unsafely, though that wouldn't be hard to fix.

A quick grep indicates 34 realloc() and 224 free() scattered around
the code.  Probably many of them are already within queue_signals().
The patch below fixes four of them that your stack traces seem to
point to as particular culprits.  It's probably not worth it to try
to pre-emptively change all the others.

If you are still able to create deadlocks after the below, I'd also
ask you to try configuring with --enable-zsh-mem and see if that gets
rid of any remaining deadlocks.

It'd also be interesting to benchmark a few versions of zsh including
the latest from git, on things like how long **/* takes in a big tree,
how long "make check" takes to run, etc., both with and without using
--enable-zsh-mem.  Any volunteers?


diff --git a/Src/glob.c b/Src/glob.c
index f82c3bd..3af4690 100644
--- a/Src/glob.c
+++ b/Src/glob.c
@@ -257,7 +257,7 @@ addpath(char *s, int l)
 {
     DPUTS(!pathbuf, "BUG: pathbuf not initialised");
     while (pathpos + l + 1 >= pathbufsz)
-	pathbuf = realloc(pathbuf, pathbufsz *= 2);
+	pathbuf = zrealloc(pathbuf, pathbufsz *= 2);
     while (l--)
 	pathbuf[pathpos++] = *s++;
     pathbuf[pathpos++] = '/';
diff --git a/Src/text.c b/Src/text.c
index cf73004..3978a26 100644
--- a/Src/text.c
+++ b/Src/text.c
@@ -77,7 +77,7 @@ taddpending(char *str1, char *str2)
      */
     if (tpending) {
 	int oldlen = strlen(tpending);
-	tpending = realloc(tpending, len + oldlen);
+	tpending = zrealloc(tpending, len + oldlen);
 	sprintf(tpending + oldlen, "%s%s", str1, str2);
     } else {
 	tpending = (char *)zalloc(len);
@@ -110,7 +110,7 @@ taddchr(int c)
 	    tptr--;
 	    return;
 	}
-	tbuf = realloc(tbuf, tsiz *= 2);
+	tbuf = zrealloc(tbuf, tsiz *= 2);
 	tlim = tbuf + tsiz;
 	tptr = tbuf + tsiz / 2;
     }
@@ -130,7 +130,7 @@ taddstr(char *s)
 
 	if (!tbuf)
 	    return;
-	tbuf = realloc(tbuf, tsiz *= 2);
+	tbuf = zrealloc(tbuf, tsiz *= 2);
 	tlim = tbuf + tsiz;
 	tptr = tbuf + x;
     }

-- 
Barton E. Schaefer