Merge pull request swiftlang#80316 from mikeash/lock-record-begone

mikeash · web-flow · commit ecdcb82b9737 · 2025-03-28T17:18:40.000-04:00
[Concurrency] Eliminate StatusRecordLockRecord.
diff --git a/include/swift/ABI/MetadataValues.h b/include/swift/ABI/MetadataValues.h
@@ -62,9 +62,6 @@ enum {
   /// in a non-default distributed actor.
   NumWords_NonDefaultDistributedActor = 12,
 
-  /// The number of words in a task.
-  NumWords_AsyncTask = 24,
-
   /// The number of words in a task group.
   NumWords_TaskGroup = 32,
 
diff --git a/include/swift/ABI/Task.h b/include/swift/ABI/Task.h
@@ -298,7 +298,18 @@ class AsyncTask : public Job {
 
   /// Private storage for the use of the runtime.
   struct alignas(2 * alignof(void*)) OpaquePrivateStorage {
-    void *Storage[14];
+#if SWIFT_CONCURRENCY_ENABLE_PRIORITY_ESCALATION && SWIFT_POINTER_IS_4_BYTES
+    static constexpr size_t ActiveTaskStatusSize = 4 * sizeof(void *);
+#else
+    static constexpr size_t ActiveTaskStatusSize = 4 * sizeof(void *);
+#endif
+
+    // Private storage is currently 6 pointers, 16 bytes of non-pointer data,
+    // the ActiveTaskStatus, and a RecursiveMutex.
+    static constexpr size_t PrivateStorageSize =
+        6 * sizeof(void *) + 16 + ActiveTaskStatusSize + sizeof(RecursiveMutex);
+
+    void *Storage[PrivateStorageSize];
 
     /// Initialize this storage during the creation of a task.
     void initialize(JobPriority basePri);
@@ -755,8 +766,6 @@ class AsyncTask : public Job {
 };
 
 // The compiler will eventually assume these.
-static_assert(sizeof(AsyncTask) == NumWords_AsyncTask * sizeof(void*),
-              "AsyncTask size is wrong");
 static_assert(alignof(AsyncTask) == 2 * alignof(void*),
               "AsyncTask alignment is wrong");
 #pragma clang diagnostic push
diff --git a/include/swift/Threading/Impl/C11.h b/include/swift/Threading/Impl/C11.h
@@ -147,6 +147,25 @@ inline void lazy_mutex_unsafe_unlock(lazy_mutex_handle &handle) {
   (void)::mtx_unlock(&handle.mutex);
 }
 
+// .. Recursive mutex support .................................................
+
+using recursive_mutex_handle = ::mtx_t;
+
+inline void recursive_mutex_init(recursive_mutex_handle &handle,
+                                 bool checked = false) {
+  SWIFT_C11THREADS_CHECK(::mtx_init(&handle, ::mtx_recursive));
+}
+inline void recursive_mutex_destroy(recursive_mutex_handle &handle) {
+  ::mtx_destroy(&handle);
+}
+
+inline void recursive_mutex_lock(recursive_mutex_handle &handle) {
+  SWIFT_C11THREADS_CHECK(::mtx_lock(&handle));
+}
+inline void recursive_mutex_unlock(recursive_mutex_handle &handle) {
+  SWIFT_C11THREADS_CHECK(::mtx_unlock(&handle));
+}
+
 // .. ConditionVariable support ..............................................
 
 struct cond_handle {
diff --git a/include/swift/Threading/Impl/Darwin.h b/include/swift/Threading/Impl/Darwin.h
@@ -129,6 +129,42 @@ inline void lazy_mutex_unsafe_unlock(lazy_mutex_handle &handle) {
   ::os_unfair_lock_unlock(&handle);
 }
 
+// .. Recursive mutex support .................................................
+
+// The os_unfair_recursive_lock interface is stable, but not in the SDK. Bring
+// our own definitions for what we need.
+
+#define OS_UNFAIR_RECURSIVE_LOCK_INIT                                          \
+  (os_unfair_recursive_lock{OS_UNFAIR_LOCK_INIT, 0})
+
+typedef struct os_unfair_recursive_lock_s {
+  os_unfair_lock ourl_lock;
+  uint32_t ourl_count;
+} os_unfair_recursive_lock, *os_unfair_recursive_lock_t;
+
+using recursive_mutex_handle = os_unfair_recursive_lock;
+
+extern "C" void
+os_unfair_recursive_lock_lock_with_options(os_unfair_recursive_lock_t lock,
+                                           uint32_t options);
+
+extern "C" void
+os_unfair_recursive_lock_unlock(os_unfair_recursive_lock_t lock);
+
+inline void recursive_mutex_init(recursive_mutex_handle &handle,
+                                 bool checked = false) {
+  handle = OS_UNFAIR_RECURSIVE_LOCK_INIT;
+}
+inline void recursive_mutex_destroy(recursive_mutex_handle &handle) {}
+
+inline void recursive_mutex_lock(recursive_mutex_handle &handle) {
+  os_unfair_recursive_lock_lock_with_options(&handle, 0);
+}
+
+inline void recursive_mutex_unlock(recursive_mutex_handle &handle) {
+  os_unfair_recursive_lock_unlock(&handle);
+}
+
 // .. ConditionVariable support ..............................................
 
 struct cond_handle {
diff --git a/include/swift/Threading/Impl/Linux.h b/include/swift/Threading/Impl/Linux.h
@@ -134,6 +134,29 @@ inline void lazy_mutex_unsafe_unlock(lazy_mutex_handle &handle) {
   (void)::pthread_mutex_unlock(&handle);
 }
 
+// .. Recursive mutex support ................................................
+
+using recursive_mutex_handle = ::pthread_mutex_t;
+
+inline void recursive_mutex_init(recursive_mutex_handle &handle, bool checked = false) {
+  ::pthread_mutexattr_t attr;
+  SWIFT_LINUXTHREADS_CHECK(::pthread_mutexattr_init(&attr));
+  SWIFT_LINUXTHREADS_CHECK(
+      ::pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE));
+  SWIFT_LINUXTHREADS_CHECK(::pthread_mutex_init(&handle, &attr));
+  SWIFT_LINUXTHREADS_CHECK(::pthread_mutexattr_destroy(&attr));
+}
+inline void recursive_mutex_destroy(recursive_mutex_handle &handle) {
+  SWIFT_LINUXTHREADS_CHECK(::pthread_mutex_destroy(&handle));
+}
+
+inline void recursive_mutex_lock(recursive_mutex_handle &handle) {
+  SWIFT_LINUXTHREADS_CHECK(::pthread_mutex_lock(&handle));
+}
+inline void recursive_mutex_unlock(recursive_mutex_handle &handle) {
+  SWIFT_LINUXTHREADS_CHECK(::pthread_mutex_unlock(&handle));
+}
+
 // .. ConditionVariable support ..............................................
 
 struct cond_handle {
diff --git a/include/swift/Threading/Impl/Nothreads.h b/include/swift/Threading/Impl/Nothreads.h
@@ -60,6 +60,16 @@ inline bool lazy_mutex_try_lock(lazy_mutex_handle &handle) { return true; }
 inline void lazy_mutex_unsafe_lock(lazy_mutex_handle &handle) {}
 inline void lazy_mutex_unsafe_unlock(lazy_mutex_handle &handle) {}
 
+// .. Recursive mutex support .................................................
+
+using recursive_mutex_handle = unsigned;
+
+inline void recursive_mutex_init(recursive_mutex_handle &handle,
+                                 bool checked = false) {}
+inline void recursive_mutex_destroy(recursive_mutex_handle &handle) {}
+inline void recursive_mutex_lock(recursive_mutex_handle &handle) {}
+inline void recursive_mutex_unlock(recursive_mutex_handle &handle) {}
+
 // .. ConditionVariable support ..............................................
 
 using cond_handle = unsigned;
diff --git a/include/swift/Threading/Impl/Pthreads.h b/include/swift/Threading/Impl/Pthreads.h
@@ -130,6 +130,29 @@ inline void lazy_mutex_unsafe_unlock(lazy_mutex_handle &handle) {
   (void)::pthread_mutex_unlock(&handle);
 }
 
+// .. Recursive mutex support ................................................
+
+using recursive_mutex_handle = ::pthread_mutex_t;
+
+inline void recursive_mutex_init(recursive_mutex_handle &handle, bool checked = false) {
+  ::pthread_mutexattr_t attr;
+  SWIFT_PTHREADS_CHECK(::pthread_mutexattr_init(&attr));
+  SWIFT_PTHREADS_CHECK(
+      ::pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE));
+  SWIFT_PTHREADS_CHECK(::pthread_mutex_init(&handle, &attr));
+  SWIFT_PTHREADS_CHECK(::pthread_mutexattr_destroy(&attr));
+}
+inline void recursive_mutex_destroy(recursive_mutex_handle &handle) {
+  SWIFT_PTHREADS_CHECK(::pthread_mutex_destroy(&handle));
+}
+
+inline void recursive_mutex_lock(recursive_mutex_handle &handle) {
+  SWIFT_PTHREADS_CHECK(::pthread_mutex_lock(&handle));
+}
+inline void recursive_mutex_unlock(recursive_mutex_handle &handle) {
+  SWIFT_PTHREADS_CHECK(::pthread_mutex_unlock(&handle));
+}
+
 // .. ConditionVariable support ..............................................
 
 struct cond_handle {
diff --git a/include/swift/Threading/Impl/Win32.h b/include/swift/Threading/Impl/Win32.h
@@ -87,6 +87,26 @@ inline void lazy_mutex_unsafe_unlock(lazy_mutex_handle &handle) {
   ReleaseSRWLockExclusive(&handle);
 }
 
+// .. Recursive mutex support ................................................
+
+using recursive_mutex_handle = SWIFT_CRITICAL_SECTION;
+
+inline void recursive_mutex_init(recursive_mutex_handle &handle,
+                                 bool checked = false) {
+  InitializeCriticalSection(&handle);
+}
+
+inline void recursive_mutex_destroy(recursive_mutex_handle &handle) {
+  DeleteCriticalSection(&handle);
+}
+
+inline void recursive_mutex_lock(recursive_mutex_handle &handle) {
+  EnterCriticalSection(&handle);
+}
+inline void recursive_mutex_unlock(recursive_mutex_handle &handle) {
+  LeaveCriticalSection(&handle);
+}
+
 // .. ConditionVariable support ..............................................
 
 struct cond_handle {
diff --git a/include/swift/Threading/Impl/Win32/Win32Defs.h b/include/swift/Threading/Impl/Win32/Win32Defs.h
@@ -36,7 +36,14 @@ typedef unsigned char BYTE;
 typedef BYTE BOOLEAN;
 typedef int BOOL;
 typedef unsigned long DWORD;
+typedef long LONG;
 typedef unsigned long ULONG;
+#if defined(_WIN64)
+typedef unsigned __int64 ULONG_PTR;
+#else
+typedef unsigned long ULONG_PTR;
+#endif
+typedef PVOID HANDLE;
 
 typedef VOID(NTAPI *PFLS_CALLBACK_FUNCTION)(PVOID lpFlsData);
 
@@ -46,6 +53,11 @@ typedef PRTL_SRWLOCK PSRWLOCK;
 typedef struct _RTL_CONDITION_VARIABLE *PRTL_CONDITION_VARIABLE;
 typedef PRTL_CONDITION_VARIABLE PCONDITION_VARIABLE;
 
+typedef struct _RTL_CRITICAL_SECTION_DEBUG *PRTL_CRITICAL_SECTION_DEBUG;
+typedef struct _RTL_CRITICAL_SECTION *PRTL_CRITICAL_SECTION;
+typedef PRTL_CRITICAL_SECTION PCRITICAL_SECTION;
+typedef PRTL_CRITICAL_SECTION LPCRITICAL_SECTION;
+
 // These have to be #defines, to avoid problems with <windows.h>
 #define RTL_SRWLOCK_INIT {0}
 #define SRWLOCK_INIT RTL_SRWLOCK_INIT
@@ -83,6 +95,19 @@ WINBASEAPI BOOL WINAPI SleepConditionVariableSRW(
   ULONG Flags
 );
 
+WINBASEAPI VOID WINAPI InitializeCriticalSection(
+  LPCRITICAL_SECTION lpCriticalSection
+);
+WINBASEAPI VOID WINAPI DeleteCriticalSection(
+  LPCRITICAL_SECTION lpCriticalSection
+);
+WINBASEAPI VOID WINAPI EnterCriticalSection(
+  LPCRITICAL_SECTION lpCriticalSection
+);
+WINBASEAPI VOID WINAPI LeaveCriticalSection(
+  LPCRITICAL_SECTION lpCriticalSection
+);
+
 WINBASEAPI DWORD WINAPI FlsAlloc(PFLS_CALLBACK_FUNCTION lpCallback);
 WINBASEAPI PVOID WINAPI FlsGetValue(DWORD dwFlsIndex);
 WINBASEAPI BOOL WINAPI FlsSetValue(DWORD dwFlsIndex, PVOID lpFlsData);
@@ -140,6 +165,34 @@ inline BOOL SleepConditionVariableSRW(PSWIFT_CONDITION_VARIABLE CondVar,
     Flags);
 }
 
+// And with CRITICAL_SECTION
+#pragma pack(push, 8)
+typedef struct SWIFT_CRITICAL_SECTION {
+  PRTL_CRITICAL_SECTION_DEBUG DebugInfo;
+  LONG LockCount;
+  LONG RecursionCount;
+  HANDLE OwningThread;
+  HANDLE LockSemaphore;
+  ULONG_PTR SpinCount;
+} SWIFT_CRITICAL_SECTION, *PSWIFT_CRITICAL_SECTION;
+#pragma pack(pop)
+
+inline VOID InitializeCriticalSection(PSWIFT_CRITICAL_SECTION CritSec) {
+  ::InitializeCriticalSection(reinterpret_cast<LPCRITICAL_SECTION>(CritSec));
+}
+
+inline VOID DeleteCriticalSection(PSWIFT_CRITICAL_SECTION CritSec) {
+  ::DeleteCriticalSection(reinterpret_cast<LPCRITICAL_SECTION>(CritSec));
+}
+
+inline VOID EnterCriticalSection(PSWIFT_CRITICAL_SECTION CritSec) {
+  ::EnterCriticalSection(reinterpret_cast<LPCRITICAL_SECTION>(CritSec));
+}
+
+inline VOID LeaveCriticalSection(PSWIFT_CRITICAL_SECTION CritSec) {
+  ::LeaveCriticalSection(reinterpret_cast<LPCRITICAL_SECTION>(CritSec));
+}
+
 } // namespace threading_impl
 } // namespace swift
 
diff --git a/include/swift/Threading/Mutex.h b/include/swift/Threading/Mutex.h
@@ -241,6 +241,48 @@ class IndirectMutex {
 using SmallMutex =
     std::conditional_t<sizeof(Mutex) <= sizeof(void *), Mutex, IndirectMutex>;
 
+/// A recursive variant of Mutex.
+class RecursiveMutex {
+
+  RecursiveMutex(const RecursiveMutex &) = delete;
+  RecursiveMutex &operator=(const RecursiveMutex &) = delete;
+  RecursiveMutex(RecursiveMutex &&) = delete;
+  RecursiveMutex &operator=(RecursiveMutex &&) = delete;
+
+public:
+  /// Constructs a non-recursive mutex.
+  ///
+  /// If `checked` is true the mutex will attempt to check for misuse and
+  /// fatalError when detected. If `checked` is false (the default) the
+  /// mutex will make little to no effort to check for misuse (more efficient).
+  explicit RecursiveMutex(bool checked = false) {
+    threading_impl::recursive_mutex_init(Handle, checked);
+  }
+  ~RecursiveMutex() { threading_impl::recursive_mutex_destroy(Handle); }
+
+  /// The lock() method has the following properties:
+  /// - Behaves as an atomic operation.
+  /// - Blocks the calling thread until exclusive ownership of the mutex
+  ///   can be obtained.
+  /// - Prior m.unlock() operations on the same mutex synchronize-with
+  ///   this lock operation.
+  /// - Does not throw exceptions but will halt on error (fatalError).
+  void lock() { threading_impl::recursive_mutex_lock(Handle); }
+
+  /// The unlock() method has the following properties:
+  /// - Behaves as an atomic operation.
+  /// - Releases the calling thread's ownership of the mutex and
+  ///   synchronizes-with the subsequent successful lock operations on
+  ///   the same object.
+  /// - The behavior is undefined if the calling thread does not own
+  ///   the mutex.
+  /// - Does not throw exceptions but will halt on error (fatalError).
+  void unlock() { threading_impl::recursive_mutex_unlock(Handle); }
+
+protected:
+  threading_impl::recursive_mutex_handle Handle;
+};
+
 } // namespace swift
 
 #endif // SWIFT_THREADING_MUTEX_H
diff --git a/stdlib/public/Concurrency/TaskPrivate.h b/stdlib/public/Concurrency/TaskPrivate.h
@@ -29,6 +29,7 @@
 #include "swift/Runtime/Error.h"
 #include "swift/Runtime/Exclusivity.h"
 #include "swift/Runtime/HeapObject.h"
+#include "swift/Threading/Mutex.h"
 #include "swift/Threading/Thread.h"
 #include "swift/Threading/ThreadSanitizer.h"
 #include <atomic>
@@ -598,27 +599,25 @@ class alignas(2 * sizeof(void*)) ActiveTaskStatus {
 #endif
   }
 
-  /// Is there a lock on the linked list of status records?
+  /// Does some thread hold the status record lock?
   bool isStatusRecordLocked() const { return Flags & IsStatusRecordLocked; }
-  ActiveTaskStatus withLockingRecord(TaskStatusRecord *lockRecord) const {
+  ActiveTaskStatus withStatusRecordLocked() const {
     assert(!isStatusRecordLocked());
-    assert(lockRecord->Parent == Record);
 #if SWIFT_CONCURRENCY_ENABLE_PRIORITY_ESCALATION
-    return ActiveTaskStatus(lockRecord, Flags | IsStatusRecordLocked, ExecutionLock);
+    return ActiveTaskStatus(Record, Flags | IsStatusRecordLocked,
+                            ExecutionLock);
 #else
-    return ActiveTaskStatus(lockRecord, Flags | IsStatusRecordLocked);
+    return ActiveTaskStatus(Record, Flags | IsStatusRecordLocked);
 #endif
   }
-  ActiveTaskStatus withoutLockingRecord() const {
+  ActiveTaskStatus withoutStatusRecordLocked() const {
     assert(isStatusRecordLocked());
-    assert(Record->getKind() == TaskStatusRecordKind::Private_RecordLock);
 
-    // Remove the lock record, and put the next one as the head
-    auto newRecord = Record->Parent;
 #if SWIFT_CONCURRENCY_ENABLE_PRIORITY_ESCALATION
-    return ActiveTaskStatus(newRecord, Flags & ~IsStatusRecordLocked, ExecutionLock);
+    return ActiveTaskStatus(Record, Flags & ~IsStatusRecordLocked,
+                            ExecutionLock);
 #else
-    return ActiveTaskStatus(newRecord, Flags & ~IsStatusRecordLocked);
+    return ActiveTaskStatus(Record, Flags & ~IsStatusRecordLocked);
 #endif
   }
 
@@ -779,6 +778,9 @@ struct AsyncTask::PrivateStorage {
   /// async task stack when it is needed.
   TaskDependencyStatusRecord *dependencyRecord = nullptr;
 
+  // The lock used to protect more complicated operations on the task status.
+  RecursiveMutex statusLock;
+
   // Always create an async task with max priority in ActiveTaskStatus = base
   // priority. It will be updated later if needed.
   PrivateStorage(JobPriority basePri)
diff --git a/stdlib/public/Concurrency/TaskStatus.cpp b/stdlib/public/Concurrency/TaskStatus.cpp
diff --git a/stdlib/toolchain/Compatibility56/include/Concurrency/Task.h b/stdlib/toolchain/Compatibility56/include/Concurrency/Task.h
diff --git a/test/embedded/concurrency-leaks.swift b/test/embedded/concurrency-leaks.swift
