From a6b74ec1a378349b72b69c97d8e9dfa72418ab07 Mon Sep 17 00:00:00 2001 From: Joannis Orlandos Date: Fri, 4 Feb 2022 15:16:47 +0100 Subject: [PATCH] Enforce Sendable conformance for LogHandlers Swift-Log, and make Logger Sendable --- Package.swift | 6 ++++- Sources/Logging/LogHandler.swift | 4 ++-- Sources/Logging/Logging.swift | 16 ++++++++------ Tests/LoggingTests/LoggingTest.swift | 33 +++++++++++++++++----------- Tests/LoggingTests/TestLogger.swift | 6 ++--- 5 files changed, 39 insertions(+), 26 deletions(-) diff --git a/Package.swift b/Package.swift index efc6ef40..29adc1df 100644 --- a/Package.swift +++ b/Package.swift @@ -1,4 +1,4 @@ -// swift-tools-version:5.0 +// swift-tools-version:5.5 //===----------------------------------------------------------------------===// // // This source file is part of the Swift Logging API open source project @@ -17,6 +17,10 @@ import PackageDescription let package = Package( name: "swift-log", + platforms: [ + .iOS(.v13), + .macOS(.v10_15), + ], products: [ .library(name: "Logging", targets: ["Logging"]), ], diff --git a/Sources/Logging/LogHandler.swift b/Sources/Logging/LogHandler.swift index 6b52b120..bd1669ea 100644 --- a/Sources/Logging/LogHandler.swift +++ b/Sources/Logging/LogHandler.swift @@ -113,7 +113,7 @@ /// Please note that the above `LogHandler` will still pass the 'log level is a value' test above it iff the global log /// level has not been overridden. And most importantly it passes the requirement listed above: A change to the log /// level on one `Logger` should not affect the log level of another `Logger` variable. -public protocol LogHandler { +public protocol LogHandler: Sendable { /// This method is called when a `LogHandler` must emit a log message. There is no need for the `LogHandler` to /// check if the `level` is above or below the configured `logLevel` as `Logger` already performed this check and /// determined that a message should be logged. @@ -126,7 +126,7 @@ public protocol LogHandler { /// - file: The file the log message was emitted from. /// - function: The function the log line was emitted from. /// - line: The line the log message was emitted from. - func log(level: Logger.Level, + @Sendable func log(level: Logger.Level, message: Logger.Message, metadata: Logger.Metadata?, source: String, diff --git a/Sources/Logging/Logging.swift b/Sources/Logging/Logging.swift index 12a0e3ef..0a2259cf 100644 --- a/Sources/Logging/Logging.swift +++ b/Sources/Logging/Logging.swift @@ -35,7 +35,7 @@ import WASILibc /// /// logger.info("Hello World!") /// -public struct Logger { +public struct Logger: Sendable { @usableFromInline var handler: LogHandler @@ -707,7 +707,7 @@ extension Logger { /// over `..., metadata: ["colors": .array([.string("\(user.topColor)"), .string("\(user.secondColor)")])` /// - prefer `logger.info("nested info", metadata: ["nested": ["fave-numbers": ["\(1)", "\(2)", "\(3)"], "foo": "bar"]])` /// over `..., metadata: ["nested": .dictionary(["fave-numbers": ...])])` - public enum MetadataValue { + public enum MetadataValue: Sendable { /// A metadata value which is a `String`. /// /// Because `MetadataValue` implements `ExpressibleByStringInterpolation`, and `ExpressibleByStringLiteral`, @@ -715,7 +715,7 @@ extension Logger { case string(String) /// A metadata value which is some `CustomStringConvertible`. - case stringConvertible(CustomStringConvertible) + case stringConvertible(CustomStringConvertible & Sendable) /// A metadata value which is a dictionary from `String` to `Logger.MetadataValue`. /// @@ -734,7 +734,7 @@ extension Logger { /// /// Log levels are ordered by their severity, with `.trace` being the least severe and /// `.critical` being the most severe. - public enum Level: String, Codable, CaseIterable { + public enum Level: String, Codable, CaseIterable, Sendable { /// Appropriate for messages that contain information normally of use only when /// tracing the execution of a program. case trace @@ -999,7 +999,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, Sendable { internal let file: CFilePointer internal let flushMode: FlushMode @@ -1074,6 +1074,8 @@ let systemStdout = WASILibc.stdout! /// `StreamLogHandler` is a simple implementation of `LogHandler` for directing /// `Logger` output to either `stderr` or `stdout` via the factory methods. public struct StreamLogHandler: LogHandler { + public typealias Stream = TextOutputStream & Sendable + /// Factory that makes a `StreamLogHandler` to directs its output to `stdout` public static func standardOutput(label: String) -> StreamLogHandler { return StreamLogHandler(label: label, stream: StdioOutputStream.stdout) @@ -1084,7 +1086,7 @@ public struct StreamLogHandler: LogHandler { return StreamLogHandler(label: label, stream: StdioOutputStream.stderr) } - private let stream: TextOutputStream + private let stream: Stream private let label: String public var logLevel: Logger.Level = .info @@ -1106,7 +1108,7 @@ public struct StreamLogHandler: LogHandler { } // internal for testing only - internal init(label: String, stream: TextOutputStream) { + internal init(label: String, stream: Stream) { self.label = label self.stream = stream } diff --git a/Tests/LoggingTests/LoggingTest.swift b/Tests/LoggingTests/LoggingTest.swift index 3400c8ef..a7ce2051 100644 --- a/Tests/LoggingTests/LoggingTest.swift +++ b/Tests/LoggingTests/LoggingTest.swift @@ -262,24 +262,28 @@ class LoggingTest: XCTestCase { // Example of custom "box" which may be used to implement "render at most once" semantics // Not thread-safe, thus should not be shared across threads. - internal class LazyMetadataBox: CustomStringConvertible { - private var makeValue: (() -> String)? + internal class LazyMetadataBox: CustomStringConvertible, @unchecked Sendable { + typealias ValueMaker = @Sendable () -> String + let lock = NSLock() + private var makeValue: ValueMaker? private var _value: String? - public init(_ makeValue: @escaping () -> String) { + public init(_ makeValue: @escaping ValueMaker) { self.makeValue = makeValue } /// This allows caching a value in case it is accessed via an by name subscript, // rather than as part of rendering all metadata that a LoggingContext was carrying public var value: String { - if let f = self.makeValue { - self._value = f() - self.makeValue = nil - } + self.lock.withLock { + if let f = self.makeValue { + self._value = f() + self.makeValue = nil + } - assert(self._value != nil, "_value MUST NOT be nil once `lazyValue` has run.") - return self._value! + assert(self._value != nil, "_value MUST NOT be nil once `lazyValue` has run.") + return self._value! + } } public var description: String { @@ -672,14 +676,17 @@ class LoggingTest: XCTestCase { XCTAssertLessThan(Logger.Level.error, Logger.Level.critical) } - final class InterceptStream: TextOutputStream { + final class InterceptStream: TextOutputStream, @unchecked Sendable { + let lock = NSLock() var interceptedText: String? var strings = [String]() func write(_ string: String) { - // This is a test implementation, a real implementation would include locking - self.strings.append(string) - self.interceptedText = (self.interceptedText ?? "") + string + lock.withLock { + // This is a test implementation, a real implementation would include locking + self.strings.append(string) + self.interceptedText = (self.interceptedText ?? "") + string + } } } diff --git a/Tests/LoggingTests/TestLogger.swift b/Tests/LoggingTests/TestLogger.swift index 984649c1..c0b49135 100644 --- a/Tests/LoggingTests/TestLogger.swift +++ b/Tests/LoggingTests/TestLogger.swift @@ -30,7 +30,7 @@ internal struct TestLogging { var history: History { return self.recorder } } -internal struct TestLogHandler: LogHandler { +internal struct TestLogHandler: LogHandler, @unchecked Sendable { private let logLevelLock = NSLock() private let metadataLock = NSLock() private let recorder: Recorder @@ -96,7 +96,7 @@ internal struct TestLogHandler: LogHandler { } } -internal class Config { +internal class Config: @unchecked Sendable { private static let ALL = "*" private let lock = NSLock() @@ -122,7 +122,7 @@ internal class Config { } } -internal class Recorder: History { +internal class Recorder: History, @unchecked Sendable { private let lock = NSLock() private var _entries = [LogEntry]()