Use a single FLS index for cleanup

glib/glib-init.h

@@ -33,7 +33,7 @@ void g_error_init (void);
 #include <windows.h>
 
 void g_thread_win32_process_detach (void);
-void __stdcall g_thread_win32_thread_detach (void *lpflsdata);
+void g_thread_win32_thread_detach (void);
 void g_thread_win32_init (void);
 void g_console_win32_init (void);
 void g_clock_win32_init (void);

glib/gthread-win32.c

@@ -304,6 +304,50 @@ struct _GPrivateDestructor
 static GPrivateDestructor *g_private_destructors;  /* (atomic) prepend-only */
 static CRITICAL_SECTION g_private_lock;
 
+/* `GPrivateDestructor`s need to be run when the glib library is unloaded
+ * (dynamic linking), on thread exit (any linking style), or on process exit
+ * (all threads exit).
+ * 
+ * Whereas on POSIX platforms, `pthread_key_create` allows setting a destructor,
+ * on Windows, `TlsAlloc` does not.
+ *
+ * Exporting a `DllMain` function lets us subscribe to lifetime events like
+ * `DLL_THREAD_DETACH`, but that only works when executables link dynamically
+ * against glib (ie. they load `glib-2.0.dll` at runtime). When static linking,
+ * multiple `DllMain` symbols collide, and none of them would be called by the
+ * runtime linker anyway.
+ * 
+ * The `.CRT$XLE` section (see
+ * https://gitlab.gnome.org/GNOME/glib/-/merge_requests/1655/) is an
+ * implementation detail of the CRT: it's not officially supported on Windows
+ * and should probably not be used.
+ * 
+ * Fiber-Local Storage (FLS) however, lets us specify a clean-up callback that
+ * works with all linking styles: a function passed to `FlsAlloc` will be called
+ * when that index is freed, on process exit, or on thread exit.
+ * 
+ * To clarify: when a thread exits, the destructor for "all FLS indexes used by
+ * that thread" will be called on that thread. When the process exits or an FLS
+ * index is freed, all destructors for all FLS indexes in use will be called on
+ * their respective threads.
+ * 
+ * Because we already keep track of which destructors need to be run, in the
+ * atomic+prepend-only `g_private_destructors` list, we only really need one FLS
+ * destructor. So, we reserve one FLS index purely for cleanup. Its destructor
+ * is called on any thread that uses a `GPrivate`, which gives that thread the
+ * opportunity to run destructors.
+ * 
+ * Because `0` might be a valid FLS/TLS index, we initialize this to
+ * `FLS_OUT_OF_INDEXES`, and abort if `FlsAlloc` fails.
+ * 
+ * This variable is protected by `g_private_lock`.
+ */
+static DWORD g_private_fls_cleanup_index = FLS_OUT_OF_INDEXES;
+
+static void __stdcall g_private_fls_cleanup_callback(void *pflsdata) {
+  g_thread_win32_thread_detach ();
+}
+
 static DWORD
 g_private_get_impl (GPrivate *key)
 {
@@ -312,29 +356,47 @@ g_private_get_impl (GPrivate *key)
   if G_UNLIKELY (impl == 0)
     {
       EnterCriticalSection (&g_private_lock);
+
+      /* This might be the first time we use `GPrivate` values in this process,
+       * we might have to initialize the FLS cleanup routine */
+      if (g_private_fls_cleanup_index == FLS_OUT_OF_INDEXES) {
+        /* Register a process-wide cleanup routine (that will be run separately on every thread,
+         * on thread exit, library unload, or process exit) */
+        g_private_fls_cleanup_index = FlsAlloc (g_private_fls_cleanup_callback);
+        if (g_private_fls_cleanup_index == FLS_OUT_OF_INDEXES)
+          g_thread_abort(0, "FlsAlloc");
+      }
+
+      /* The global FLS cleanup routine is registered, but for it to run on this
+       * thread, we have to use the index from this thread at least once. Because
+       * We're already in the slow path (we're about to allocate a TLS index for this
+       * `key`), we set the value for the FLS cleanup slot */
+      FlsSetValue (g_private_fls_cleanup_index, 1 /* any non-zero value will do */); 
+
       impl = (UINT_PTR) key->p;
       if (impl == 0)
         {
           GPrivateDestructor *destructor;
 
-          /* FLS (Fiber-Local Storage) is used instead of TLS (Thread-Local Storage)
-           * to take advantage of its cleanup mechanism, see 
-           * https://devblogs.microsoft.com/oldnewthing/20191011-00/?p=102989
-           */
-          impl = FlsAlloc (g_thread_win32_thread_detach);
+          impl = TlsAlloc ();
 
           if G_UNLIKELY (impl == 0)
             {
-              /* Ignore FLS index 0 temporarily (as 0 is the indicator that we
-               * haven't allocated TLS yet) and alloc again;
-               * See https://gitlab.gnome.org/GNOME/glib/-/issues/2058 */
-              DWORD impl2 = FlsAlloc (g_thread_win32_thread_detach);
+              /* Technically, no documentation ever says that 0 is /not/ a valid
+               * TLS index. However, due to the definition of `G_PRIVATE_INIT`, 0
+               * is also our "uninitialized" sentinel value. In the very unlikely
+               * case `TlsAlloc` returned zero, just ignore that index and try
+               * again.
+               *
+               * See https://gitlab.gnome.org/GNOME/glib/-/issues/2058 
+               */
+              DWORD impl2 = TlsAlloc ();
               TlsFree (impl);
               impl = impl2;
             }
 
-          if (impl == FLS_OUT_OF_INDEXES || impl == 0)
-            g_thread_abort (0, "FlsAlloc");
+          if (impl == TLS_OUT_OF_INDEXES || impl == 0)
+            g_thread_abort (0, "TlsAlloc");
 
           if (key->notify != NULL)
             {
@@ -370,14 +432,14 @@ g_private_get_impl (GPrivate *key)
 gpointer
 g_private_get (GPrivate *key)
 {
-  return FlsGetValue (g_private_get_impl (key));
+  return TlsGetValue (g_private_get_impl (key));
 }
 
 void
 g_private_set (GPrivate *key,
                gpointer  value)
 {
-  FlsSetValue (g_private_get_impl (key), value);
+  TlsSetValue (g_private_get_impl (key), value);
 }
 
 void
@@ -387,8 +449,8 @@ g_private_replace (GPrivate *key,
   DWORD impl = g_private_get_impl (key);
   gpointer old;
 
-  old = FlsGetValue (impl);
-  FlsSetValue (impl, value);
+  old = TlsGetValue (impl);
+  TlsSetValue (impl, value);
   if (old && key->notify)
     key->notify (old);
 }
@@ -685,7 +747,7 @@ g_thread_win32_init (void)
 #endif
 }
 
-void __stdcall g_thread_win32_thread_detach (void *lpflsdata)
+void g_thread_win32_thread_detach (void)
 {
   gboolean dtors_called;
 
@@ -696,7 +758,7 @@ void __stdcall g_thread_win32_thread_detach (void *lpflsdata)
       /* We go by the POSIX book on this one.
        *
        * If we call a destructor then there is a chance that some new
-       * FLS variables got set by code called in that destructor.
+       * TLS variables got set by code called in that destructor.
        *
        * Loop until nothing is left.
        */
@@ -706,11 +768,11 @@ void __stdcall g_thread_win32_thread_detach (void *lpflsdata)
         {
           gpointer value;
 
-          value = FlsGetValue (dtor->index);
+          value = TlsGetValue (dtor->index);
           if (value != NULL && dtor->notify != NULL)
             {
               /* POSIX says to clear this before the call */
-              FlsSetValue (dtor->index, NULL);
+              TlsSetValue (dtor->index, NULL);
               dtor->notify (value);
               dtors_called = TRUE;
             }