-
Notifications
You must be signed in to change notification settings - Fork 24
Deadlock Reproduction Playground for <Swift 6 #267
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
baksha97
wants to merge
2
commits into
okta:master
Choose a base branch
from
baksha97:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+165
−0
Draft
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,97 @@ | ||
| # Bug: Deadlock in `SDKVersion.register` due to Main Thread Synchronization | ||
|
|
||
| ## Summary | ||
| A deadlock occurs when initializing `SDKVersion` or calling `SDKVersion.register` from a background thread. This often happens implicitly when initializing components like `OAuth2Client`. The method holds a private lock while synchronously intercepting the Main Thread to retrieve `UIDevice` information. If the Main Thread simultaneously attempts to access `SDKVersion` (e.g., reading `userAgent`), the application deadlocks. | ||
|
|
||
| ## Reproduction Steps | ||
|
|
||
| 1. **Background Initialization**: Initialize an instance of `OAuth2Client` (or other SDK components) from a background thread. In Swift < 6, unstructured `Task { }` blocks often run on a background executor. | ||
| 2. **Transitive Registration**: The `OAuth2Client` initializer implicitly accesses `SDKVersion.authFoundation`, which triggers static initialization and calls `SDKVersion.register`. | ||
| 3. **Lock Acquisition**: `register` acquires the private `SDKVersion.lock`. | ||
| 4. **Main Thread Block**: While holding the lock, the code attempts to read `UIDevice.current.systemVersion`. This forces a synchronous thread hop (`DispatchQueue.main.sync`) to the Main Thread. | ||
| 5. **Deadlock**: The background thread is now blocked waiting for the Main Thread. If the Main Thread is blocked or waiting on any resource (like the same `SDKVersion.lock`), the app deadlocks. | ||
|
|
||
| ### Reproduction Snippet | ||
|
|
||
| ```swift | ||
| // Background Thread initialization | ||
| // In Swift < 6, `Task { ... }` runs on the background generic executor. | ||
| // Initializing `OAuth2Client` (or other components) transitively calls `SDKVersion.register`. | ||
| // The deadlock occurs when initializing the `OAuth2Client` class itself. | ||
| ```swift | ||
| // Background Thread initialization | ||
| // In Swift < 6, `Task { ... }` runs on the background generic executor. | ||
| // Initializing `OAuth2Client` (or other components) transitively calls `SDKVersion.register`. | ||
| Task { | ||
| // Acquires lock during initialization, then performs dispatch_sync to Main Thread | ||
| let client = OAuth2Client(issuerURL: URL(string: "https://example.com")!, | ||
| clientId: "clientId", | ||
| scope: "openid") | ||
| } | ||
| ``` | ||
|
|
||
| ### Expected Output (Hang) | ||
| When the deadlock occurs, the console will show the following logs and then stop indefinitely: | ||
|
|
||
| ```text | ||
| 🏁 Starting Deadlock Reproduction | ||
| Background: Initializing OAuth2Client... (Acquiring Lock) | ||
| Main: Accessing SDKVersion.userAgent... (Requiring Lock) | ||
| ``` | ||
|
|
||
| **Note**: The success messages (`✅`) will never appear. | ||
|
|
||
| ### Call Chain Analysis | ||
| The initialization of `OAuth2Client` triggers the deadlock through the following chain: | ||
| 1. `OAuth2Client.init` call `assert(SDKVersion.authFoundation != nil)` | ||
| 2. Accessing `SDKVersion.authFoundation` triggers its static initialization. | ||
| 3. Static closure calls `SDKVersion.register(...)` | ||
| 4. `register` holds `SDKVersion.lock` while synchronously dispatching to the Main Thread for device info. | ||
|
|
||
|
|
||
| ## Affected Code | ||
|
|
||
| ### `SDKVersion.swift` | ||
| The `register` method holds a lock before accessing `systemVersion`, which triggers the main thread sync. | ||
|
|
||
| ```swift | ||
| // Sources/AuthFoundation/Migration/SDKVersion.swift | ||
|
|
||
| public static func register(sdk: SDKVersion) -> SDKVersion { | ||
| lock.withLock { // [!] Lock acquired here | ||
| // ... | ||
| // [!] Accessing systemVersion triggers MainActor.nonisolatedUnsafe | ||
| _userAgent = "\(sdkVersionString) \(systemName)/\(systemVersion) Device/\(deviceModel)" | ||
| return sdk | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| ### `ExpressionUtilities.swift` | ||
| The helper utility forces a synchronous wait on the Main Thread. | ||
|
|
||
| ```swift | ||
| // Sources/CommonSupport/ExpressionUtilities.swift | ||
|
|
||
| public static func nonisolatedUnsafe<T: Sendable>(_ block: @MainActor () -> T) -> T { | ||
| if Thread.isMainThread { | ||
| return MainActor.assumeIsolated { block() } | ||
| } else { | ||
| return DispatchQueue.main.sync { // [!] Synchronous wait for Main Thread | ||
| // ... | ||
| block() | ||
| // ... | ||
| } | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| ## Critique & Recommendation | ||
|
|
||
| **Issue**: Inversion of Control / Unsafe Threading Assumption. | ||
| The `SDKVersion.register` method is designed with a critical flaw: it holds a lock while synchronously waiting for the Main Thread (`DispatchQueue.main.sync`). This implementation implicitly assumes that registration will either occur on the Main Thread or when the Main Thread is free. | ||
|
|
||
| However, initializing core components like `OAuth2Client` on a background thread (a common pattern for performance) triggers this registration. This creates a **deadlock trap**: the background thread holds the lock and waits for Main, while any Main Thread operation needing that lock (such as fetching `User-Agent` for headers) will hang indefinitely. | ||
|
|
||
| **Fix**: | ||
| Decouple the `UIDevice` access from the lock scope. Fetch the system version independently or asynchronously before acquiring the lock, or cache it safely without blocking the critical section. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| import UIKit | ||
| import PlaygroundSupport | ||
| // Note: Ensure the 'AuthFoundation' target is built and available to this Playground. | ||
| import AuthFoundation | ||
|
|
||
| // Enable indefinite execution to allow async tasks to run | ||
| PlaygroundPage.current.needsIndefiniteExecution = true | ||
|
|
||
| print("🏁 Starting Deadlock Reproduction") | ||
|
|
||
| // 1. BACKGROUND THREAD | ||
| // Initialize OAuth2Client on a detached background task. | ||
| // This triggers: | ||
| // -> OAuth2Client.init | ||
| // -> SDKVersion.authFoundation (Lazy Static Init) | ||
| // -> SDKVersion.register (ACQUIRES LOCK) | ||
| // -> UIDevice.current.systemVersion (DispatchQueue.main.sync calls) | ||
| Task.detached { | ||
| print("Background: Initializing OAuth2Client... (Acquiring Lock)") | ||
|
|
||
| // This line triggers the registration lock | ||
| let client = OAuth2Client(issuerURL: URL(string: "https://example.com")!, | ||
| clientId: "repro-client", | ||
| scope: "openid") | ||
|
|
||
| print("Background: ✅ Initialization Complete! (Lock Released)") | ||
| } | ||
|
|
||
| // 2. MAIN THREAD | ||
|
|
||
| print("Main: Accessing SDKVersion.userAgent... (Requiring Lock)") | ||
|
|
||
| // If the bug exists, this line will hang indefinitely because: | ||
| // - Main Thread is stuck here waiting for SDKVersion.lock | ||
| // - Background Thread is holding SDKVersion.lock waiting for Main Thread (to get UIDevice info) | ||
| let agent = SDKVersion.userAgent | ||
|
|
||
| print("Main: ✅ UserAgent retrieved: \(agent)") | ||
| print("🎉 NO DEADLOCK DETECTED") | ||
|
|
||
| PlaygroundPage.current.finishExecution() | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Swift 5 enabled for this playground.