From owner-freebsd-threads@FreeBSD.ORG Wed Jan 6 21:25:27 2010 Return-Path: Delivered-To: freebsd-threads@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 13F7E106568F; Wed, 6 Jan 2010 21:25:27 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id C78BD8FC26; Wed, 6 Jan 2010 21:25:26 +0000 (UTC) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id 64B0746B51; Wed, 6 Jan 2010 16:25:26 -0500 (EST) Received: from jhbbsd.localnet (unknown [209.249.190.9]) by bigwig.baldwin.cx (Postfix) with ESMTPA id 9E18D8A01F; Wed, 6 Jan 2010 16:25:25 -0500 (EST) From: John Baldwin To: freebsd-threads@freebsd.org Date: Wed, 6 Jan 2010 16:00:47 -0500 User-Agent: KMail/1.12.1 (FreeBSD/7.2-CBSD-20091103; KDE/4.3.1; amd64; ; ) References: <200912052034.nB5KYfaY000395@www.freebsd.org> <200912070850.37112.jhb@freebsd.org> In-Reply-To: <200912070850.37112.jhb@freebsd.org> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Message-Id: <201001061600.47628.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.0.1 (bigwig.baldwin.cx); Wed, 06 Jan 2010 16:25:25 -0500 (EST) X-Virus-Scanned: clamav-milter 0.95.1 at bigwig.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-2.5 required=4.2 tests=AWL,BAYES_00,RDNS_NONE autolearn=no version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on bigwig.baldwin.cx Cc: freebsd-gnats-submit@freebsd.org, Jeremy Huddleston Subject: Re: threads/141198: src/lib/libc/stdio does not properly initialize mutexes X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 06 Jan 2010 21:25:27 -0000 On Monday 07 December 2009 8:50:37 am John Baldwin wrote: > 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); Does this patch address the concerns? -- John Baldwin