Skip to content

Commit

Permalink
fscache: make fscache_enable() thread safe
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
benpeart authored and dscho committed Sep 18, 2024
1 parent a6a4906 commit 29054d2
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 10 deletions.
4 changes: 4 additions & 0 deletions compat/mingw.c
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <sspi.h>
#include "../write-or-die.h"
#include "../repository.h"
#include "win32/fscache.h"

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

Expand Down Expand Up @@ -3490,6 +3491,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);
Expand Down
23 changes: 13 additions & 10 deletions compat/win32/fscache.c
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -388,12 +388,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;
}
}
Expand All @@ -402,12 +402,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;
Expand Down Expand Up @@ -441,7 +442,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, "
Expand All @@ -454,12 +455,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;
Expand Down Expand Up @@ -628,7 +631,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)))
Expand All @@ -640,9 +643,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);
}
2 changes: 2 additions & 0 deletions compat/win32/fscache.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down

0 comments on commit 29054d2

Please sign in to comment.