From 6553ef5893f873304a12d066cd420b02a77e768d Mon Sep 17 00:00:00 2001 From: Peter Adams <63288215+PeterAdams-A@users.noreply.github.com> Date: Tue, 18 Jun 2024 16:28:58 +0100 Subject: [PATCH] Add Sendability annotations (#308) Motivation: This becomes a critical feature in swift 6. Modifications: Add Sendability annotations where required. Factor out code protected by locks. Result: Compiles without warnings with Sendability checking --- Sources/Logging/Locks.swift | 10 +- Sources/Logging/Logging.swift | 173 +++++++++------ Tests/LoggingTests/CompatibilityTest.swift | 2 +- Tests/LoggingTests/GlobalLoggingTest.swift | 11 +- Tests/LoggingTests/LocalLoggingTest.swift | 9 +- Tests/LoggingTests/LoggingTest.swift | 202 +++++++++++------- Tests/LoggingTests/MDCTest.swift | 2 +- Tests/LoggingTests/MetadataProviderTest.swift | 2 +- Tests/LoggingTests/TestLogger.swift | 11 +- Tests/LoggingTests/TestSendable.swift | 3 +- docker/docker-compose.2004.58.yaml | 2 +- docker/docker-compose.2204.510.yaml | 2 +- docker/docker-compose.2204.59.yaml | 2 +- docker/docker-compose.2204.main.yaml | 5 +- docker/docker-compose.yaml | 2 +- 15 files changed, 268 insertions(+), 170 deletions(-) diff --git a/Sources/Logging/Locks.swift b/Sources/Logging/Locks.swift index 59a76552..a4d062ad 100644 --- a/Sources/Logging/Locks.swift +++ b/Sources/Logging/Locks.swift @@ -46,7 +46,7 @@ import Musl /// of lock is safe to use with `libpthread`-based threading models, such as the /// one used by NIO. On Windows, the lock is based on the substantially similar /// `SRWLOCK` type. -internal final class Lock { +internal final class Lock: @unchecked Sendable { #if canImport(WASILibc) // WASILibc is single threaded, provides no locks #elseif os(Windows) @@ -148,7 +148,7 @@ extension Lock { /// of lock is safe to use with `libpthread`-based threading models, such as the /// one used by NIO. On Windows, the lock is based on the substantially similar /// `SRWLOCK` type. -internal final class ReadWriteLock { +internal final class ReadWriteLock: @unchecked Sendable { #if canImport(WASILibc) // WASILibc is single threaded, provides no locks #elseif os(Windows) @@ -189,7 +189,7 @@ internal final class ReadWriteLock { /// /// Whenever possible, consider using `withReaderLock` instead of this /// method and `unlock`, to simplify lock handling. - public func lockRead() { + fileprivate func lockRead() { #if canImport(WASILibc) // WASILibc is single threaded, provides no locks #elseif os(Windows) @@ -205,7 +205,7 @@ internal final class ReadWriteLock { /// /// Whenever possible, consider using `withWriterLock` instead of this /// method and `unlock`, to simplify lock handling. - public func lockWrite() { + fileprivate func lockWrite() { #if canImport(WASILibc) // WASILibc is single threaded, provides no locks #elseif os(Windows) @@ -222,7 +222,7 @@ internal final class ReadWriteLock { /// Whenever possible, consider using `withReaderLock` and `withWriterLock` /// instead of this method and `lockRead` and `lockWrite`, to simplify lock /// handling. - public func unlock() { + fileprivate func unlock() { #if canImport(WASILibc) // WASILibc is single threaded, provides no locks #elseif os(Windows) diff --git a/Sources/Logging/Logging.swift b/Sources/Logging/Logging.swift index 7fb33de4..dce08948 100644 --- a/Sources/Logging/Logging.swift +++ b/Sources/Logging/Logging.swift @@ -17,7 +17,11 @@ import Darwin #elseif os(Windows) import CRT #elseif canImport(Glibc) +#if compiler(>=6.0) +@preconcurrency import Glibc +#else import Glibc +#endif #elseif canImport(Musl) import Musl #elseif canImport(WASILibc) @@ -498,11 +502,12 @@ extension Logger { /// configured. `LoggingSystem` is set up just once in a given program to set up the desired logging backend /// implementation. public enum LoggingSystem { - private static let _factory = FactoryBox { label, _ in StreamLogHandler.standardError(label: label) } - private static let _metadataProviderFactory = MetadataProviderBox(nil) + private static let _factory = FactoryBox({ label, _ in StreamLogHandler.standardError(label: label) }, + violationErrorMesage: "logging system can only be initialized once per process.") + private static let _metadataProviderFactory = MetadataProviderBox(nil, violationErrorMesage: "logging system can only be initialized once per process.") #if DEBUG - private static var _warnOnceBox: WarnOnceBox = WarnOnceBox() + private static let _warnOnceBox: WarnOnceBox = WarnOnceBox() #endif /// `bootstrap` is a one-time configuration function which globally selects the desired logging backend @@ -511,8 +516,9 @@ public enum LoggingSystem { /// /// - parameters: /// - factory: A closure that given a `Logger` identifier, produces an instance of the `LogHandler`. - public static func bootstrap(_ factory: @escaping (String) -> any LogHandler) { - self._factory.replaceFactory({ label, _ in + @preconcurrency + public static func bootstrap(_ factory: @escaping @Sendable(String) -> any LogHandler) { + self._factory.replace({ label, _ in factory(label) }, validate: true) } @@ -527,25 +533,26 @@ public enum LoggingSystem { /// - parameters: /// - metadataProvider: The `MetadataProvider` used to inject runtime-generated metadata from the execution context. /// - factory: A closure that given a `Logger` identifier, produces an instance of the `LogHandler`. - public static func bootstrap(_ factory: @escaping (String, Logger.MetadataProvider?) -> any LogHandler, + @preconcurrency + public static func bootstrap(_ factory: @escaping @Sendable(String, Logger.MetadataProvider?) -> any LogHandler, metadataProvider: Logger.MetadataProvider?) { - self._metadataProviderFactory.replaceMetadataProvider(metadataProvider, validate: true) - self._factory.replaceFactory(factory, validate: true) + self._metadataProviderFactory.replace(metadataProvider, validate: true) + self._factory.replace(factory, validate: true) } // for our testing we want to allow multiple bootstrapping - internal static func bootstrapInternal(_ factory: @escaping (String) -> any LogHandler) { - self._metadataProviderFactory.replaceMetadataProvider(nil, validate: false) - self._factory.replaceFactory({ label, _ in + internal static func bootstrapInternal(_ factory: @escaping @Sendable(String) -> any LogHandler) { + self._metadataProviderFactory.replace(nil, validate: false) + self._factory.replace({ label, _ in factory(label) }, validate: false) } // for our testing we want to allow multiple bootstrapping - internal static func bootstrapInternal(_ factory: @escaping (String, Logger.MetadataProvider?) -> any LogHandler, + internal static func bootstrapInternal(_ factory: @escaping @Sendable(String, Logger.MetadataProvider?) -> any LogHandler, metadataProvider: Logger.MetadataProvider?) { - self._metadataProviderFactory.replaceMetadataProvider(metadataProvider, validate: false) - self._factory.replaceFactory(factory, validate: false) + self._metadataProviderFactory.replace(metadataProvider, validate: false) + self._factory.replace(factory, validate: false) } fileprivate static var factory: (String, Logger.MetadataProvider?) -> any LogHandler { @@ -564,7 +571,7 @@ public enum LoggingSystem { /// factory to avoid using the bootstrapped metadata provider may sometimes be useful, usually it will lead to /// un-expected behavior, so make sure to always propagate it to your handlers. public static var metadataProvider: Logger.MetadataProvider? { - return self._metadataProviderFactory.metadataProvider + return self._metadataProviderFactory.underlying } #if DEBUG @@ -576,54 +583,71 @@ public enum LoggingSystem { } #endif - private final class FactoryBox { + /// Protects an object such that it can only be accessed through a Reader-Writer lock. + final class RWLockedValueBox: @unchecked Sendable { private let lock = ReadWriteLock() - fileprivate var _underlying: (_ label: String, _ provider: Logger.MetadataProvider?) -> any LogHandler - private var initialized = false + private var storage: Value - init(_ underlying: @escaping (String, Logger.MetadataProvider?) -> any LogHandler) { - self._underlying = underlying + init(initialValue: Value) { + self.storage = initialValue } - func replaceFactory(_ factory: @escaping (String, Logger.MetadataProvider?) -> any LogHandler, validate: Bool) { - self.lock.withWriterLock { - precondition(!validate || !self.initialized, "logging system can only be initialized once per process.") - self._underlying = factory - self.initialized = true + func withReadLock(_ operation: (Value) -> Result) -> Result { + self.lock.withReaderLock { + operation(self.storage) } } - var underlying: (String, Logger.MetadataProvider?) -> any LogHandler { - return self.lock.withReaderLock { - return self._underlying + func withWriteLock(_ operation: (inout Value) -> Result) -> Result { + self.lock.withWriterLock { + operation(&self.storage) } } } - private final class MetadataProviderBox { - private let lock = ReadWriteLock() - - internal var _underlying: Logger.MetadataProvider? - private var initialized = false - - init(_ underlying: Logger.MetadataProvider?) { - self._underlying = underlying - } + /// Protects an object applying the constraints that it can only be accessed through a Reader-Writer lock + /// and can ony bre updated once from the initial value given. + private struct ReplaceOnceBox { + private struct ReplaceOnce: Sendable { + private var initialized = false + private var _underlying: BoxedType + private let violationErrorMessage: String - func replaceMetadataProvider(_ metadataProvider: Logger.MetadataProvider?, validate: Bool) { - self.lock.withWriterLock { - precondition(!validate || !self.initialized, "logging system can only be initialized once per process.") - self._underlying = metadataProvider + mutating func replaceUnderlying(_ underlying: BoxedType, validate: Bool) { + precondition(!validate || !self.initialized, self.violationErrorMessage) + self._underlying = underlying self.initialized = true } - } - var metadataProvider: Logger.MetadataProvider? { - return self.lock.withReaderLock { + var underlying: BoxedType { return self._underlying } + + init(underlying: BoxedType, violationErrorMessage: String) { + self._underlying = underlying + self.violationErrorMessage = violationErrorMessage + } + } + + private let storage: RWLockedValueBox + + init(_ underlying: BoxedType, violationErrorMesage: String) { + self.storage = .init(initialValue: ReplaceOnce(underlying: underlying, + violationErrorMessage: violationErrorMesage)) + } + + func replace(_ newUnderlying: BoxedType, validate: Bool) { + self.storage.withWriteLock { $0.replaceUnderlying(newUnderlying, validate: validate) } + } + + var underlying: BoxedType { + self.storage.withReadLock { $0.underlying } } } + + private typealias FactoryBox = ReplaceOnceBox< @Sendable(_ label: String, _ provider: Logger.MetadataProvider?) -> any LogHandler> + + private typealias MetadataProviderBox = ReplaceOnceBox } extension Logger { @@ -1021,7 +1045,7 @@ internal typealias CFilePointer = UnsafeMutablePointer /// A wrapper to facilitate `print`-ing to stderr and stdio that /// ensures access to the underlying `FILE` is locked to prevent /// cross-thread interleaving of output. -internal struct StdioOutputStream: TextOutputStream { +internal struct StdioOutputStream: TextOutputStream, @unchecked Sendable { internal let file: CFilePointer internal let flushMode: FlushMode @@ -1062,8 +1086,41 @@ internal struct StdioOutputStream: TextOutputStream { return contiguousString.utf8 } - internal static let stderr = StdioOutputStream(file: systemStderr, flushMode: .always) - internal static let stdout = StdioOutputStream(file: systemStdout, flushMode: .always) + internal static let stderr = { + // Prevent name clashes + #if canImport(Darwin) + let systemStderr = Darwin.stderr + #elseif os(Windows) + let systemStderr = CRT.stderr + #elseif canImport(Glibc) + let systemStderr = Glibc.stderr! + #elseif canImport(Musl) + let systemStderr = Musl.stderr! + #elseif canImport(WASILibc) + let systemStderr = WASILibc.stderr! + #else + #error("Unsupported runtime") + #endif + return StdioOutputStream(file: systemStderr, flushMode: .always) + }() + + internal static let stdout = { + // Prevent name clashes + #if canImport(Darwin) + let systemStdout = Darwin.stdout + #elseif os(Windows) + let systemStdout = CRT.stdout + #elseif canImport(Glibc) + let systemStdout = Glibc.stdout! + #elseif canImport(Musl) + let systemStdout = Musl.stdout! + #elseif canImport(WASILibc) + let systemStdout = WASILibc.stdout! + #else + #error("Unsupported runtime") + #endif + return StdioOutputStream(file: systemStdout, flushMode: .always) + }() /// Defines the flushing strategy for the underlying stream. internal enum FlushMode { @@ -1072,26 +1129,6 @@ internal struct StdioOutputStream: TextOutputStream { } } -// Prevent name clashes -#if canImport(Darwin) -let systemStderr = Darwin.stderr -let systemStdout = Darwin.stdout -#elseif os(Windows) -let systemStderr = CRT.stderr -let systemStdout = CRT.stdout -#elseif canImport(Glibc) -let systemStderr = Glibc.stderr! -let systemStdout = Glibc.stdout! -#elseif canImport(Musl) -let systemStderr = Musl.stderr! -let systemStdout = Musl.stdout! -#elseif canImport(WASILibc) -let systemStderr = WASILibc.stderr! -let systemStdout = WASILibc.stdout! -#else -#error("Unsupported runtime") -#endif - /// `StreamLogHandler` is a simple implementation of `LogHandler` for directing /// `Logger` output to either `stderr` or `stdout` via the factory methods. /// @@ -1341,7 +1378,7 @@ extension Logger.MetadataValue: ExpressibleByArrayLiteral { #if DEBUG /// Contains state to manage all kinds of "warn only once" warnings which the logging system may want to issue. -private final class WarnOnceBox { +private final class WarnOnceBox: @unchecked Sendable { private let lock: Lock = Lock() private var warnOnceLogHandlerNotSupportedMetadataProviderPerType = Set() diff --git a/Tests/LoggingTests/CompatibilityTest.swift b/Tests/LoggingTests/CompatibilityTest.swift index 63e0c7b6..6ee4fb70 100644 --- a/Tests/LoggingTests/CompatibilityTest.swift +++ b/Tests/LoggingTests/CompatibilityTest.swift @@ -19,7 +19,7 @@ final class CompatibilityTest: XCTestCase { func testAllLogLevelsWorkWithOldSchoolLogHandlerWorks() { let testLogging = OldSchoolTestLogging() - var logger = Logger(label: "\(#function)", factory: testLogging.make) + var logger = Logger(label: "\(#function)", factory: { testLogging.make(label: $0) }) logger.logLevel = .trace logger.trace("yes: trace") diff --git a/Tests/LoggingTests/GlobalLoggingTest.swift b/Tests/LoggingTests/GlobalLoggingTest.swift index a1ffb8ea..72a12d54 100644 --- a/Tests/LoggingTests/GlobalLoggingTest.swift +++ b/Tests/LoggingTests/GlobalLoggingTest.swift @@ -13,12 +13,17 @@ //===----------------------------------------------------------------------===// @testable import Logging import XCTest +#if compiler(>=6.0) || canImport(Darwin) +import Dispatch +#else +@preconcurrency import Dispatch +#endif class GlobalLoggerTest: XCTestCase { func test1() throws { // bootstrap with our test logging impl let logging = TestLogging() - LoggingSystem.bootstrapInternal(logging.make) + LoggingSystem.bootstrapInternal { logging.make(label: $0) } // change test logging config to log traces and above logging.config.set(value: Logger.Level.debug) @@ -45,7 +50,7 @@ class GlobalLoggerTest: XCTestCase { func test2() throws { // bootstrap with our test logging impl let logging = TestLogging() - LoggingSystem.bootstrapInternal(logging.make) + LoggingSystem.bootstrapInternal { logging.make(label: $0) } // change test logging config to log errors and above logging.config.set(value: Logger.Level.error) @@ -72,7 +77,7 @@ class GlobalLoggerTest: XCTestCase { func test3() throws { // bootstrap with our test logging impl let logging = TestLogging() - LoggingSystem.bootstrapInternal(logging.make) + LoggingSystem.bootstrapInternal { logging.make(label: $0) } // change test logging config logging.config.set(value: .warning) diff --git a/Tests/LoggingTests/LocalLoggingTest.swift b/Tests/LoggingTests/LocalLoggingTest.swift index 2f15e22e..87cb37f7 100644 --- a/Tests/LoggingTests/LocalLoggingTest.swift +++ b/Tests/LoggingTests/LocalLoggingTest.swift @@ -13,12 +13,17 @@ //===----------------------------------------------------------------------===// @testable import Logging import XCTest +#if compiler(>=6.0) || canImport(Darwin) +import Dispatch +#else +@preconcurrency import Dispatch +#endif class LocalLoggerTest: XCTestCase { func test1() throws { // bootstrap with our test logging impl let logging = TestLogging() - LoggingSystem.bootstrapInternal(logging.make) + LoggingSystem.bootstrapInternal { logging.make(label: $0) } // change test logging config to log traces and above logging.config.set(value: Logger.Level.debug) @@ -46,7 +51,7 @@ class LocalLoggerTest: XCTestCase { func test2() throws { // bootstrap with our test logging impl let logging = TestLogging() - LoggingSystem.bootstrapInternal(logging.make) + LoggingSystem.bootstrapInternal { logging.make(label: $0) } // change test logging config to log errors and above logging.config.set(value: Logger.Level.error) diff --git a/Tests/LoggingTests/LoggingTest.swift b/Tests/LoggingTests/LoggingTest.swift index e2a92ab7..48860972 100644 --- a/Tests/LoggingTests/LoggingTest.swift +++ b/Tests/LoggingTests/LoggingTest.swift @@ -22,11 +22,25 @@ import WinSDK import Glibc #endif +private extension LogHandler { + func with(logLevel: Logger.Level) -> any LogHandler { + var result = self + result.logLevel = logLevel + return result + } + + func withMetadata(_ key: String, _ value: Logger.MetadataValue) -> any LogHandler { + var result = self + result.metadata[key] = value + return result + } +} + class LoggingTest: XCTestCase { func testAutoclosure() throws { // bootstrap with our test logging impl let logging = TestLogging() - LoggingSystem.bootstrapInternal(logging.make) + LoggingSystem.bootstrapInternal { logging.make(label: $0) } var logger = Logger(label: "test") logger.logLevel = .info @@ -80,11 +94,8 @@ class LoggingTest: XCTestCase { let logging1 = TestLogging() let logging2 = TestLogging() - var logger1 = logging1.make(label: "1") - logger1.logLevel = .info - - var logger2 = logging2.make(label: "2") - logger2.logLevel = .debug + let logger1 = logging1.make(label: "1").with(logLevel: .info) + let logger2 = logging2.make(label: "2").with(logLevel: .debug) LoggingSystem.bootstrapInternal { _ in MultiplexLogHandler([logger1, logger2]) @@ -111,11 +122,8 @@ class LoggingTest: XCTestCase { let logging1 = TestLogging() let logging2 = TestLogging() - var logger1 = logging1.make(label: "1") - logger1.logLevel = .info - - var logger2 = logging2.make(label: "2") - logger2.logLevel = .info + let logger1 = logging1.make(label: "1").with(logLevel: .info) + let logger2 = logging2.make(label: "2").with(logLevel: .info) LoggingSystem.bootstrapInternal { _ in MultiplexLogHandler([logger1, logger2]) @@ -147,12 +155,12 @@ class LoggingTest: XCTestCase { let logging1 = TestLogging() let logging2 = TestLogging() - var logger1 = logging1.make(label: "1") - logger1.metadata["one"] = "111" - logger1.metadata["in"] = "in-1" - var logger2 = logging2.make(label: "2") - logger2.metadata["two"] = "222" - logger2.metadata["in"] = "in-2" + let logger1 = logging1.make(label: "1") + .withMetadata("one", "111") + .withMetadata("in", "in-1") + let logger2 = logging2.make(label: "2") + .withMetadata("two", "222") + .withMetadata("in", "in-2") LoggingSystem.bootstrapInternal { _ in MultiplexLogHandler([logger1, logger2]) @@ -204,12 +212,12 @@ class LoggingTest: XCTestCase { let logging1 = TestLogging() let logging2 = TestLogging() - var logger1 = logging1.make(label: "1") - logger1.metadata["one"] = "111" - logger1.metadata["in"] = "in-1" - var logger2 = logging2.make(label: "2") - logger2.metadata["two"] = "222" - logger2.metadata["in"] = "in-2" + let logger1 = logging1.make(label: "1") + .withMetadata("one", "111") + .withMetadata("in", "in-1") + let logger2 = logging2.make(label: "2") + .withMetadata("two", "222") + .withMetadata("in", "in-2") LoggingSystem.bootstrapInternal { _ in MultiplexLogHandler([logger1, logger2]) @@ -228,20 +236,26 @@ class LoggingTest: XCTestCase { let logging1 = TestLogging() let logging2 = TestLogging() - var handler1 = logging1.make(label: "1") - handler1.metadata["one"] = "111" - handler1.metadata["in"] = "in-1" - handler1.metadataProvider = .constant([ - "provider-1": "provided-111", - "provider-overlap": "provided-111", - ]) - var handler2 = logging2.make(label: "2") - handler2.metadata["two"] = "222" - handler2.metadata["in"] = "in-2" - handler2.metadataProvider = .constant([ - "provider-2": "provided-222", - "provider-overlap": "provided-222", - ]) + let handler1 = { + var handler1 = logging1.make(label: "1") + handler1.metadata["one"] = "111" + handler1.metadata["in"] = "in-1" + handler1.metadataProvider = .constant([ + "provider-1": "provided-111", + "provider-overlap": "provided-111", + ]) + return handler1 + }() + let handler2 = { + var handler2 = logging2.make(label: "2") + handler2.metadata["two"] = "222" + handler2.metadata["in"] = "in-2" + handler2.metadataProvider = .constant([ + "provider-2": "provided-222", + "provider-overlap": "provided-222", + ]) + return handler2 + }() LoggingSystem.bootstrapInternal { _ in MultiplexLogHandler([handler1, handler2]) @@ -268,18 +282,24 @@ class LoggingTest: XCTestCase { let logging1 = TestLogging() let logging2 = TestLogging() - var handler1 = logging1.make(label: "1") - handler1.metadataProvider = .constant([ - "provider-1": "provided-111", - "provider-overlap": "provided-111", - ]) - var handler2 = logging2.make(label: "2") - handler2.metadata["two"] = "222" - handler2.metadata["in"] = "in-2" - handler2.metadataProvider = .constant([ - "provider-2": "provided-222", - "provider-overlap": "provided-222", - ]) + let handler1 = { + var handler1 = logging1.make(label: "1") + handler1.metadataProvider = .constant([ + "provider-1": "provided-111", + "provider-overlap": "provided-111", + ]) + return handler1 + }() + let handler2 = { + var handler2 = logging2.make(label: "2") + handler2.metadata["two"] = "222" + handler2.metadata["in"] = "in-2" + handler2.metadataProvider = .constant([ + "provider-2": "provided-222", + "provider-overlap": "provided-222", + ]) + return handler2 + }() LoggingSystem.bootstrapInternal({ _, metadataProvider in MultiplexLogHandler( @@ -307,7 +327,7 @@ class LoggingTest: XCTestCase { func testDictionaryMetadata() { let testLogging = TestLogging() - LoggingSystem.bootstrapInternal(testLogging.make) + LoggingSystem.bootstrapInternal { testLogging.make(label: $0) } var logger = Logger(label: "\(#function)") logger[metadataKey: "foo"] = ["bar": "buz"] @@ -323,7 +343,7 @@ class LoggingTest: XCTestCase { func testListMetadata() { let testLogging = TestLogging() - LoggingSystem.bootstrapInternal(testLogging.make) + LoggingSystem.bootstrapInternal { testLogging.make(label: $0) } var logger = Logger(label: "\(#function)") logger[metadataKey: "foo"] = ["bar", "buz"] @@ -366,7 +386,7 @@ class LoggingTest: XCTestCase { func testStringConvertibleMetadata() { let testLogging = TestLogging() - LoggingSystem.bootstrapInternal(testLogging.make) + LoggingSystem.bootstrapInternal { testLogging.make(label: $0) } var logger = Logger(label: "\(#function)") logger[metadataKey: "foo"] = .stringConvertible("raw-string") @@ -386,7 +406,7 @@ class LoggingTest: XCTestCase { func testAutoClosuresAreNotForcedUnlessNeeded() { let testLogging = TestLogging() - LoggingSystem.bootstrapInternal(testLogging.make) + LoggingSystem.bootstrapInternal { testLogging.make(label: $0) } var logger = Logger(label: "\(#function)") logger.logLevel = .error @@ -400,7 +420,7 @@ class LoggingTest: XCTestCase { func testLocalMetadata() { let testLogging = TestLogging() - LoggingSystem.bootstrapInternal(testLogging.make) + LoggingSystem.bootstrapInternal { testLogging.make(label: $0) } var logger = Logger(label: "\(#function)") logger.info("hello world!", metadata: ["foo": "bar"]) @@ -441,7 +461,7 @@ class LoggingTest: XCTestCase { func testAllLogLevelsExceptCriticalCanBeBlocked() { let testLogging = TestLogging() - LoggingSystem.bootstrapInternal(testLogging.make) + LoggingSystem.bootstrapInternal { testLogging.make(label: $0) } var logger = Logger(label: "\(#function)") logger.logLevel = .critical @@ -465,7 +485,7 @@ class LoggingTest: XCTestCase { func testAllLogLevelsWork() { let testLogging = TestLogging() - LoggingSystem.bootstrapInternal(testLogging.make) + LoggingSystem.bootstrapInternal { testLogging.make(label: $0) } var logger = Logger(label: "\(#function)") logger.logLevel = .trace @@ -489,7 +509,7 @@ class LoggingTest: XCTestCase { func testAllLogLevelByFunctionRefWithSource() { let testLogging = TestLogging() - LoggingSystem.bootstrapInternal(testLogging.make) + LoggingSystem.bootstrapInternal { testLogging.make(label: $0) } var logger = Logger(label: "\(#function)") logger.logLevel = .trace @@ -521,7 +541,7 @@ class LoggingTest: XCTestCase { func testAllLogLevelByFunctionRefWithoutSource() { let testLogging = TestLogging() - LoggingSystem.bootstrapInternal(testLogging.make) + LoggingSystem.bootstrapInternal { testLogging.make(label: $0) } var logger = Logger(label: "\(#function)") logger.logLevel = .trace @@ -553,7 +573,7 @@ class LoggingTest: XCTestCase { func testLogsEmittedFromSubdirectoryGetCorrectModuleInNewerSwifts() { let testLogging = TestLogging() - LoggingSystem.bootstrapInternal(testLogging.make) + LoggingSystem.bootstrapInternal { testLogging.make(label: $0) } var logger = Logger(label: "\(#function)") logger.logLevel = .trace @@ -573,7 +593,7 @@ class LoggingTest: XCTestCase { func testLogMessageWithStringInterpolation() { let testLogging = TestLogging() - LoggingSystem.bootstrapInternal(testLogging.make) + LoggingSystem.bootstrapInternal { testLogging.make(label: $0) } var logger = Logger(label: "\(#function)") logger.logLevel = .debug @@ -586,7 +606,7 @@ class LoggingTest: XCTestCase { func testLoggingAString() { let testLogging = TestLogging() - LoggingSystem.bootstrapInternal(testLogging.make) + LoggingSystem.bootstrapInternal { testLogging.make(label: $0) } var logger = Logger(label: "\(#function)") logger.logLevel = .debug @@ -619,9 +639,8 @@ class LoggingTest: XCTestCase { func testLoggerWithoutFactoryOverrideDefaultsToUsingLoggingSystemMetadataProvider() { let logging = TestLogging() - LoggingSystem.bootstrapInternal({ label, metadataProvider in - logging.makeWithMetadataProvider(label: label, metadataProvider: metadataProvider) - }, metadataProvider: .init { ["provider": "42"] }) + LoggingSystem.bootstrapInternal({ logging.makeWithMetadataProvider(label: $0, metadataProvider: $1) }, + metadataProvider: .init { ["provider": "42"] }) let logger = Logger(label: #function) @@ -635,7 +654,7 @@ class LoggingTest: XCTestCase { func testLoggerWithPredefinedLibraryMetadataProvider() { let logging = TestLogging() LoggingSystem.bootstrapInternal( - logging.makeWithMetadataProvider, + { logging.makeWithMetadataProvider(label: $0, metadataProvider: $1) }, metadataProvider: .exampleMetadataProvider ) @@ -650,7 +669,8 @@ class LoggingTest: XCTestCase { func testLoggerWithFactoryOverrideDefaultsToUsingLoggingSystemMetadataProvider() { let logging = TestLogging() - LoggingSystem.bootstrapInternal(logging.makeWithMetadataProvider, metadataProvider: .init { ["provider": "42"] }) + LoggingSystem.bootstrapInternal({ logging.makeWithMetadataProvider(label: $0, metadataProvider: $1) }, + metadataProvider: .init { ["provider": "42"] }) let logger = Logger(label: #function, factory: { label in logging.makeWithMetadataProvider(label: label, metadataProvider: LoggingSystem.metadataProvider) @@ -686,11 +706,37 @@ class LoggingTest: XCTestCase { logger1.error("hey") } + /// Protects an object such that it can only be accessed while holding a lock. + private final class LockedValueBox: @unchecked Sendable { + private let lock = Lock() + private var storage: Value + + init(initialValue: Value) { + self.storage = initialValue + } + + func withLock(_ operation: (Value) -> Result) -> Result { + self.lock.withLock { + operation(self.storage) + } + } + + func withLockMutating(_ operation: (inout Value) -> Void) { + self.lock.withLockVoid { + operation(&self.storage) + } + } + + var underlying: Value { + get { self.withLock { $0 } } + set { self.withLockMutating { $0 = newValue } } + } + } + func testLoggerWithGlobalOverride() { struct LogHandlerWithGlobalLogLevelOverride: LogHandler { // the static properties hold the globally overridden log level (if overridden) - private static let overrideLock = Lock() - private static var overrideLogLevel: Logger.Level? + private static let overrideLogLevel = LockedValueBox(initialValue: nil) private let recorder: Recorder // this holds the log level if not overridden @@ -706,9 +752,7 @@ class LoggingTest: XCTestCase { var logLevel: Logger.Level { // when we get asked for the log level, we check if it was globally overridden or not get { - return LogHandlerWithGlobalLogLevelOverride.overrideLock.withLock { - LogHandlerWithGlobalLogLevelOverride.overrideLogLevel - } ?? self._logLevel + return LogHandlerWithGlobalLogLevelOverride.overrideLogLevel.underlying ?? self._logLevel } // we set the log level whenever we're asked (note: this might not have an effect if globally // overridden) @@ -733,9 +777,7 @@ class LoggingTest: XCTestCase { // this is the function to globally override the log level, it is not part of the `LogHandler` protocol static func overrideGlobalLogLevel(_ logLevel: Logger.Level) { - LogHandlerWithGlobalLogLevelOverride.overrideLock.withLock { - LogHandlerWithGlobalLogLevelOverride.overrideLogLevel = logLevel - } + LogHandlerWithGlobalLogLevelOverride.overrideLogLevel.underlying = logLevel } } @@ -1007,7 +1049,7 @@ class LoggingTest: XCTestCase { } // bootstrap with our test logging impl let logging = TestLogging() - LoggingSystem.bootstrapInternal(logging.make) + LoggingSystem.bootstrapInternal { logging.make(label: $0) } var logger = Logger(label: "test") logger.logLevel = .error @@ -1024,8 +1066,8 @@ class LoggingTest: XCTestCase { } // default usage - LoggingSystem.bootstrap(StreamLogHandler.standardOutput) - LoggingSystem.bootstrap(StreamLogHandler.standardError) + LoggingSystem.bootstrap { (label: String) in StreamLogHandler.standardOutput(label: label) } + LoggingSystem.bootstrap { (label: String) in StreamLogHandler.standardError(label: label) } // with metadata handler, explicitly, public api LoggingSystem.bootstrap({ label, metadataProvider in @@ -1036,8 +1078,10 @@ class LoggingTest: XCTestCase { }, metadataProvider: .exampleMetadataProvider) // with metadata handler, still pretty - LoggingSystem.bootstrap(StreamLogHandler.standardOutput, metadataProvider: .exampleMetadataProvider) - LoggingSystem.bootstrap(StreamLogHandler.standardError, metadataProvider: .exampleMetadataProvider) + LoggingSystem.bootstrap({ (label: String, metadataProvider: Logger.MetadataProvider?) in StreamLogHandler.standardOutput(label: label, metadataProvider: metadataProvider) }, + metadataProvider: .exampleMetadataProvider) + LoggingSystem.bootstrap({ (label: String, metadataProvider: Logger.MetadataProvider?) in StreamLogHandler.standardError(label: label, metadataProvider: metadataProvider) }, + metadataProvider: .exampleMetadataProvider) } func testLoggerIsJustHoldingASinglePointer() { diff --git a/Tests/LoggingTests/MDCTest.swift b/Tests/LoggingTests/MDCTest.swift index c5cef474..029cd6aa 100644 --- a/Tests/LoggingTests/MDCTest.swift +++ b/Tests/LoggingTests/MDCTest.swift @@ -19,7 +19,7 @@ class MDCTest: XCTestCase { func test1() throws { // bootstrap with our test logger let logging = TestLogging() - LoggingSystem.bootstrapInternal(logging.make) + LoggingSystem.bootstrapInternal { logging.make(label: $0) } // run the program MDC.global["foo"] = "bar" diff --git a/Tests/LoggingTests/MetadataProviderTest.swift b/Tests/LoggingTests/MetadataProviderTest.swift index 22542531..0730816d 100644 --- a/Tests/LoggingTests/MetadataProviderTest.swift +++ b/Tests/LoggingTests/MetadataProviderTest.swift @@ -25,7 +25,7 @@ import Glibc final class MetadataProviderTest: XCTestCase { func testLoggingMergesOneOffMetadataWithProvidedMetadataFromExplicitlyPassed() throws { let logging = TestLogging() - LoggingSystem.bootstrapInternal(logging.makeWithMetadataProvider, + LoggingSystem.bootstrapInternal({ logging.makeWithMetadataProvider(label: $0, metadataProvider: $1) }, metadataProvider: .init { ["common": "initial"] }) diff --git a/Tests/LoggingTests/TestLogger.swift b/Tests/LoggingTests/TestLogger.swift index cf64f845..c324c41d 100644 --- a/Tests/LoggingTests/TestLogger.swift +++ b/Tests/LoggingTests/TestLogger.swift @@ -17,6 +17,11 @@ import XCTest #if os(Windows) import WinSDK #endif +#if compiler(>=6.0) || canImport(Darwin) +import Dispatch +#else +@preconcurrency import Dispatch +#endif internal struct TestLogging { private let _config = Config() // shared among loggers @@ -276,7 +281,7 @@ public class MDC { private let lock = NSLock() private var storage = [Int: Logger.Metadata]() - public static var global = MDC() + public static let global = MDC() private init() {} @@ -352,7 +357,7 @@ internal extension NSLock { } } -internal struct TestLibrary { +internal struct TestLibrary: Sendable { private let logger = Logger(label: "TestLibrary") private let queue = DispatchQueue(label: "TestLibrary") @@ -362,7 +367,7 @@ internal struct TestLibrary { self.logger.info("TestLibrary::doSomething") } - public func doSomethingAsync(completion: @escaping () -> Void) { + public func doSomethingAsync(completion: @escaping @Sendable() -> Void) { // libraries that use global loggers and async, need to make sure they propagate the // logging metadata when creating a new thread let metadata = MDC.global.metadata diff --git a/Tests/LoggingTests/TestSendable.swift b/Tests/LoggingTests/TestSendable.swift index b33a9cc6..37da940b 100644 --- a/Tests/LoggingTests/TestSendable.swift +++ b/Tests/LoggingTests/TestSendable.swift @@ -11,14 +11,13 @@ // SPDX-License-Identifier: Apache-2.0 // //===----------------------------------------------------------------------===// -import Dispatch @testable import Logging import XCTest class SendableTest: XCTestCase { func testSendableLogger() async { let testLogging = TestLogging() - LoggingSystem.bootstrapInternal(testLogging.make) + LoggingSystem.bootstrapInternal { testLogging.make(label: $0) } let logger = Logger(label: "test") let message1 = Logger.Message(stringLiteral: "critical 1") diff --git a/docker/docker-compose.2004.58.yaml b/docker/docker-compose.2004.58.yaml index 9c13dbc2..e0c78d32 100644 --- a/docker/docker-compose.2004.58.yaml +++ b/docker/docker-compose.2004.58.yaml @@ -12,7 +12,7 @@ services: test: image: swift-log:20.04-5.8 environment: - - FORCE_TEST_DISCOVERY=--enable-test-discovery + - WARN_AS_ERROR_ARG=-Xswiftc -warnings-as-errors #- SANITIZER_ARG=--sanitize=thread shell: diff --git a/docker/docker-compose.2204.510.yaml b/docker/docker-compose.2204.510.yaml index 5e54ee8d..7985c44d 100644 --- a/docker/docker-compose.2204.510.yaml +++ b/docker/docker-compose.2204.510.yaml @@ -11,7 +11,7 @@ services: test: image: swift-log:22.04-5.10 environment: - - FORCE_TEST_DISCOVERY=--enable-test-discovery + - WARN_AS_ERROR_ARG=-Xswiftc -warnings-as-errors #- SANITIZER_ARG=--sanitize=thread shell: diff --git a/docker/docker-compose.2204.59.yaml b/docker/docker-compose.2204.59.yaml index da84b6b0..c174f47f 100644 --- a/docker/docker-compose.2204.59.yaml +++ b/docker/docker-compose.2204.59.yaml @@ -12,7 +12,7 @@ services: test: image: swift-log:22.04-5.9 environment: - - FORCE_TEST_DISCOVERY=--enable-test-discovery + - WARN_AS_ERROR_ARG=-Xswiftc -warnings-as-errors #- SANITIZER_ARG=--sanitize=thread shell: diff --git a/docker/docker-compose.2204.main.yaml b/docker/docker-compose.2204.main.yaml index f428ad28..46109331 100644 --- a/docker/docker-compose.2204.main.yaml +++ b/docker/docker-compose.2204.main.yaml @@ -11,8 +11,11 @@ services: test: image: swift-log:22.04-main environment: - - FORCE_TEST_DISCOVERY=--enable-test-discovery + - WARN_AS_ERROR_ARG=-Xswiftc -warnings-as-errors #- SANITIZER_ARG=--sanitize=thread + - IMPORT_CHECK_ARG=--explicit-target-dependency-import-check error + - EXPLICIT_SENDABLE_ARG=-Xswiftc -require-explicit-sendable + - STRICT_CONCURRENCY_ARG=-Xswiftc -strict-concurrency=complete shell: image: swift-log:22.04-main diff --git a/docker/docker-compose.yaml b/docker/docker-compose.yaml index c20a46b8..8974de91 100644 --- a/docker/docker-compose.yaml +++ b/docker/docker-compose.yaml @@ -28,7 +28,7 @@ services: test: <<: *common - command: /bin/bash -xcl "swift test -Xswiftc -warnings-as-errors --enable-test-discovery $${SANITIZER_ARG-}" + command: /bin/bash -xcl "swift test $${WARN_AS_ERROR_ARG-} $${SANITIZER_ARG-} $${IMPORT_CHECK_ARG-} $${EXPLICIT_SENDABLE_ARG-} $${STRICT_CONCURRENCY_ARG-}" # util