This is the mail archive of the
cygwin-patches
mailing list for the Cygwin project.
Re: [patch] Fix multithreaded snprintf
- From: Christopher Faylor <cgf-use-the-mailinglist-please at cygwin dot com>
- To: cygwin-patches at cygwin dot com
- Date: Thu, 6 Sep 2007 14:33:25 -0400
- Subject: Re: [patch] Fix multithreaded snprintf
- References: <46E04739.F0B0D169@dessent.net>
- Reply-to: cygwin-patches at cygwin dot com
On Thu, Sep 06, 2007 at 11:30:17AM -0700, Brian Dessent wrote:
>
>I tracked down the problem reported in
><http://www.cygwin.com/ml/cygwin/2007-09/msg00120.html>. The crash was
>occuring in pthread_mutex_lock, but that's a bit of a red herring. The
>real problem is that both newlib and Cygwin provide a
>include/sys/stdio.h file, however they were out of sync with regard to
>the _flockfile definition.
>
>This comes about because vsnprintf() is implemented by creating a struct
>FILE that represents the string buffer and then this is passed to the
>standard vfprintf(). The 'flags' member of this FILE has the __SSTR
>flag set to indicate that this is just a string buffer, and consequently
>no locking should or can be performed; the lock member isn't even
>initialized.
>
>The newlib/libc/include/sys/stdio.h therefore has this:
>
>#if !defined(_flockfile)
>#ifndef __SINGLE_THREAD__
># define _flockfile(fp) (((fp)->_flags & __SSTR) ? 0 :
>__lock_acquire_recursive((fp)->_lock))
>#else
># define _flockfile(fp)
>#endif
>#endif
>
>#if !defined(_funlockfile)
>#ifndef __SINGLE_THREAD__
># define _funlockfile(fp) (((fp)->_flags & __SSTR) ? 0 :
>__lock_release_recursive((fp)->_lock))
>#else
># define _funlockfile(fp)
>#endif
>#endif
>
>However, the Cygwin version of this header with the same name gets
>preference and doesn't know to check the flags like this, and thus
>unconditionally tries to lock the stream. This leads ultimately to a
>crash in pthread_mutex_lock because the lock member is just
>uninitialized junk.
>
>The attached patch fixes Cygwin's copy of the header and makes the
>poster's testcase function as expected. This only would appear in a
>multithreaded program because the __cygwin_lock_* functions expand to
>no-op in the case where there's only one thread.
>
>Since this is used in a C++ file (syscalls.cc) I couldn't do the "test ?
>0 : func()" idiom where void is the return type as that generates a
>compiler error, so I use an 'if'.
Thanks for the patch.
Go ahead and check this in but could you add a comment indicating that
this part of include/sys/stdio.h has to be kept in sync with newlib?
Nice catch!
cgf