fix!: Add no-op behavior for initialize methods of logging system.

[sichanyoo]

Issue #

awslabs/aws-sdk-swift#903

Description of changes

Calling bootstrap method of swift-log's LoggingSystem more than once per program execution results in unexpected behavior (swift-log spec), most likely a crash.

To avoid this crash, no-op behavior is added to SDKLoggingSystem which wraps LoggingSystem. Whether the initialize methods have been already called or not is internally recorded to isInitialized variable. If yes, methods return with no-op done.

To make it thread-safe, SDKLoggingSystem is made an actor, and methods made async. This change is a breaking change.

Scope

• [] Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[jbelkins]

This approach is not thread-safe because this may still result in multiple calls to bootstrap if it is simultaneously called from different threads.

Two possible approaches to thread-safe initialization:

• Use NSRecursiveLock to enforce exclusive access to LoggingSystem and the isInitialized variable. You can find a recursive lock example in our codebase here:
  

  smithy-swift/Sources/ClientRuntime/Networking/Streaming/BufferedStream.swift

  Line 71 in f11e2e8

  private let lock = NSRecursiveLock()

• Use a Swift actor to enforce exclusive access to initialization. Look up Swift actors if you have not used them before.

The first approach is technically not async-safe since it blocks the calling thread, though the effect is minimal if the lock is released quickly. The second approach would require marking the initialize methods async which, depending on where they're called from, may be a breaking API change.

Look at either of these methods & let me know if you have further questions about how they work.

[dayaffe]

This approach is not thread-safe because this may still result in multiple calls to bootstrap if it is simultaneously called from different threads.

Two possible approaches to thread-safe initialization:

• Use NSRecursiveLock to enforce exclusive access to LoggingSystem and the isInitialized variable. You can find a recursive lock example in our codebase here:
  

  smithy-swift/Sources/ClientRuntime/Networking/Streaming/BufferedStream.swift

  Line 71 in f11e2e8

  private let lock = NSRecursiveLock()

• Use a Swift actor to enforce exclusive access to initialization. Look up Swift actors if you have not used them before.

The first approach is technically not async-safe since it blocks the calling thread, though the effect is minimal if the lock is released quickly. The second approach would require marking the initialize methods async which, depending on where they're called from, may be a breaking API change.

Look at either of these methods & let me know if you have further questions about how they work.

Super interesting! I just looked up these approaches and using actor seems like a good approach to me since it is part of Swift's concurrency features which we already utilize elsewhere (async/await).

Based on your latest commit seems like you agree @sichanyoo! From my limited understanding it looks like you want to declare the add and initialize with async to make it work with actor.

[jbelkins]

Approved, with a change of title to match Conventional Commits format.