Skip to content

Commit b67da53

Browse files
benpeartdscho
authored andcommitted
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>
1 parent f2863c0 commit b67da53

File tree

3 files changed

+19
-10
lines changed

3 files changed

+19
-10
lines changed

compat/mingw.c

+4
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include <sspi.h>
2424
#include "../write-or-die.h"
2525
#include "../repository.h"
26+
#include "win32/fscache.h"
2627

2728
#define HCAST(type, handle) ((type)(intptr_t)handle)
2829

@@ -3730,6 +3731,9 @@ int wmain(int argc, const wchar_t **wargv)
37303731
/* initialize critical section for waitpid pinfo_t list */
37313732
InitializeCriticalSection(&pinfo_cs);
37323733

3734+
/* initialize critical section for fscache */
3735+
InitializeCriticalSection(&fscache_cs);
3736+
37333737
/* set up default file mode and file modes for stdin/out/err */
37343738
_fmode = _O_BINARY;
37353739
_setmode(_fileno(stdin), _O_BINARY);

compat/win32/fscache.c

+13-10
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010

1111
static volatile long initialized;
1212
static DWORD dwTlsIndex;
13-
static CRITICAL_SECTION mutex;
13+
CRITICAL_SECTION fscache_cs;
1414

1515
/*
1616
* Store one fscache per thread to avoid thread contention and locking.
@@ -388,12 +388,12 @@ int fscache_enable(size_t initial_size)
388388
* opendir and lstat function pointers are redirected if
389389
* any threads are using the fscache.
390390
*/
391+
EnterCriticalSection(&fscache_cs);
391392
if (!initialized) {
392-
InitializeCriticalSection(&mutex);
393393
if (!dwTlsIndex) {
394394
dwTlsIndex = TlsAlloc();
395395
if (dwTlsIndex == TLS_OUT_OF_INDEXES) {
396-
LeaveCriticalSection(&mutex);
396+
LeaveCriticalSection(&fscache_cs);
397397
return 0;
398398
}
399399
}
@@ -402,12 +402,13 @@ int fscache_enable(size_t initial_size)
402402
opendir = fscache_opendir;
403403
lstat = fscache_lstat;
404404
}
405-
InterlockedIncrement(&initialized);
405+
initialized++;
406+
LeaveCriticalSection(&fscache_cs);
406407

407408
/* refcount the thread specific initialization */
408409
cache = fscache_getcache();
409410
if (cache) {
410-
InterlockedIncrement(&cache->enabled);
411+
cache->enabled++;
411412
} else {
412413
cache = (struct fscache *)xcalloc(1, sizeof(*cache));
413414
cache->enabled = 1;
@@ -441,7 +442,7 @@ void fscache_disable(void)
441442
BUG("fscache_disable() called on a thread where fscache has not been initialized");
442443
if (!cache->enabled)
443444
BUG("fscache_disable() called on an fscache that is already disabled");
444-
InterlockedDecrement(&cache->enabled);
445+
cache->enabled--;
445446
if (!cache->enabled) {
446447
TlsSetValue(dwTlsIndex, NULL);
447448
trace_printf_key(&trace_fscache, "fscache_disable: lstat %u, opendir %u, "
@@ -454,12 +455,14 @@ void fscache_disable(void)
454455
}
455456

456457
/* update the global fscache initialization */
457-
InterlockedDecrement(&initialized);
458+
EnterCriticalSection(&fscache_cs);
459+
initialized--;
458460
if (!initialized) {
459461
/* reset opendir and lstat to the original implementations */
460462
opendir = dirent_opendir;
461463
lstat = mingw_lstat;
462464
}
465+
LeaveCriticalSection(&fscache_cs);
463466

464467
trace_printf_key(&trace_fscache, "fscache: disable\n");
465468
return;
@@ -628,7 +631,7 @@ void fscache_merge(struct fscache *dest)
628631
* isn't being used so the critical section only needs to prevent
629632
* the the child threads from stomping on each other.
630633
*/
631-
EnterCriticalSection(&mutex);
634+
EnterCriticalSection(&fscache_cs);
632635

633636
hashmap_iter_init(&cache->map, &iter);
634637
while ((e = hashmap_iter_next(&iter)))
@@ -640,9 +643,9 @@ void fscache_merge(struct fscache *dest)
640643
dest->opendir_requests += cache->opendir_requests;
641644
dest->fscache_requests += cache->fscache_requests;
642645
dest->fscache_misses += cache->fscache_misses;
643-
LeaveCriticalSection(&mutex);
646+
initialized--;
647+
LeaveCriticalSection(&fscache_cs);
644648

645649
free(cache);
646650

647-
InterlockedDecrement(&initialized);
648651
}

compat/win32/fscache.h

+2
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
* for each thread where caching is desired.
77
*/
88

9+
extern CRITICAL_SECTION fscache_cs;
10+
911
int fscache_enable(size_t initial_size);
1012
#define enable_fscache(initial_size) fscache_enable(initial_size)
1113

0 commit comments

Comments
 (0)