]> git.ipfire.org Git - thirdparty/strongswan.git/commitdiff
thread-value: Defer cleanup handling to thread termination on Windows
authorMartin Willi <martin@revosec.ch>
Wed, 11 Jun 2014 12:24:22 +0000 (14:24 +0200)
committerMartin Willi <martin@revosec.ch>
Tue, 17 Jun 2014 13:19:40 +0000 (15:19 +0200)
Instead of cleaning up all thread-values during destruction, cleanup handler
is invoked when a thread detaches. Thread detaching is cough using the Windows
DllMain() entry point, and allows us to basically revert 204098a7.

Using this mechanism, we make sure that the cleanup handler is invoked by the
the correct thread. Further, this mechanism works for externally-spawned
threads which run outside of our thread_cb() routine, and works more efficiently
with short-running threads.

src/libstrongswan/threading/windows/thread.c
src/libstrongswan/threading/windows/thread.h
src/libstrongswan/threading/windows/thread_value.c

index 2b273413c2ef5907cbf2fe63f48ddb4171deb355..8792237e62f17866944e1ecdeff8ab04eded8d62 100644 (file)
@@ -245,36 +245,6 @@ void* thread_tls_remove(void *key)
        return value;
 }
 
-/**
- * See header.
- */
-void thread_tls_remove_all(void *key)
-{
-       private_thread_t *thread;
-       enumerator_t *enumerator;
-       void *value;
-       bool old;
-
-       old = set_leak_detective(FALSE);
-       threads_lock->lock(threads_lock);
-
-       enumerator = threads->create_enumerator(threads);
-       while (enumerator->enumerate(enumerator, NULL, &thread))
-       {
-               value = thread->tls->remove(thread->tls, key);
-               if (value)
-               {
-                       set_leak_detective(old);
-                       thread_tls_cleanup(value);
-                       set_leak_detective(FALSE);
-               }
-       }
-       enumerator->destroy(enumerator);
-
-       threads_lock->unlock(threads_lock);
-       set_leak_detective(old);
-}
-
 /**
  * Thread cleanup data
  */
@@ -634,6 +604,50 @@ void thread_exit(void *val)
        ExitThread(0);
 }
 
+/**
+ * Clean up thread data while it detaches
+ */
+static void cleanup_tls()
+{
+       private_thread_t *this;
+       bool old;
+
+       old = set_leak_detective(FALSE);
+       threads_lock->lock(threads_lock);
+
+       this = threads->remove(threads, (void*)(uintptr_t)GetCurrentThreadId());
+
+       threads_lock->unlock(threads_lock);
+       set_leak_detective(old);
+
+       if (this)
+       {
+               /* If the thread exited, but has not been joined, it is in terminated
+                * state. We must not mangle it, as we target externally spawned
+                * threads only. */
+               if (!this->terminated && !this->detached)
+               {
+                       destroy(this);
+               }
+       }
+}
+
+/**
+ * DllMain called for dll events
+ */
+BOOL WINAPI DllMain(HINSTANCE hinstDLL, DWORD fdwReason, LPVOID lpvReserved)
+{
+       switch (fdwReason)
+       {
+               case DLL_THREAD_DETACH:
+                       cleanup_tls();
+                       break;
+               default:
+                       break;
+       }
+       return TRUE;
+}
+
 /*
  * Described in header.
  */
index 3c470522bb897dcd01711804378397c5b7c586e9..965d9690e492fc7cc8a2e843fbb6824314c99d11 100644 (file)
@@ -60,15 +60,6 @@ void* thread_tls_get(void *key);
  */
 void* thread_tls_remove(void *key);
 
-/**
- * Remove a thread specific value from all threads.
- *
- * For each found TLS value thread_tls_cleanup() is invoked.
- *
- * @param key          unique key specifying the TLS variable
- */
-void thread_tls_remove_all(void *key);
-
 /**
  * Cleanup function for thread specific value.
  *
index 1dd8a781655742a7ae61a5f87ae5ce91a1a29933..d7bd7e64c0ca9736683651658319cb13f84dbf48 100644 (file)
@@ -104,7 +104,13 @@ METHOD(thread_value_t, tls_get, void*,
 METHOD(thread_value_t, tls_destroy, void,
        private_thread_value_t *this)
 {
-       thread_tls_remove_all(this);
+       entry_t *entry;
+
+       entry = thread_tls_remove(this);
+       if (entry)
+       {
+               thread_tls_cleanup(entry);
+       }
        free(this);
 }