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 Feb 12, 2019
1 parent 684a739 commit 575e914
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 9 deletions.
4 changes: 4 additions & 0 deletions compat/mingw.c
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "dir.h"
#include "../attr.h"
#include "../string-list.h"
#include "win32/fscache.h"

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

Expand Down Expand Up @@ -3156,6 +3157,9 @@ int wmain(int argc, const wchar_t **wargv)
InitializeCriticalSection(&pinfo_cs);
InitializeCriticalSection(&phantom_symlinks_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
21 changes: 12 additions & 9 deletions compat/win32/fscache.c
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,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 @@ -370,8 +370,8 @@ 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)
Expand All @@ -382,12 +382,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 @@ -421,7 +422,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 @@ -434,12 +435,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 @@ -606,7 +609,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 @@ -618,9 +621,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 575e914

Please sign in to comment.