Skip to content

Commit

Permalink
adopt sendable (#218)
Browse files Browse the repository at this point in the history
motivation: adjust to swift 5.6

changes:
* define sendable shims for protocols and structs that may be used in async context
* adjust tests
* add a test to make sure no warning are emitted
  • Loading branch information
tomerd authored Jul 1, 2022
1 parent 9f3731a commit f2e8667
Show file tree
Hide file tree
Showing 7 changed files with 103 additions and 21 deletions.
10 changes: 9 additions & 1 deletion Sources/Logging/LogHandler.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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: _SwiftLogSendableLogHandler {
/// 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.
Expand Down Expand Up @@ -186,3 +186,11 @@ extension LogHandler {
line: line)
}
}

// MARK: - Sendable support helpers

#if compiler(>=5.6)
@preconcurrency public protocol _SwiftLogSendableLogHandler: Sendable {}
#else
public protocol _SwiftLogSendableLogHandler {}
#endif
24 changes: 21 additions & 3 deletions Sources/Logging/Logging.swift
Original file line number Diff line number Diff line change
Expand Up @@ -721,8 +721,11 @@ extension Logger {
case string(String)

/// A metadata value which is some `CustomStringConvertible`.
#if compiler(>=5.6)
case stringConvertible(CustomStringConvertible & Sendable)
#else
case stringConvertible(CustomStringConvertible)

#endif
/// A metadata value which is a dictionary from `String` to `Logger.MetadataValue`.
///
/// Because `MetadataValue` implements `ExpressibleByDictionaryLiteral`, you don't need to type
Expand Down Expand Up @@ -1076,6 +1079,12 @@ 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 {
#if compiler(>=5.6)
internal typealias _SendableTextOutputStream = TextOutputStream & Sendable
#else
internal typealias _SendableTextOutputStream = TextOutputStream
#endif

/// 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)
Expand All @@ -1086,7 +1095,7 @@ public struct StreamLogHandler: LogHandler {
return StreamLogHandler(label: label, stream: StdioOutputStream.stderr)
}

private let stream: TextOutputStream
private let stream: _SendableTextOutputStream
private let label: String

public var logLevel: Logger.Level = .info
Expand All @@ -1108,7 +1117,7 @@ public struct StreamLogHandler: LogHandler {
}

// internal for testing only
internal init(label: String, stream: TextOutputStream) {
internal init(label: String, stream: _SendableTextOutputStream) {
self.label = label
self.stream = stream
}
Expand Down Expand Up @@ -1264,3 +1273,12 @@ extension Logger.MetadataValue: ExpressibleByArrayLiteral {
self = .array(elements)
}
}

// MARK: - Sendable support helpers

#if compiler(>=5.6)
extension Logger: Sendable {}
extension Logger.MetadataValue: Sendable {}
extension Logger.Level: Sendable {}
extension Logger.Message: Sendable {}
#endif
14 changes: 13 additions & 1 deletion Tests/LoggingTests/LoggingTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ 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 {
internal final class LazyMetadataBox: CustomStringConvertible {
private var makeValue: (() -> String)?
private var _value: String?

Expand Down Expand Up @@ -889,3 +889,15 @@ extension Logger {
}
#endif
}

// Sendable

#if compiler(>=5.6)
// used to test logging metadata which requires Sendable conformance
// @unchecked Sendable since manages it own state
extension LoggingTest.LazyMetadataBox: @unchecked Sendable {}

// used to test logging stream which requires Sendable conformance
// @unchecked Sendable since manages it own state
extension LoggingTest.InterceptStream: @unchecked Sendable {}
#endif
29 changes: 15 additions & 14 deletions Tests/LoggingTests/TestLogger.swift
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ internal struct TestLogging {
}

internal struct TestLogHandler: LogHandler {
private let logLevelLock = NSLock()
private let metadataLock = NSLock()
private let recorder: Recorder
private let config: Config
private var logger: Logger // the actual logger
Expand All @@ -56,10 +54,10 @@ internal struct TestLogHandler: LogHandler {
var logLevel: Logger.Level {
get {
// get from config unless set
return self.logLevelLock.withLock { self._logLevel } ?? self.config.get(key: self.label)
return self._logLevel ?? self.config.get(key: self.label)
}
set {
self.logLevelLock.withLock { self._logLevel = newValue }
self._logLevel = newValue
}
}

Expand All @@ -72,26 +70,20 @@ internal struct TestLogHandler: LogHandler {

public var metadata: Logger.Metadata {
get {
// return self.logger.metadata
return self.metadataLock.withLock { self._metadata }
return self._metadata
}
set {
// self.logger.metadata = newValue
self.metadataLock.withLock { self._metadata = newValue }
self._metadata = newValue
}
}

// TODO: would be nice to delegate to local copy of logger but StdoutLogger is a reference type. why?
subscript(metadataKey metadataKey: Logger.Metadata.Key) -> Logger.Metadata.Value? {
get {
// return self.logger[metadataKey: metadataKey]
return self.metadataLock.withLock { self._metadata[metadataKey] }
return self._metadata[metadataKey]
}
set {
// return logger[metadataKey: metadataKey] = newValue
self.metadataLock.withLock {
self._metadata[metadataKey] = newValue
}
self._metadata[metadataKey] = newValue
}
}
}
Expand Down Expand Up @@ -342,3 +334,12 @@ internal struct TestLibrary {
}
}
}

// Sendable

#if compiler(>=5.6)
extension TestLogHandler: @unchecked Sendable {}
extension Recorder: @unchecked Sendable {}
extension Config: @unchecked Sendable {}
extension MDC: @unchecked Sendable {}
#endif
43 changes: 43 additions & 0 deletions Tests/LoggingTests/TestSendable.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
//===----------------------------------------------------------------------===//
//
// This source file is part of the Swift Logging API open source project
//
// Copyright (c) 2022 Apple Inc. and the Swift Logging API project authors
// Licensed under Apache License v2.0
//
// See LICENSE.txt for license information
// See CONTRIBUTORS.txt for the list of Swift Logging API project authors
//
// SPDX-License-Identifier: Apache-2.0
//
//===----------------------------------------------------------------------===//
import Dispatch
@testable import Logging
import XCTest

class SendableTest: XCTestCase {
#if compiler(>=5.6)
func testSendableLogger() async {
let testLogging = TestLogging()
LoggingSystem.bootstrapInternal(testLogging.make)

let logger = Logger(label: "test")
let message1 = Logger.Message(stringLiteral: "critical 1")
let message2 = Logger.Message(stringLiteral: "critical 2")
let metadata: Logger.Metadata = ["key": "value"]

let task = Task.detached {
logger.info("info")
logger.critical(message1)
logger.critical(message2)
logger.warning(.init(stringLiteral: "warning"), metadata: metadata)
}

await task.value
testLogging.history.assertExist(level: .info, message: "info", metadata: [:])
testLogging.history.assertExist(level: .critical, message: "critical 1", metadata: [:])
testLogging.history.assertExist(level: .critical, message: "critical 2", metadata: [:])
testLogging.history.assertExist(level: .warning, message: "warning", metadata: metadata)
}
#endif
}
2 changes: 1 addition & 1 deletion docker/docker-compose.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,4 @@ services:

shell:
<<: *common
entrypoint: /bin/bash
entrypoint: /bin/bash -l
2 changes: 1 addition & 1 deletion scripts/soundness.sh
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ here="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"

function replace_acceptable_years() {
# this needs to replace all acceptable forms with 'YEARS'
sed -e 's/20[12][78901]-20[12][8901]/YEARS/' -e 's/20[12][8901]/YEARS/'
sed -e 's/20[12][789012]-20[12][89012]/YEARS/' -e 's/20[12][89012]/YEARS/'
}

printf "=> Checking linux tests... "
Expand Down

0 comments on commit f2e8667

Please sign in to comment.