This is the mail archive of the cygwin-patches mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch] Fix multithreaded snprintf


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]