From 511f6c9a507152df1c8f782b3be29e67e12b654e Mon Sep 17 00:00:00 2001
From: Alexander Smarus <bender@readdle.com>
Date: Sat, 11 Apr 2020 17:03:04 +0300
Subject: [PATCH] [Windows] CFThreadSpecific implementation allows access to
 deallocated value

Unlike pthread-based implementation, TLS/FLS on Windows doesn't
return NULL when reading value after destructor call.

To avoid that we have to nullify value in destructor callback.
The implementation needs to store key along with user
data to perform proper cleanup.
---
 CoreFoundation/Base.subproj/CFPlatform.c      | 36 +++++++++++++++++--
 .../Tests/TestNotificationQueue.swift         |  7 ++++
 2 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/CoreFoundation/Base.subproj/CFPlatform.c b/CoreFoundation/Base.subproj/CFPlatform.c
index 5e9ad96c29..8dc9b944f0 100644
--- a/CoreFoundation/Base.subproj/CFPlatform.c
+++ b/CoreFoundation/Base.subproj/CFPlatform.c
@@ -1390,8 +1390,22 @@ CF_PRIVATE int asprintf(char **ret, const char *format, ...) {
 extern void swift_retain(void *);
 extern void swift_release(void *);
 
+#if TARGET_OS_WIN32
+typedef struct _CFThreadSpecificData {
+    CFTypeRef value;
+    _CFThreadSpecificKey key;
+} _CFThreadSpecificData;
+#endif
+
 static void _CFThreadSpecificDestructor(void *ctx) {
+#if TARGET_OS_WIN32
+    _CFThreadSpecificData *data = (_CFThreadSpecificData *)ctx;
+    FlsSetValue(data->key, NULL);
+    swift_release(data->value);
+    free(data);
+#else
     swift_release(ctx);
+#endif
 }
 
 _CFThreadSpecificKey _CFThreadSpecificKeyCreate() {
@@ -1406,18 +1420,36 @@ _CFThreadSpecificKey _CFThreadSpecificKeyCreate() {
 
 CFTypeRef _Nullable _CFThreadSpecificGet(_CFThreadSpecificKey key) {
 #if TARGET_OS_WIN32
-  return (CFTypeRef)FlsGetValue(key);
+    _CFThreadSpecificData *data = (_CFThreadSpecificData *)FlsGetValue(key);
+    if (data == NULL) {
+        return NULL;
+    }
+    return data->value;
 #else
     return (CFTypeRef)pthread_getspecific(key);
 #endif
 }
 
 void _CFThreadSpecificSet(_CFThreadSpecificKey key, CFTypeRef _Nullable value) {
+    // Intentionally not calling `swift_release` for previous value.
+    // OperationQueue uses these API (through NSThreadSpecific), and balances
+    // retain count manually.
 #if TARGET_OS_WIN32
+    free(FlsGetValue(key));
+
+    _CFThreadSpecificData *data = NULL;
     if (value != NULL) {
+        data = malloc(sizeof(_CFThreadSpecificData));
+        if (!data) {
+            HALT_MSG("Out of memory");
+        }
+        data->value = value;
+        data->key = key;
+
         swift_retain((void *)value);
     }
-    FlsSetValue(key, value);
+
+    FlsSetValue(key, data);
 #else
     if (value != NULL) {
         swift_retain((void *)value);
diff --git a/Tests/Foundation/Tests/TestNotificationQueue.swift b/Tests/Foundation/Tests/TestNotificationQueue.swift
index c23f778bf7..be26af0ef2 100644
--- a/Tests/Foundation/Tests/TestNotificationQueue.swift
+++ b/Tests/Foundation/Tests/TestNotificationQueue.swift
@@ -229,5 +229,12 @@ class TestNotificationQueue : XCTestCase {
         bgThread.start()
 
         waitForExpectations(timeout: 0.2)
+
+        // There is a small time gap between "e.fulfill()"
+        // and actuall thread termination.
+        // We need a little delay to allow bgThread actually die.
+        // Callers of this function are assuming thread is
+        // deallocated after call.
+        Thread.sleep(forTimeInterval: 0.05)
     }
 }