diff options
| author | Patrick Steinhardt <ps@pks.im> | 2019-11-29 11:06:11 +0100 |
|---|---|---|
| committer | Patrick Steinhardt <ps@pks.im> | 2019-11-29 13:08:48 +0100 |
| commit | 5c6180b5a0e9f9bd1c82938137582720d7a2e6e6 (patch) | |
| tree | e11803137ebde8a54a44acd9307b56a2dc4e8b8a /src/win32 | |
| parent | 7f20778ba68d3f2c3fe9f523e24bead57392960d (diff) | |
| download | libgit2-5c6180b5a0e9f9bd1c82938137582720d7a2e6e6.tar.gz | |
global: convert to fiber-local storage to fix exit races
On Windows platforms, we automatically clean up the thread-local storage
upon detaching a thread via `DllMain()`. The thing is that this happens
for every thread of applications that link against the libgit2 DLL, even
those that don't have anything to do with libgit2 itself. As a result,
we cannot assume that these unsuspecting threads make use of our
`git_libgit2_init()` and `git_libgit2_shutdow()` reference counting,
which may lead to racy situations:
Thread 1 Thread 2
git_libgit2_shutdown()
DllMain(DETACH_THREAD)
git__free_tls_data()
git_atomic_dec() == 0
git__free_tls_data()
TlsFree(_tls_index)
TlsGetValue(_tls_index)
Due to the second thread never having executed `git_libgit2_init()`, the
first thread will clean up TLS data and as a result also free the
`_tls_index` variable. When detaching the second thread, we
unconditionally access the now-free'd `_tls_index` variable, which is
obviously not going to work out well.
Fix the issue by converting the code to use fiber-local storage instead
of thread-local storage. While FLS will behave the exact same as TLS if
no fibers are in use, it does allow us to specify a destructor similar
to the one that is accepted by pthread_key_create(3P). Like this, we do
not have to manually free indices anymore, but will let the FLS handle
calling the destructor. This allows us to get rid of `DllMain()`
completely, as we only used it to keep track of when threads were
exiting and results in an overall simplification of TLS cleanup.
Diffstat (limited to 'src/win32')
| -rw-r--r-- | src/win32/thread.c | 5 |
1 files changed, 0 insertions, 5 deletions
diff --git a/src/win32/thread.c b/src/win32/thread.c index c3a6c0e28..42dba7f97 100644 --- a/src/win32/thread.c +++ b/src/win32/thread.c @@ -32,8 +32,6 @@ static DWORD WINAPI git_win32__threadproc(LPVOID lpParameter) thread->result = thread->proc(thread->param); - git__free_tls_data(); - return CLEAN_THREAD_EXIT; } @@ -103,9 +101,6 @@ void git_thread_exit(void *value) { assert(GIT_GLOBAL->current_thread); GIT_GLOBAL->current_thread->result = value; - - git__free_tls_data(); - ExitThread(CLEAN_THREAD_EXIT); } |
