threads/141198: src/lib/libc/stdio does not properly initialize mutexes

[John Baldwin]

On Saturday 05 December 2009 3:34:41 pm Jeremy Huddleston wrote:
> 
> >Number:         141198
> >Category:       threads
> >Synopsis:       src/lib/libc/stdio does not properly initialize mutexes
> >Confidential:   no
> >Severity:       non-critical
> >Priority:       low
> >Responsible:    freebsd-threads
> >State:          open
> >Quarter:        
> >Keywords:       
> >Date-Required:
> >Class:          sw-bug
> >Submitter-Id:   current-users
> >Arrival-Date:   Sat Dec 05 20:40:01 UTC 2009
> >Closed-Date:
> >Last-Modified:
> >Originator:     Jeremy Huddleston
> >Release:        8.0
> >Organization:
> Apple
> >Environment:
> NA
> >Description:
> libc/stdio assumes PTHREAD_MUTEX_INITIALIZER is NULL (which it is in 
FreeBSD), but this makes the code not as portable.
> 
> Earlier versions of stdio did properly initialize the lock to 
PTHREAD_MUTEX_INITIALIZER in INITEXTRA() when it was part of the _extra 
extension substructure.

This is my fault.  I suspect it was more an error of omission on my part than 
assuming the default value of PTHREAD_MUTEX_INITIALIZER. :)  Hmm, so I 
reviewed the change that removed INITEXTRA() and all the places it was used to 
construct a 'fake' FILE object it was passed to an internal stdio routine that 
did not use locking.  One thing I do see that is an old bug is that the three 
static FILE structures used for stdin, stdout, and stderr have never had their 
internal locks properly initialized.  Also, it seems __sfp() never initializes 
fp->_lock at all.  Oh, it gets set to NULL via 'empty' in moreglue().  That is 
also an old bug.  I think this will fix those problems:

Index: stdio/findfp.c
===================================================================
--- stdio/findfp.c	(revision 199969)
+++ stdio/findfp.c	(working copy)
@@ -61,6 +61,7 @@
 	._read = __sread,		\
 	._seek = __sseek,		\
 	._write = __swrite,		\
+	._fl_mutex = PTHREAD_MUTEX_INITIALIZER, \
 }
 				/* the usual - (stdin + stdout + stderr) */
 static FILE usual[FOPEN_MAX - 3];
@@ -96,7 +97,7 @@
 	int n;
 {
 	struct glue *g;
-	static FILE empty;
+	static FILE empty = { ._fl_mutex = PTHREAD_MUTEX_INITIALIZER };
 	FILE *p;
 
 	g = (struct glue *)malloc(sizeof(*g) + ALIGNBYTES + n * sizeof(FILE));
@@ -154,7 +155,7 @@
 	fp->_ub._size = 0;
 	fp->_lb._base = NULL;	/* no line buffer */
 	fp->_lb._size = 0;
-/*	fp->_lock = NULL; */	/* once set always set (reused) */
+/*	fp->_fl_mutex = NULL; */ /* once set always set (reused) */
 	fp->_orientation = 0;
 	memset(&fp->_mbstate, 0, sizeof(mbstate_t));
 	return (fp);


-- 
John Baldwin