threads/90392: libc stdio memory leak with pthread

Daniel Hartmeier daniel at benzedrine.cx
Wed Dec 14 12:49:28 PST 2005


It's actually a waste to lock the mutex and destroy it again, since it's
really private to sscanf() and only provided as part of the API and not
because it serves a real locking purpose in this case.

The patch below splits fread() into a MT-safe and Non-MT-safe version,
so __svfscanf() can call the latter. This is in the spirit of how many
other stdio functions were split, for instance see the diff and commit
message of rev. 1.8 of ungetc.c

  http://www.freebsd.org/cgi/cvsweb.cgi/src/lib/libc/stdio/ungetc.c

With this, sscanf() never triggers a mutex allocation or lock, and
there's no memory leak due to lacking pthread_mutex_destroy().

I checked all the other uses of INITEXTRA(), and none of them share this
problem, since in those cases only the non-locking functions (with
prefix __) are called. There might some risk of someone later
introducing calls to locking functions, but that fread() call in
__svfscanf() was really the only offender I can spot. You could add the
pthread_mutex_destroy() as suggested in the first patch additionally, to
cover future mistakes.

Daniel

--- local.h.orig	Wed Dec 14 20:29:56 2005
+++ local.h	Wed Dec 14 20:31:44 2005
@@ -79,6 +79,8 @@
 extern int	__vfwprintf(FILE *, const wchar_t *, __va_list);
 extern int	__vfwscanf(FILE * __restrict, const wchar_t * __restrict,
 		    __va_list);
+extern size_t	__fread(void * __restrict buf, size_t size, size_t count,
+		    FILE * __restrict fp);
 
 extern int	__sdidinit;
 
--- fread.c.orig	Wed Dec 14 20:22:31 2005
+++ fread.c	Wed Dec 14 20:29:11 2005
@@ -47,11 +47,25 @@
 #include "local.h"
 #include "libc_private.h"
 
+/*
+ * MT-safe version
+ */
 size_t
-fread(buf, size, count, fp)
-	void * __restrict buf;
-	size_t size, count;
-	FILE * __restrict fp;
+fread(void * __restrict buf, size_t size, size_t count, FILE * __restrict fp)
+{
+	int ret;
+
+	FLOCKFILE(fp);
+	ret = __fread(buf, size, count, fp);
+	FUNLOCKFILE(fp);
+	return (ret);
+}
+
+/*
+ * Non-MT-safe version
+ */
+size_t
+__fread(void * __restrict buf, size_t size, size_t count, FILE * __restrict fp)
 {
 	size_t resid;
 	char *p;
@@ -65,7 +79,6 @@
 	 */
 	if ((resid = count * size) == 0)
 		return (0);
-	FLOCKFILE(fp);
 	ORIENT(fp, -1);
 	if (fp->_r < 0)
 		fp->_r = 0;
@@ -79,13 +92,11 @@
 		resid -= r;
 		if (__srefill(fp)) {
 			/* no more input: return partial result */
-			FUNLOCKFILE(fp);
 			return ((total - resid) / size);
 		}
 	}
 	(void)memcpy((void *)p, (void *)fp->_p, resid);
 	fp->_r -= resid;
 	fp->_p += resid;
-	FUNLOCKFILE(fp);
 	return (count);
 }
--- vfscanf.c.orig	Wed Dec 14 20:22:53 2005
+++ vfscanf.c	Wed Dec 14 20:32:08 2005
@@ -412,7 +412,7 @@
 				}
 				nread += sum;
 			} else {
-				size_t r = fread((void *)va_arg(ap, char *), 1,
+				size_t r = __fread((void *)va_arg(ap, char *), 1,
 				    width, fp);
 
 				if (r == 0)


More information about the freebsd-threads mailing list