Refactor ScopeGuard

Use pthread_mutex_t on Darwin platforms

Author: CodaFi

Sources/SwiftSyntax/CMakeLists.txt

@@ -10,6 +10,7 @@ add_library(SwiftSyntax STATIC
   AbsolutePosition.swift
   BumpPtrAllocator.swift
   IncrementalParseTransition.swift
+  PlatformMutex.swift
   SourceLength.swift
   SourceLocation.swift
   SourcePresence.swift

Sources/SwiftSyntax/PlatformMutex.swift

@@ -0,0 +1,82 @@
+//===------ PlatformMutex.swift - Platform-specific Mutual Exclusion  -----===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2022 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+#if canImport(Darwin)
+@_implementationOnly import Darwin
+#elseif canImport(Glibc)
+@_implementationOnly import Glibc
+#elseif canImport(WinSDK)
+@_implementationOnly import WinSDK
+#endif
+
+/// A protocol that platform-specific mutual exclusion primitives should conform to.
+struct PlatformMutex {
+  // FIXME: Use os_unfair_lock when we bump to macOS 12.0 on Darwin
+#if canImport(Darwin) || canImport(Glibc)
+  typealias Primitive = pthread_mutex_t
+#elseif canImport(WinSDK)
+  typealias Primitive = SRWLOCK
+#endif
+  typealias PlatformLock = UnsafeMutablePointer<Primitive>
+
+  private let lock: PlatformLock
+
+  /// Allocate memory for, and initialize, the mutex in a platform-specific fashion.
+  ///
+  /// - Parameter allocator: An allocator
+  init(allocator: BumpPtrAllocator) {
+    let storage = allocator.allocate(Primitive.self, count: 1).baseAddress!
+    storage.initialize(to: Primitive())
+#if canImport(Darwin) || canImport(Glibc)
+    pthread_mutex_init(storage, nil)
+#elseif canImport(WinSDK)
+    InitializeSRWLock(storage)
+#endif
+    self.lock = storage
+  }
+
+  /// Deinitialize the memory associated with the mutex.
+  ///
+  /// - Warning: Do not call `.deallocate()` on  any pointers allocated by the
+  ///            `BumpPtrAllocator` here.
+  func deinitialize() {
+#if canImport(Darwin) || canImport(Glibc)
+    pthread_mutex_destroy(self.lock)
+#endif
+    self.lock.deinitialize(count: 1)
+  }
+
+  /// Invoke the provided block under the protection of the mutex.
+  func withGuard<T>(body: () throws -> T) rethrows -> T {
+    Self.lock(self.lock)
+    defer { Self.unlock(self.lock) }
+    return try body()
+  }
+}
+
+extension PlatformMutex {
+  private static func lock(_ platformLock: PlatformLock) {
+#if canImport(Darwin) || canImport(Glibc)
+    pthread_mutex_lock(platformLock)
+#elseif canImport(WinSDK)
+    AcquireSRWLockExclusive(platformLock)
+#endif
+  }
+
+  private static func unlock(_ platformLock: PlatformLock) {
+#if canImport(Darwin) || canImport(Glibc)
+    pthread_mutex_unlock(platformLock)
+#elseif canImport(WinSDK)
+    ReleaseSRWLockExclusive(platformLock)
+#endif
+  }
+}

Sources/SwiftSyntax/SyntaxArena.swift

@@ -10,67 +10,13 @@
 //
 //===----------------------------------------------------------------------===//
 
-#if canImport(Darwin)
-import Darwin
-
-struct ScopeGuard {
-  private let lock: os_unfair_lock_t
-  init(allocator: BumpPtrAllocator) {
-    let storage = allocator.allocate(os_unfair_lock.self, count: 1).baseAddress!
-    storage.initialize(to: os_unfair_lock())
-    self.lock = os_unfair_lock_t(storage)
-  }
-
-  func deinitialize() {}
-
-  func withGuard<T>(body: () throws -> T) rethrows -> T {
-    os_unfair_lock_lock(lock)
-    defer { os_unfair_lock_unlock(lock)}
-    return try body()
-  }
-}
-
-#elseif canImport(Glibc)
-import Glibc
-
-struct ScopeGuard {
-  private let lock: UnsafeMutablePointer<pthread_mutex_t>
-  init(allocator: BumpPtrAllocator) {
-    let storage = allocator.allocate(pthread_mutex_t.self, count: 1).baseAddress!
-    storage.initialize(to: pthread_mutex_t())
-    pthread_mutex_init(storage, nil)
-    self.lock = storage
-  }
-  func deinitialize() {
-    pthread_mutex_destroy(self.lock)
-  }
-  func withGuard<T>(body: () throws -> T) rethrows -> T {
-    pthread_mutex_lock(self.lock)
-    defer { pthread_mutex_unlock(self.lock) }
-    return try body()
-  }
-}
-
-#else
-// FIXME: Support other platforms.
-
-/// Dummy mutex that doesn't actually guard at all.
-class ScopeGuard {
-  init() {}
-  func deinitialize() {}
-  func withGuard<T>(body: () throws -> T) rethrows -> T {
-    return try body()
-  }
-}
-#endif
-
 public class SyntaxArena {
 
   @_spi(RawSyntax)
   public typealias ParseTriviaFunction = (_ source: SyntaxText, _ position: TriviaPosition) -> [RawTriviaPiece]
 
   /// Thread safe guard.
-  private let lock: ScopeGuard
+  private let lock: PlatformMutex
   private var singleThreadMode: Bool
 
   /// Bump-pointer allocator for all "intern" methods.
@@ -89,7 +35,7 @@ public class SyntaxArena {
   @_spi(RawSyntax)
   public init(parseTriviaFunction: @escaping ParseTriviaFunction) {
     let allocator = BumpPtrAllocator()
-    self.lock = ScopeGuard(allocator: allocator)
+    self.lock = PlatformMutex(allocator: allocator)
     self.singleThreadMode = false
     self.allocator = allocator
     children = []
@@ -99,9 +45,8 @@ public class SyntaxArena {
   }
 
   deinit {
-    // NOTE: We don't make `ScopeGuard` a class and `deinit` in it to
-    // deinitialize it because the actual lock value is in `allocator`, and we
-    // want to make sure to deinitialize the lock before destroying the allocator.
+    // Make sure we give the platform lock a chance to deinitialize any
+    // memory it used.
     lock.deinitialize()
   }
 

Sources/SwiftSyntax/SyntaxText.swift

@@ -11,9 +11,9 @@
 //===----------------------------------------------------------------------===//
 
 #if canImport(Darwin)
-import Darwin
+@_implementationOnly import Darwin
 #elseif canImport(Glibc)
-import Glibc
+@_implementationOnly import Glibc
 #endif
 
 /// Represent a string.