better abstration for MetricsSystem state (#112)

motivation: prepare for sendable checks (they dont work well with static state)

changes:
* abstract the MetricsSystem state into a "boxed" class that handles the locking
* adjust call sites

Sources/CoreMetrics/Locks.swift

@@ -26,29 +26,54 @@
 //
 //===----------------------------------------------------------------------===//
 
-#if os(macOS) || os(iOS) || os(tvOS) || os(watchOS)
+#if canImport(WASILibc)
+// No locking on WASILibc
+#elseif canImport(Darwin)
 import Darwin
-#else
+#elseif os(Windows)
+import WinSDK
+#elseif canImport(Glibc)
 import Glibc
+#else
+#error("Unsupported runtime")
 #endif
 
 /// A threading lock based on `libpthread` instead of `libdispatch`.
 ///
 /// This object provides a lock on top of a single `pthread_mutex_t`. This kind
 /// of lock is safe to use with `libpthread`-based threading models, such as the
-/// one used by NIO.
+/// one used by NIO. On Windows, the lock is based on the substantially similar
+/// `SRWLOCK` type.
 internal final class Lock {
-    private let mutex: UnsafeMutablePointer<pthread_mutex_t> = UnsafeMutablePointer.allocate(capacity: 1)
+    #if os(Windows)
+    fileprivate let mutex: UnsafeMutablePointer<SRWLOCK> =
+        UnsafeMutablePointer.allocate(capacity: 1)
+    #else
+    fileprivate let mutex: UnsafeMutablePointer<pthread_mutex_t> =
+        UnsafeMutablePointer.allocate(capacity: 1)
+    #endif
 
     /// Create a new lock.
     public init() {
-        let err = pthread_mutex_init(self.mutex, nil)
-        precondition(err == 0, "pthread_mutex_init failed with error \(err)")
+        #if os(Windows)
+        InitializeSRWLock(self.mutex)
+        #else
+        var attr = pthread_mutexattr_t()
+        pthread_mutexattr_init(&attr)
+        pthread_mutexattr_settype(&attr, .init(PTHREAD_MUTEX_ERRORCHECK))
+
+        let err = pthread_mutex_init(self.mutex, &attr)
+        precondition(err == 0, "\(#function) failed in pthread_mutex with error \(err)")
+        #endif
     }
 
     deinit {
+        #if os(Windows)
+        // SRWLOCK does not need to be free'd
+        #else
         let err = pthread_mutex_destroy(self.mutex)
-        precondition(err == 0, "pthread_mutex_destroy failed with error \(err)")
+        precondition(err == 0, "\(#function) failed in pthread_mutex with error \(err)")
+        #endif
         self.mutex.deallocate()
     }
 
@@ -57,17 +82,25 @@ internal final class Lock {
     /// Whenever possible, consider using `withLock` instead of this method and
     /// `unlock`, to simplify lock handling.
     public func lock() {
+        #if os(Windows)
+        AcquireSRWLockExclusive(self.mutex)
+        #else
         let err = pthread_mutex_lock(self.mutex)
-        precondition(err == 0, "pthread_mutex_lock failed with error \(err)")
+        precondition(err == 0, "\(#function) failed in pthread_mutex with error \(err)")
+        #endif
     }
 
     /// Release the lock.
     ///
     /// Whenever possible, consider using `withLock` instead of this method and
     /// `lock`, to simplify lock handling.
     public func unlock() {
+        #if os(Windows)
+        ReleaseSRWLockExclusive(self.mutex)
+        #else
         let err = pthread_mutex_unlock(self.mutex)
-        precondition(err == 0, "pthread_mutex_unlock failed with error \(err)")
+        precondition(err == 0, "\(#function) failed in pthread_mutex with error \(err)")
+        #endif
     }
 }
 
@@ -96,62 +129,109 @@ extension Lock {
     }
 }
 
-/// A threading lock based on `libpthread` instead of `libdispatch`.
+/// A reader/writer threading lock based on `libpthread` instead of `libdispatch`.
 ///
-/// This object provides a lock on top of a single `pthread_mutex_t`. This kind
+/// This object provides a lock on top of a single `pthread_rwlock_t`. This kind
 /// of lock is safe to use with `libpthread`-based threading models, such as the
-/// one used by NIO.
+/// one used by NIO. On Windows, the lock is based on the substantially similar
+/// `SRWLOCK` type.
 internal final class ReadWriteLock {
-    private let rwlock: UnsafeMutablePointer<pthread_rwlock_t> = UnsafeMutablePointer.allocate(capacity: 1)
+    #if canImport(WASILibc)
+    // WASILibc is single threaded, provides no locks
+    #elseif os(Windows)
+    fileprivate let rwlock: UnsafeMutablePointer<SRWLOCK> =
+        UnsafeMutablePointer.allocate(capacity: 1)
+    fileprivate var shared: Bool = true
+    #else
+    fileprivate let rwlock: UnsafeMutablePointer<pthread_rwlock_t> =
+        UnsafeMutablePointer.allocate(capacity: 1)
+    #endif
 
     /// Create a new lock.
     public init() {
+        #if canImport(WASILibc)
+        // WASILibc is single threaded, provides no locks
+        #elseif os(Windows)
+        InitializeSRWLock(self.rwlock)
+        #else
         let err = pthread_rwlock_init(self.rwlock, nil)
-        precondition(err == 0, "pthread_rwlock_init failed with error \(err)")
+        precondition(err == 0, "\(#function) failed in pthread_rwlock with error \(err)")
+        #endif
     }
 
     deinit {
+        #if canImport(WASILibc)
+        // WASILibc is single threaded, provides no locks
+        #elseif os(Windows)
+        // SRWLOCK does not need to be free'd
+        #else
         let err = pthread_rwlock_destroy(self.rwlock)
-        precondition(err == 0, "pthread_rwlock_destroy failed with error \(err)")
+        precondition(err == 0, "\(#function) failed in pthread_rwlock with error \(err)")
+        #endif
         self.rwlock.deallocate()
     }
 
     /// Acquire a reader lock.
     ///
-    /// Whenever possible, consider using `withLock` instead of this method and
-    /// `unlock`, to simplify lock handling.
+    /// Whenever possible, consider using `withReaderLock` instead of this
+    /// method and `unlock`, to simplify lock handling.
     public func lockRead() {
+        #if canImport(WASILibc)
+        // WASILibc is single threaded, provides no locks
+        #elseif os(Windows)
+        AcquireSRWLockShared(self.rwlock)
+        self.shared = true
+        #else
         let err = pthread_rwlock_rdlock(self.rwlock)
-        precondition(err == 0, "pthread_rwlock_rdlock failed with error \(err)")
+        precondition(err == 0, "\(#function) failed in pthread_rwlock with error \(err)")
+        #endif
     }
 
     /// Acquire a writer lock.
     ///
-    /// Whenever possible, consider using `withLock` instead of this method and
-    /// `unlock`, to simplify lock handling.
+    /// Whenever possible, consider using `withWriterLock` instead of this
+    /// method and `unlock`, to simplify lock handling.
     public func lockWrite() {
+        #if canImport(WASILibc)
+        // WASILibc is single threaded, provides no locks
+        #elseif os(Windows)
+        AcquireSRWLockExclusive(self.rwlock)
+        self.shared = false
+        #else
         let err = pthread_rwlock_wrlock(self.rwlock)
-        precondition(err == 0, "pthread_rwlock_wrlock failed with error \(err)")
+        precondition(err == 0, "\(#function) failed in pthread_rwlock with error \(err)")
+        #endif
     }
 
     /// Release the lock.
     ///
-    /// Whenever possible, consider using `withLock` instead of this method and
-    /// `lock`, to simplify lock handling.
+    /// Whenever possible, consider using `withReaderLock` and `withWriterLock`
+    /// instead of this method and `lockRead` and `lockWrite`, to simplify lock
+    /// handling.
     public func unlock() {
+        #if canImport(WASILibc)
+        // WASILibc is single threaded, provides no locks
+        #elseif os(Windows)
+        if self.shared {
+            ReleaseSRWLockShared(self.rwlock)
+        } else {
+            ReleaseSRWLockExclusive(self.rwlock)
+        }
+        #else
         let err = pthread_rwlock_unlock(self.rwlock)
-        precondition(err == 0, "pthread_rwlock_unlock failed with error \(err)")
+        precondition(err == 0, "\(#function) failed in pthread_rwlock with error \(err)")
+        #endif
     }
 }
 
 extension ReadWriteLock {
     /// Acquire the reader lock for the duration of the given block.
     ///
-    /// This convenience method should be preferred to `lock` and `unlock` in
-    /// most situations, as it ensures that the lock will be released regardless
-    /// of how `body` exits.
+    /// This convenience method should be preferred to `lockRead` and `unlock`
+    /// in most situations, as it ensures that the lock will be released
+    /// regardless of how `body` exits.
     ///
-    /// - Parameter body: The block to execute while holding the lock.
+    /// - Parameter body: The block to execute while holding the reader lock.
     /// - Returns: The value returned by the block.
     @inlinable
     internal func withReaderLock<T>(_ body: () throws -> T) rethrows -> T {
@@ -164,11 +244,11 @@ extension ReadWriteLock {
 
     /// Acquire the writer lock for the duration of the given block.
     ///
-    /// This convenience method should be preferred to `lock` and `unlock` in
-    /// most situations, as it ensures that the lock will be released regardless
-    /// of how `body` exits.
+    /// This convenience method should be preferred to `lockWrite` and `unlock`
+    /// in most situations, as it ensures that the lock will be released
+    /// regardless of how `body` exits.
     ///
-    /// - Parameter body: The block to execute while holding the lock.
+    /// - Parameter body: The block to execute while holding the writer lock.
     /// - Returns: The value returned by the block.
     @inlinable
     internal func withWriterLock<T>(_ body: () throws -> T) rethrows -> T {

Sources/CoreMetrics/Metrics.swift

@@ -455,17 +455,7 @@ extension Timer: CustomStringConvertible {
 /// configured. `MetricsSystem` is set up just once in a given program to set up the desired metrics backend
 /// implementation.
 public enum MetricsSystem {
-    fileprivate static let lock = ReadWriteLock()
-    fileprivate static var _factory: MetricsFactory = NOOPMetricsHandler.instance
-    fileprivate static var initialized = false
-
-    /// Acquire a writer lock for the duration of the given block.
-    ///
-    /// - Parameter body: The block to execute while holding the lock.
-    /// - Returns: The value returned by the block.
-    public static func withWriterLock<T>(_ body: () throws -> T) rethrows -> T {
-        return try self.lock.withWriterLock(body)
-    }
+    private static let _factory = FactoryBox(NOOPMetricsHandler.instance)
 
     /// `bootstrap` is an one-time configuration function which globally selects the desired metrics backend
     /// implementation. `bootstrap` can be called at maximum once in any given program, calling it more than once will
@@ -474,23 +464,53 @@ public enum MetricsSystem {
     /// - parameters:
     ///     - factory: A factory that given an identifier produces instances of metrics handlers such as `CounterHandler`, `RecorderHandler` and `TimerHandler`.
     public static func bootstrap(_ factory: MetricsFactory) {
-        self.lock.withWriterLock {
-            precondition(!self.initialized, "metrics system can only be initialized once per process. currently used factory: \(self._factory)")
-            self._factory = factory
-            self.initialized = true
-        }
+        self._factory.replaceFactory(factory, validate: true)
     }
 
     // for our testing we want to allow multiple bootstrapping
     internal static func bootstrapInternal(_ factory: MetricsFactory) {
-        self.lock.withWriterLock {
-            self._factory = factory
-        }
+        self._factory.replaceFactory(factory, validate: false)
     }
 
     /// Returns a reference to the configured factory.
     public static var factory: MetricsFactory {
-        return self.lock.withReaderLock { self._factory }
+        return self._factory.underlying
+    }
+
+    /// Acquire a writer lock for the duration of the given block.
+    ///
+    /// - Parameter body: The block to execute while holding the lock.
+    /// - Returns: The value returned by the block.
+    public static func withWriterLock<T>(_ body: () throws -> T) rethrows -> T {
+        return try self._factory.withWriterLock(body)
+    }
+
+    private final class FactoryBox {
+        private let lock = ReadWriteLock()
+        fileprivate var _underlying: MetricsFactory
+        private var initialized = false
+
+        init(_ underlying: MetricsFactory) {
+            self._underlying = underlying
+        }
+
+        func replaceFactory(_ factory: MetricsFactory, validate: Bool) {
+            self.lock.withWriterLock {
+                precondition(!validate || !self.initialized, "metrics system can only be initialized once per process. currently used factory: \(self._underlying)")
+                self._underlying = factory
+                self.initialized = true
+            }
+        }
+
+        var underlying: MetricsFactory {
+            return self.lock.withReaderLock {
+                return self._underlying
+            }
+        }
+
+        func withWriterLock<T>(_ body: () throws -> T) rethrows -> T {
+            return try self.lock.withWriterLock(body)
+        }
     }
 }