fscache: make fscache_enable() thread safe

The recent change to make fscache thread specific relied on fscache_enable()
being called first from the primary thread before being called in parallel
from worker threads.  Make that more robust and protect it with a critical
section to avoid any issues.

Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Ben Peart <benpeart@microsoft.com>

Author: benpeart

compat/mingw.c

@@ -14,6 +14,7 @@
 #include "symlinks.h"
 #include "trace2.h"
 #include "win32.h"
+#include "win32/fscache.h"
 #include "win32/lazyload.h"
 #include "wrapper.h"
 #include "write-or-die.h"
@@ -3735,6 +3736,9 @@ int wmain(int argc, const wchar_t **wargv)
 	/* initialize critical section for waitpid pinfo_t list */
 	InitializeCriticalSection(&pinfo_cs);
 
+	/* initialize critical section for fscache */
+	InitializeCriticalSection(&fscache_cs);
+
 	/* set up default file mode and file modes for stdin/out/err */
 	_fmode = _O_BINARY;
 	_setmode(_fileno(stdin), _O_BINARY);

compat/win32/fscache.c

@@ -10,7 +10,7 @@
 
 static volatile long initialized;
 static DWORD dwTlsIndex;
-static CRITICAL_SECTION mutex;
+CRITICAL_SECTION fscache_cs;
 
 /*
  * Store one fscache per thread to avoid thread contention and locking.
@@ -393,12 +393,12 @@ int fscache_enable(size_t initial_size)
 	 * opendir and lstat function pointers are redirected if
 	 * any threads are using the fscache.
 	 */
+	EnterCriticalSection(&fscache_cs);
 	if (!initialized) {
-		InitializeCriticalSection(&mutex);
 		if (!dwTlsIndex) {
 			dwTlsIndex = TlsAlloc();
 			if (dwTlsIndex == TLS_OUT_OF_INDEXES) {
-				LeaveCriticalSection(&mutex);
+				LeaveCriticalSection(&fscache_cs);
 				return 0;
 			}
 		}
@@ -407,12 +407,13 @@ int fscache_enable(size_t initial_size)
 		opendir = fscache_opendir;
 		lstat = fscache_lstat;
 	}
-	InterlockedIncrement(&initialized);
+	initialized++;
+	LeaveCriticalSection(&fscache_cs);
 
 	/* refcount the thread specific initialization */
 	cache = fscache_getcache();
 	if (cache) {
-		InterlockedIncrement(&cache->enabled);
+		cache->enabled++;
 	} else {
 		cache = (struct fscache *)xcalloc(1, sizeof(*cache));
 		cache->enabled = 1;
@@ -446,7 +447,7 @@ void fscache_disable(void)
 		BUG("fscache_disable() called on a thread where fscache has not been initialized");
 	if (!cache->enabled)
 		BUG("fscache_disable() called on an fscache that is already disabled");
-	InterlockedDecrement(&cache->enabled);
+	cache->enabled--;
 	if (!cache->enabled) {
 		TlsSetValue(dwTlsIndex, NULL);
 		trace_printf_key(&trace_fscache, "fscache_disable: lstat %u, opendir %u, "
@@ -459,12 +460,14 @@ void fscache_disable(void)
 	}
 
 	/* update the global fscache initialization */
-	InterlockedDecrement(&initialized);
+	EnterCriticalSection(&fscache_cs);
+	initialized--;
 	if (!initialized) {
 		/* reset opendir and lstat to the original implementations */
 		opendir = dirent_opendir;
 		lstat = mingw_lstat;
 	}
+	LeaveCriticalSection(&fscache_cs);
 
 	trace_printf_key(&trace_fscache, "fscache: disable\n");
 	return;
@@ -638,7 +641,7 @@ void fscache_merge(struct fscache *dest)
 	 * isn't being used so the critical section only needs to prevent
 	 * the the child threads from stomping on each other.
 	 */
-	EnterCriticalSection(&mutex);
+	EnterCriticalSection(&fscache_cs);
 
 	hashmap_iter_init(&cache->map, &iter);
 	while ((e = hashmap_iter_next(&iter)))
@@ -650,9 +653,9 @@ void fscache_merge(struct fscache *dest)
 	dest->opendir_requests += cache->opendir_requests;
 	dest->fscache_requests += cache->fscache_requests;
 	dest->fscache_misses += cache->fscache_misses;
-	LeaveCriticalSection(&mutex);
+	initialized--;
+	LeaveCriticalSection(&fscache_cs);
 
 	free(cache);
 
-	InterlockedDecrement(&initialized);
 }

compat/win32/fscache.h

@@ -6,6 +6,8 @@
  * for each thread where caching is desired.
  */
 
+extern CRITICAL_SECTION fscache_cs;
+
 int fscache_enable(size_t initial_size);
 #define enable_fscache(initial_size) fscache_enable(initial_size)