From 1123bb3cb56d7454ce5e6ffc138b9351cbb37682 Mon Sep 17 00:00:00 2001 From: Toomas Vahter Date: Wed, 24 Sep 2025 11:07:21 +0300 Subject: [PATCH 1/4] Make Logger thread-safe --- Sources/StreamCore/Logger/Logger.swift | 198 ++++++++++++------ .../StreamCoreTests/Logger/Logger_Tests.swift | 147 +++++++++++++ 2 files changed, 285 insertions(+), 60 deletions(-) create mode 100644 Tests/StreamCoreTests/Logger/Logger_Tests.swift diff --git a/Sources/StreamCore/Logger/Logger.swift b/Sources/StreamCore/Logger/Logger.swift index 7be1094..5428bab 100644 --- a/Sources/StreamCore/Logger/Logger.swift +++ b/Sources/StreamCore/Logger/Logger.swift @@ -130,91 +130,157 @@ public struct LogSubsystem: OptionSet, CustomStringConvertible, Sendable { } public enum LogConfig { + struct Configuration { + var identifier: String = "" + var level: LogLevel = .error + var dateFormatter: DateFormatter = { + let df = DateFormatter() + df.dateFormat = "yyyy-MM-dd HH:mm:ss.SSS" + return df + }() + var formatters: [LogFormatter] = [] + var showDate: Bool = true + var showLevel: Bool = true + var showIdentifier: Bool = false + var showThreadName: Bool = true + var showFileName: Bool = true + var showLineNumber: Bool = true + var showFunctionName: Bool = true + var subsystems: LogSubsystem = .all + var destinationTypes: [LogDestination.Type] = LogConfig.defaultDestinations + } + + static let configuration = AllocatedUnfairLock(Configuration()) + /// Identifier for the logger. Defaults to empty. - public nonisolated(unsafe) static var identifier = "" { - didSet { + public static var identifier: String { + get { + configuration.withLock { $0.identifier } + } + set { + configuration.withLock { $0.identifier = newValue } invalidateLogger() } } /// Output level for the logger. - public nonisolated(unsafe) static var level: LogLevel = .error { - didSet { + public static var level: LogLevel { + get { + configuration.withLock { $0.level } + } + set { + configuration.withLock { $0.level = newValue } invalidateLogger() } } /// Date formatter for the logger. Defaults to ISO8601 - public nonisolated(unsafe) static var dateFormatter: DateFormatter = { - let df = DateFormatter() - df.dateFormat = "yyyy-MM-dd HH:mm:ss.SSS" - return df - }() { - didSet { + public static var dateFormatter: DateFormatter { + get { + configuration.withLock { $0.dateFormatter } + } + set { + configuration.withLock { $0.dateFormatter = newValue } invalidateLogger() } } /// Log formatters to be applied in order before logs are outputted. Defaults to empty (no formatters). /// Please see `LogFormatter` for more info. - public nonisolated(unsafe) static var formatters = [LogFormatter]() { - didSet { + public static var formatters: [LogFormatter] { + get { + configuration.withLock { $0.formatters } + } + set { + configuration.withLock { $0.formatters = newValue } invalidateLogger() } } /// Toggle for showing date in logs - public nonisolated(unsafe) static var showDate = true { - didSet { + public static var showDate: Bool { + get { + configuration.withLock { $0.showDate } + } + set { + configuration.withLock { $0.showDate = newValue } invalidateLogger() } } /// Toggle for showing log level in logs - public nonisolated(unsafe) static var showLevel = true { - didSet { + public static var showLevel: Bool { + get { + configuration.withLock { $0.showLevel } + } + set { + configuration.withLock { $0.showLevel = newValue } invalidateLogger() } } /// Toggle for showing identifier in logs - public nonisolated(unsafe) static var showIdentifier = false { - didSet { + public static var showIdentifier: Bool { + get { + configuration.withLock { $0.showIdentifier } + } + set { + configuration.withLock { $0.showIdentifier = newValue } invalidateLogger() } } /// Toggle for showing thread name in logs - public nonisolated(unsafe) static var showThreadName = true { - didSet { + public static var showThreadName: Bool { + get { + configuration.withLock { $0.showThreadName } + } + set { + configuration.withLock { $0.showThreadName = newValue } invalidateLogger() } } /// Toggle for showing file name in logs - public nonisolated(unsafe) static var showFileName = true { - didSet { + public static var showFileName: Bool { + get { + configuration.withLock { $0.showFileName } + } + set { + configuration.withLock { $0.showFileName = newValue } invalidateLogger() } } /// Toggle for showing line number in logs - public nonisolated(unsafe) static var showLineNumber = true { - didSet { + public static var showLineNumber: Bool { + get { + configuration.withLock { $0.showLineNumber } + } + set { + configuration.withLock { $0.showLineNumber = newValue } invalidateLogger() } } /// Toggle for showing function name in logs - public nonisolated(unsafe) static var showFunctionName = true { - didSet { + public static var showFunctionName: Bool { + get { + configuration.withLock { $0.showFunctionName } + } + set { + configuration.withLock { $0.showFunctionName = newValue } invalidateLogger() } } /// Subsystems for the logger - public nonisolated(unsafe) static var subsystems: LogSubsystem = .all { - didSet { + public static var subsystems: LogSubsystem { + get { + configuration.withLock { $0.subsystems } + } + set { + configuration.withLock { $0.subsystems = newValue } invalidateLogger() } } @@ -223,8 +289,12 @@ public enum LogConfig { /// /// Logger will initialize the destinations with its own parameters. If you want full control on the parameters, use `destinations` directly, /// where you can pass parameters to destination initializers yourself. - public nonisolated(unsafe) static var destinationTypes: [LogDestination.Type] = Self.defaultDestinations { - didSet { + public static var destinationTypes: [LogDestination.Type] { + get { + configuration.withLock { $0.destinationTypes } + } + set { + configuration.withLock { $0.destinationTypes = newValue } invalidateLogger() } } @@ -237,7 +307,7 @@ public enum LogConfig { } } - private nonisolated(unsafe) static var _destinations: [LogDestination]? + private static let _destinations = AllocatedUnfairLock<[LogDestination]?>(nil) /// Destinations for the default logger. Please see `LogDestination`. /// Defaults to only `ConsoleLogDestination`, which only prints the messages. @@ -245,58 +315,66 @@ public enum LogConfig { /// - Important: Other options in `ChatClientConfig.Logging` will not take affect if this is changed. public static var destinations: [LogDestination] { get { - if let destinations = _destinations { - return destinations - } else { - _destinations = destinationTypes.map { - $0.init( - identifier: identifier, - level: level, - subsystems: subsystems, - showDate: showDate, - dateFormatter: dateFormatter, - formatters: formatters, - showLevel: showLevel, - showIdentifier: showIdentifier, - showThreadName: showThreadName, - showFileName: showFileName, - showLineNumber: showLineNumber, - showFunctionName: showFunctionName - ) + _destinations.withLock { destinations in + if let destinations = destinations { + return destinations + } else { + let state = configuration.withLock { $0 } + let newDestinations = state.destinationTypes.map { + $0.init( + identifier: state.identifier, + level: state.level, + subsystems: state.subsystems, + showDate: state.showDate, + dateFormatter: state.dateFormatter, + formatters: state.formatters, + showLevel: state.showLevel, + showIdentifier: state.showIdentifier, + showThreadName: state.showThreadName, + showFileName: state.showFileName, + showLineNumber: state.showLineNumber, + showFunctionName: state.showFunctionName + ) + } + destinations = newDestinations + return newDestinations } - return _destinations! } } set { + _destinations.withLock { $0 = newValue } invalidateLogger() - _destinations = newValue } } /// Underlying logger instance to control singleton. - private nonisolated(unsafe) static var _logger: Logger? + private static let _logger = AllocatedUnfairLock(nil) /// Logger instance to be used by StreamChat. /// /// - Important: Other options in `LogConfig` will not take affect if this is changed. public static var logger: Logger { get { - if let logger = _logger { - return logger - } else { - _logger = Logger(identifier: identifier, destinations: destinations) - return _logger! + _logger.withLock { logger in + if let logger { + return logger + } else { + let state = configuration.withLock { $0 } + let newLogger = Logger(identifier: state.identifier, destinations: destinations) + logger = newLogger + return newLogger + } } } set { - _logger = newValue + _logger.withLock { $0 = newValue } } } /// Invalidates the current logger instance so it can be recreated. private static func invalidateLogger() { - _logger = nil - _destinations = nil + _logger.withLock { $0 = nil } + _destinations.withLock { $0 = nil } } } diff --git a/Tests/StreamCoreTests/Logger/Logger_Tests.swift b/Tests/StreamCoreTests/Logger/Logger_Tests.swift new file mode 100644 index 0000000..46f1375 --- /dev/null +++ b/Tests/StreamCoreTests/Logger/Logger_Tests.swift @@ -0,0 +1,147 @@ +// +// Logger_Tests.swift +// StreamCoreTests +// +// Created by Toomas Vahter on 24.09.2025. +// + +import Testing +@testable import StreamCore + +struct Logger_Tests { + + // MARK: - Concurrent Logger Invalidation Tests + + @Test func test_concurrentLoggerInvalidation() async throws { + let iterations = 50 + defer { + resetLogConfig() + } + await withTaskGroup(of: Void.self) { group in + for i in 0.. Date: Wed, 24 Sep 2025 11:23:59 +0300 Subject: [PATCH 2/4] Fix file header and add template --- .../xcshareddata/IDETemplateMacros.plist | 10 ++++++++++ Tests/StreamCoreTests/Logger/Logger_Tests.swift | 5 +---- 2 files changed, 11 insertions(+), 4 deletions(-) create mode 100644 StreamCore.xcodeproj/xcshareddata/IDETemplateMacros.plist diff --git a/StreamCore.xcodeproj/xcshareddata/IDETemplateMacros.plist b/StreamCore.xcodeproj/xcshareddata/IDETemplateMacros.plist new file mode 100644 index 0000000..b5bf641 --- /dev/null +++ b/StreamCore.xcodeproj/xcshareddata/IDETemplateMacros.plist @@ -0,0 +1,10 @@ + + + + + FILEHEADER + +// Copyright © ___YEAR___ Stream.io Inc. All rights reserved. +// + + diff --git a/Tests/StreamCoreTests/Logger/Logger_Tests.swift b/Tests/StreamCoreTests/Logger/Logger_Tests.swift index 46f1375..4ebb761 100644 --- a/Tests/StreamCoreTests/Logger/Logger_Tests.swift +++ b/Tests/StreamCoreTests/Logger/Logger_Tests.swift @@ -1,8 +1,5 @@ // -// Logger_Tests.swift -// StreamCoreTests -// -// Created by Toomas Vahter on 24.09.2025. +// Copyright © 2025 Stream.io Inc. All rights reserved. // import Testing From d58547859f47052085e084a8a68acb3f32af658a Mon Sep 17 00:00:00 2001 From: Toomas Vahter Date: Wed, 24 Sep 2025 11:58:00 +0300 Subject: [PATCH 3/4] Fix linter errors --- Sources/StreamCore/Logger/Logger.swift | 3 ++- Tests/StreamCoreTests/Logger/Logger_Tests.swift | 13 ++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Sources/StreamCore/Logger/Logger.swift b/Sources/StreamCore/Logger/Logger.swift index 5428bab..af3827b 100644 --- a/Sources/StreamCore/Logger/Logger.swift +++ b/Sources/StreamCore/Logger/Logger.swift @@ -138,6 +138,7 @@ public enum LogConfig { df.dateFormat = "yyyy-MM-dd HH:mm:ss.SSS" return df }() + var formatters: [LogFormatter] = [] var showDate: Bool = true var showLevel: Bool = true @@ -316,7 +317,7 @@ public enum LogConfig { public static var destinations: [LogDestination] { get { _destinations.withLock { destinations in - if let destinations = destinations { + if let destinations { return destinations } else { let state = configuration.withLock { $0 } diff --git a/Tests/StreamCoreTests/Logger/Logger_Tests.swift b/Tests/StreamCoreTests/Logger/Logger_Tests.swift index 4ebb761..57701f5 100644 --- a/Tests/StreamCoreTests/Logger/Logger_Tests.swift +++ b/Tests/StreamCoreTests/Logger/Logger_Tests.swift @@ -2,14 +2,13 @@ // Copyright © 2025 Stream.io Inc. All rights reserved. // -import Testing @testable import StreamCore +import Testing struct Logger_Tests { - // MARK: - Concurrent Logger Invalidation Tests - @Test func test_concurrentLoggerInvalidation() async throws { + @Test func concurrentLoggerInvalidation() async throws { let iterations = 50 defer { resetLogConfig() @@ -41,7 +40,7 @@ struct Logger_Tests { logger.info("Test message after concurrent invalidation") } - @Test func test_concurrentLoggerAccessDuringInvalidation() async throws { + @Test func concurrentLoggerAccessDuringInvalidation() async throws { let iterations = 100 defer { resetLogConfig() @@ -64,7 +63,7 @@ struct Logger_Tests { } } - @Test func test_concurrentLoggerInstanceReplacement() async throws { + @Test func concurrentLoggerInstanceReplacement() async throws { let iterations = 50 defer { resetLogConfig() @@ -84,7 +83,7 @@ struct Logger_Tests { logger.info("Test message after concurrent logger replacement") } - @Test func test_concurrentMixedOperations() async throws { + @Test func concurrentMixedOperations() async throws { let iterations = 40 defer { resetLogConfig() @@ -117,7 +116,7 @@ struct Logger_Tests { finalLogger.info("Final test message after mixed operations") } - @Test func test_loggerThreadSafetyHighContention() async throws { + @Test func loggerThreadSafetyHighContention() async throws { let iterations = 200 defer { resetLogConfig() From 09b2cdc2fa5ed77de890707c4fffa8fdfc435bf3 Mon Sep 17 00:00:00 2001 From: Toomas Vahter Date: Thu, 25 Sep 2025 14:30:47 +0300 Subject: [PATCH 4/4] Reduce to one lock --- Sources/StreamCore/Logger/Logger.swift | 232 ++++++++++-------- .../StreamCoreTests/Logger/Logger_Tests.swift | 2 +- 2 files changed, 136 insertions(+), 98 deletions(-) diff --git a/Sources/StreamCore/Logger/Logger.swift b/Sources/StreamCore/Logger/Logger.swift index af3827b..ca74a8d 100644 --- a/Sources/StreamCore/Logger/Logger.swift +++ b/Sources/StreamCore/Logger/Logger.swift @@ -130,7 +130,7 @@ public struct LogSubsystem: OptionSet, CustomStringConvertible, Sendable { } public enum LogConfig { - struct Configuration { + private struct State { var identifier: String = "" var level: LogLevel = .error var dateFormatter: DateFormatter = { @@ -148,41 +148,104 @@ public enum LogConfig { var showLineNumber: Bool = true var showFunctionName: Bool = true var subsystems: LogSubsystem = .all - var destinationTypes: [LogDestination.Type] = LogConfig.defaultDestinations + + var destinationTypes: [LogDestination.Type] = if #available(iOS 14.0, *) { + [OSLogDestination.self] + } else { + [ConsoleLogDestination.self] + } + + private var _destinations: [LogDestination]? + + var destinations: [LogDestination] { + mutating get { + if let _destinations { + return _destinations + } + let newDestinations = destinationTypes.map { + $0.init( + identifier: identifier, + level: level, + subsystems: subsystems, + showDate: showDate, + dateFormatter: dateFormatter, + formatters: formatters, + showLevel: showLevel, + showIdentifier: showIdentifier, + showThreadName: showThreadName, + showFileName: showFileName, + showLineNumber: showLineNumber, + showFunctionName: showFunctionName + ) + } + _destinations = newDestinations + return newDestinations + } + set { + _destinations = newValue + } + } + + private var _logger: Logger? + + var logger: Logger { + mutating get { + if let _logger { + return _logger + } + let logger = Logger(identifier: identifier, destinations: destinations) + _logger = logger + return logger + } + set { + _logger = newValue + } + } + + mutating func invalidateLogger() { + _destinations = nil + _logger = nil + } } - static let configuration = AllocatedUnfairLock(Configuration()) + private static let _state = AllocatedUnfairLock(State()) /// Identifier for the logger. Defaults to empty. public static var identifier: String { get { - configuration.withLock { $0.identifier } + _state.withLock { $0.identifier } } set { - configuration.withLock { $0.identifier = newValue } - invalidateLogger() + _state.withLock { + $0.identifier = newValue + $0.invalidateLogger() + } } } /// Output level for the logger. public static var level: LogLevel { get { - configuration.withLock { $0.level } + _state.withLock { $0.level } } set { - configuration.withLock { $0.level = newValue } - invalidateLogger() + _state.withLock { + $0.level = newValue + $0.invalidateLogger() + } } } /// Date formatter for the logger. Defaults to ISO8601 public static var dateFormatter: DateFormatter { get { - configuration.withLock { $0.dateFormatter } + _state.withLock { $0.dateFormatter } } set { - configuration.withLock { $0.dateFormatter = newValue } - invalidateLogger() + _state.withLock { + $0.dateFormatter = newValue + $0.invalidateLogger() + } } } @@ -190,99 +253,117 @@ public enum LogConfig { /// Please see `LogFormatter` for more info. public static var formatters: [LogFormatter] { get { - configuration.withLock { $0.formatters } + _state.withLock { $0.formatters } } set { - configuration.withLock { $0.formatters = newValue } - invalidateLogger() + _state.withLock { + $0.formatters = newValue + $0.invalidateLogger() + } } } /// Toggle for showing date in logs public static var showDate: Bool { get { - configuration.withLock { $0.showDate } + _state.withLock { $0.showDate } } set { - configuration.withLock { $0.showDate = newValue } - invalidateLogger() + _state.withLock { + $0.showDate = newValue + $0.invalidateLogger() + } } } /// Toggle for showing log level in logs public static var showLevel: Bool { get { - configuration.withLock { $0.showLevel } + _state.withLock { $0.showLevel } } set { - configuration.withLock { $0.showLevel = newValue } - invalidateLogger() + _state.withLock { + $0.showLevel = newValue + $0.invalidateLogger() + } } } /// Toggle for showing identifier in logs public static var showIdentifier: Bool { get { - configuration.withLock { $0.showIdentifier } + _state.withLock { $0.showIdentifier } } set { - configuration.withLock { $0.showIdentifier = newValue } - invalidateLogger() + _state.withLock { + $0.showIdentifier = newValue + $0.invalidateLogger() + } } } /// Toggle for showing thread name in logs public static var showThreadName: Bool { get { - configuration.withLock { $0.showThreadName } + _state.withLock { $0.showThreadName } } set { - configuration.withLock { $0.showThreadName = newValue } - invalidateLogger() + _state.withLock { + $0.showThreadName = newValue + $0.invalidateLogger() + } } } /// Toggle for showing file name in logs public static var showFileName: Bool { get { - configuration.withLock { $0.showFileName } + _state.withLock { $0.showFileName } } set { - configuration.withLock { $0.showFileName = newValue } - invalidateLogger() + _state.withLock { + $0.showFileName = newValue + $0.invalidateLogger() + } } } /// Toggle for showing line number in logs public static var showLineNumber: Bool { get { - configuration.withLock { $0.showLineNumber } + _state.withLock { $0.showLineNumber } } set { - configuration.withLock { $0.showLineNumber = newValue } - invalidateLogger() + _state.withLock { + $0.showLineNumber = newValue + $0.invalidateLogger() + } } } /// Toggle for showing function name in logs public static var showFunctionName: Bool { get { - configuration.withLock { $0.showFunctionName } + _state.withLock { $0.showFunctionName } } set { - configuration.withLock { $0.showFunctionName = newValue } - invalidateLogger() + _state.withLock { + $0.showFunctionName = newValue + $0.invalidateLogger() + } } } /// Subsystems for the logger public static var subsystems: LogSubsystem { get { - configuration.withLock { $0.subsystems } + _state.withLock { $0.subsystems } } set { - configuration.withLock { $0.subsystems = newValue } - invalidateLogger() + _state.withLock { + $0.subsystems = newValue + $0.invalidateLogger() + } } } @@ -292,23 +373,15 @@ public enum LogConfig { /// where you can pass parameters to destination initializers yourself. public static var destinationTypes: [LogDestination.Type] { get { - configuration.withLock { $0.destinationTypes } + _state.withLock { $0.destinationTypes } } set { - configuration.withLock { $0.destinationTypes = newValue } - invalidateLogger() - } - } - - static var defaultDestinations: [LogDestination.Type] { - if #available(iOS 14.0, *) { - [OSLogDestination.self] - } else { - [ConsoleLogDestination.self] + _state.withLock { + $0.destinationTypes = newValue + $0.invalidateLogger() + } } } - - private static let _destinations = AllocatedUnfairLock<[LogDestination]?>(nil) /// Destinations for the default logger. Please see `LogDestination`. /// Defaults to only `ConsoleLogDestination`, which only prints the messages. @@ -316,66 +389,31 @@ public enum LogConfig { /// - Important: Other options in `ChatClientConfig.Logging` will not take affect if this is changed. public static var destinations: [LogDestination] { get { - _destinations.withLock { destinations in - if let destinations { - return destinations - } else { - let state = configuration.withLock { $0 } - let newDestinations = state.destinationTypes.map { - $0.init( - identifier: state.identifier, - level: state.level, - subsystems: state.subsystems, - showDate: state.showDate, - dateFormatter: state.dateFormatter, - formatters: state.formatters, - showLevel: state.showLevel, - showIdentifier: state.showIdentifier, - showThreadName: state.showThreadName, - showFileName: state.showFileName, - showLineNumber: state.showLineNumber, - showFunctionName: state.showFunctionName - ) - } - destinations = newDestinations - return newDestinations - } - } + _state.withLock { $0.destinations } } set { - _destinations.withLock { $0 = newValue } - invalidateLogger() + _state.withLock { + // Order is important + $0.invalidateLogger() + $0.destinations = newValue + } } } - - /// Underlying logger instance to control singleton. - private static let _logger = AllocatedUnfairLock(nil) /// Logger instance to be used by StreamChat. /// /// - Important: Other options in `LogConfig` will not take affect if this is changed. public static var logger: Logger { get { - _logger.withLock { logger in - if let logger { - return logger - } else { - let state = configuration.withLock { $0 } - let newLogger = Logger(identifier: state.identifier, destinations: destinations) - logger = newLogger - return newLogger - } - } + _state.withLock { $0.logger } } set { - _logger.withLock { $0 = newValue } + _state.withLock { $0.logger = newValue } } } - /// Invalidates the current logger instance so it can be recreated. - private static func invalidateLogger() { - _logger.withLock { $0 = nil } - _destinations.withLock { $0 = nil } + static func reset() { + _state.withLock { $0 = State() } } } diff --git a/Tests/StreamCoreTests/Logger/Logger_Tests.swift b/Tests/StreamCoreTests/Logger/Logger_Tests.swift index 57701f5..3a63587 100644 --- a/Tests/StreamCoreTests/Logger/Logger_Tests.swift +++ b/Tests/StreamCoreTests/Logger/Logger_Tests.swift @@ -138,6 +138,6 @@ struct Logger_Tests { // MARK: - private func resetLogConfig() { - LogConfig.configuration.withLock { $0 = LogConfig.Configuration() } + LogConfig.reset() } }