Re: deadlock caused by gettext usage in a signal handler

[Peter Stephenson <p.w.stephenson@ntlworld.com>]

On Fri, 30 Nov 2007 20:35:34 +0100
Guillaume Chazarain <guichaz@yahoo.fr> wrote:
> I just had a Zsh process (using zsh-4.2.6-6.fc7) deadlock, the
> backtrace seems to show it is initializing the gettext infrastructure
> to print "Input/output error" in a signal handler.
> 
> As a solution, avoiding gettext() in signal handlers seems harder than
> forcing gettext() initialization before its first usage, but the cost
> may be prohibitive.

I'm not sure how we should handle this.  zsh doesn't ever use gettext()
directly and the interface it is using (strerror()) doesn't document
what needs to be initialised and when (nor even that it calls
gettext()).  I suppose a call to strerror() right at the top of the main
function might do the trick, but there's no guarantee and that seems
rather a hack.  Alternatively, we could use strerror_r() with, say, 80
characters off the stack, but there's no guarantee there aren't other
calls with the same problem.  So I can see possible things to do but
nothing that looks reliable.

I was going to say using strerror_r() was at least hack free, but it
turns out it isn't... there are two incompatible versions whose
prototypes differ only in return value, so it's difficult to test for
them.  So I've tried to handle both.  Yuk.  Suggestions welcome.

To get diff output for a test failure while I was writing this I needed
to use "diff -a" in ztst.zsh.  I'm not sure if the option is universal,
however.

Index: configure.ac
===================================================================
RCS file: /cvsroot/zsh/zsh/configure.ac,v
retrieving revision 1.82
diff -u -r1.82 configure.ac
--- configure.ac	24 Nov 2007 01:56:49 -0000	1.82
+++ configure.ac	3 Dec 2007 22:29:29 -0000
@@ -1157,7 +1157,7 @@
 	       getlogin getpwent getpwnam getpwuid getgrgid getgrnam \
 	       initgroups nis_list \
 	       setuid seteuid setreuid setresuid setsid \
-	       memcpy memmove strstr strerror \
+	       memcpy memmove strstr strerror strerror_r \
 	       getrlimit getrusage \
 	       setlocale \
 	       uname \
Index: Src/utils.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/utils.c,v
retrieving revision 1.171
diff -u -r1.171 utils.c
--- Src/utils.c	20 Nov 2007 09:55:10 -0000	1.171
+++ Src/utils.c	3 Dec 2007 22:29:31 -0000
@@ -255,6 +255,12 @@
 {
     const char *str;
     int num;
+#ifdef HAVE_STRERROR_R
+#define ERRBUFSIZE (80)
+    int olderrno;
+    char errbuf[ERRBUFSIZE];
+#endif
+    char *errmsg;
 
     if ((unset(SHINSTDIN) || locallevel) && lineno)
 	fprintf(file, "%ld: ", (long)lineno);
@@ -304,12 +310,39 @@
 		    errflag = 1;
 		    return;
 		}
+#ifdef HAVE_STRERROR_R
+		/*
+		 * There are two incompatible strerror_r()s floating round.
+		 * The GNU extension refuses to copy the message into the
+		 * buffer if it can return a constant string.  To suppress it
+		 * we need to define _XOPEN_SOURCE to 600.  I don't dare do
+		 * this because we're already depending on _GNU_SOURCE.  So
+		 * try to handle both by looking for errno being set (for the
+		 * standard version failing) or errbuf being left untouched
+		 * (for the GNU version).  One presumes that if strerror_r()
+		 * didn't copy anything to errbuf, then it's safe to
+		 * call strerror() to get the string.
+		 *
+		 * This is a mess, but it's about a decade and half
+		 * too late to shirk from messes in the source.
+		 */
+		olderrno = errno;
+		errno = 0;
+		errbuf[0] = '\0';
+		strerror_r(num, errbuf, ERRBUFSIZE);
+		if (errno || errbuf[0] == '\0')
+		    errmsg = strerror(num);
+		else
+		    errmsg = errbuf;
+		errno = olderrno;
+#else
+		errmsg = strerror(num);
+#endif
 		/* If the message is not about I/O problems, it looks better *
 		 * if we uncapitalize the first letter of the message        */
 		if (num == EIO)
-		    fputs(strerror(num), file);
+		    fputs(errmsg, file);
 		else {
-		    char *errmsg = strerror(num);
 		    fputc(tulower(errmsg[0]), file);
 		    fputs(errmsg + 1, file);
 		}


-- 
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/