Re: [PATCH 27/36] attr: convert to new threadsafe API

From: Jeff King <peff@peff.net>
Date: Oct 26, 2016, 12:15 UTC
Message-ID: <20161026121525.twgiavpgfbr2ahvn@sigill.intra.peff.net>
In-Reply-To: <20161026093558.drsdwn2hm7bbcktt@ruderich.org>

On Wed, Oct 26, 2016 at 11:35:58AM +0200, Simon Ruderich wrote:

Show 24 quoted lines

> >  static pthread_mutex_t attr_mutex;
> > -#define attr_lock()pthread_mutex_lock(&attr_mutex)
> > +static inline void attr_lock(void)
> > +{
> > +	static int initialized;
> > +
> > +	if (!initialized) {
> > +		pthread_mutex_init(&attr_mutex, NULL);
> > +		initialized = 1;
> > +	}
> > +	pthread_mutex_lock(&attr_mutex);
> > +}
> 
> This may initialize the mutex multiple times during the first
> lock (which may happen in parallel).
> 
> pthread provides static initializers. To quote the man page:
> 
>     Variables of type pthread_mutex_t can also be initialized
>     statically, using the constants PTHREAD_MUTEX_INITIALIZER
>     (for fast mutexes), PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP
>     (for recursive mutexes), and
>     PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP (for error checking
>     mutexes).

I seem to recall this does not work on Windows, where the pthread functions are thin wrappers over CRITICAL_SECTION. Other threaded code in git does an explicit setup step before entering threaded sections. E.g., see start_threads() in builtin/grep.c.

-Peff