withTimeLimit isolation extraction
#9
Conversation
WalkthroughThe changes introduce updates to concurrency utilities and their tests, specify a new Swift language version, and adjust a GitHub Actions workflow script. The Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test Function
participant CustomActor as CustomActor
participant TaskTimeLimit as TaskTimeLimit Utility
Test->>CustomActor: Start Task on @CustomActor
CustomActor->>TaskTimeLimit: Call withDeadline/withTimeout (operation: assertIsolated)
TaskTimeLimit->>CustomActor: Execute operation with actor isolation
CustomActor-->>TaskTimeLimit: Operation result
TaskTimeLimit-->>CustomActor: Return result
CustomActor-->>Test: Task completes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
Tests/PrincipleConcurrencyTests/CustomActor.swift (1)
1-7: Check file creation dateThe file header indicates a creation date of April 24, 2025, which is in the future. Consider updating to the correct current date.
-// Created by Kamil Strzelecki on 24/04/2025. -// Copyright © 2025 Kamil Strzelecki. All rights reserved. +// Created by Kamil Strzelecki on 24/04/2024. +// Copyright © 2024 Kamil Strzelecki. All rights reserved.Sources/PrincipleConcurrency/TaskTimeLimit.swift (1)
92-105: Well-documented workaround for Swift isolation limitationsThe
unpackOperationfunction addresses current limitations in Swift's isolation model. The comments with links to Swift evolution proposals and forum discussions are particularly helpful for understanding the rationale.However, there's an unused parameter that could be removed:
private func unpackOperation<Success: Sendable, R>( _ operation: sending @escaping TaskTimeLimit<Success>.Operation, - callerIsolation _: isolated (any Actor)?, transform: (sending @escaping TaskTimeLimit<Success>.Operation, isolated(any Actor)?) -> sending R ) async -> sending R {Also note that line 59 with
_ = operationIsolationappears to be a no-op but is likely necessary due to current Swift isolation limitations as explained in the comments.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/pull-request.yml(1 hunks).swift-version(1 hunks)Sources/PrincipleConcurrency/TaskTimeLimit.swift(4 hunks)Tests/PrincipleConcurrencyTests/CustomActor.swift(1 hunks)Tests/PrincipleConcurrencyTests/TaskTimeLimitTests.swift(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
Tests/PrincipleConcurrencyTests/TaskTimeLimitTests.swift (2)
Sources/PrincipleConcurrency/DeadlineExceededError.swift (2)
withDeadline(30-47)withDeadline(63-78)Sources/PrincipleConcurrency/TimeoutError.swift (2)
withTimeout(30-47)withTimeout(63-78)
Sources/PrincipleConcurrency/TaskTimeLimit.swift (1)
Sources/PrincipleConcurrency/SingleUseTransfer.swift (1)
finalize(79-87)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-and-test (visionos)
🔇 Additional comments (12)
.swift-version (1)
1-1: Swift version updated to 6.1This update aligns with the concurrent enhancements in the codebase, specifically the actor isolation improvements being implemented in this PR.
.github/workflows/pull-request.yml (2)
73-75: Reordering of case branches looks goodThe iOS platform case was moved after maccatalyst, which doesn't affect the functionality since each case is handled independently.
82-84: Reordering of case branches looks goodThe visionOS platform case was moved after watchOS, which doesn't affect the functionality since each case is handled independently.
Tests/PrincipleConcurrencyTests/CustomActor.swift (1)
9-13: LGTM: Well-defined global actor for testingThe global actor definition looks good. It provides a simple way to test actor isolation in the time-limited operations.
Since this is used for testing actor isolation, you might want to add an
assertIsolated()method to validate isolation. Something like:func assertIsolated() { // Empty method used to verify actor isolation }Tests/PrincipleConcurrencyTests/TaskTimeLimitTests.swift (2)
58-66: LGTM: Good test for actor isolation with deadlineThis test correctly verifies that actor isolation is preserved when using
withDeadline. The test creates a task with@CustomActorisolation and verifies that the actor context is correctly propagated within the time-limited operation.
113-121: LGTM: Good test for actor isolation with timeoutThis test correctly verifies that actor isolation is preserved when using
withTimeout. The test creates a task with@CustomActorisolation and verifies that the actor context is correctly propagated within the time-limited operation.Sources/PrincipleConcurrency/TaskTimeLimit.swift (6)
12-12: Generic enum enhances type safety and code reuseThe change from a non-generic private enum to a generic internal enum parameterized by
Success: Sendableimproves type safety and allows for better code reuse across the module.
14-15: Good use of type alias for improved readabilityThe new
Operationtype alias clearly defines the operation signature with explicit isolation annotation, improving code readability and maintainability.
16-16: Appropriate visibility level for Event enumMaking the
Eventenum fileprivate and non-generic is a good choice as it relies on the outer generic parameter while restricting access to just this file.
30-31: Improved parameter naming and type safetyRenaming the isolation parameter to
callerIsolationis more descriptive, and using the newTaskTimeLimit<Success>.Operationtype with thesendingkeyword improves type safety and aligns with Swift's ownership model.
36-39: Consistent generic type parameter and parameter namingThe changes to use
TaskTimeLimit<Success>.Event.selfandisolation: callerIsolationmaintain consistency with the other changes in the file.
49-69:Details
✅ Verification successful
Complex but necessary isolation handling
The new approach to adding the operation task to the group is more complex but addresses actor isolation concerns more effectively. The pattern of creating an unsafe mutable reference with a proper defer block for restoration is appropriate in this context.
However, the code could be vulnerable to isolation violations if the Swift concurrency model changes in the future.
🏁 Script executed:
#!/bin/bash # Check if there are any other uses of nonisolated(unsafe) in the codebase # that might indicate established patterns rg "nonisolated\(unsafe\)" --type swiftLength of output: 332
Nonisolated(unsafe) usage is intentional and correctly scoped
I ran a search for other
nonisolated(unsafe)uses and found only this instance in TaskTimeLimit.swift (the only other mention is a commented example in SingleUseTransfer.swift). Given its singular, well-scoped application here—and the properdeferto restore actor isolation—the pattern is both necessary and correctly implemented. No further changes are required at this time.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Sources/PrincipleConcurrency/TaskTimeLimit.swift (1)
94-103: Consider documentation for unpackOperation helper.The
unpackOperationfunction implements a critical piece of the isolation extraction logic. Consider adding a documentation comment explaining its purpose and behavior beyond just providing links to external resources.private func unpackOperation<Success: Sendable, R>( _ operation: sending @escaping TaskTimeLimit<Success>.Operation, callerIsolation _: isolated (any Actor)?, transform: (sending @escaping TaskTimeLimit<Success>.Operation, isolated(any Actor)?) -> sending R ) async -> sending R { + /// Extracts the actor isolation from the operation closure and passes it along with the operation + /// to the transform closure. This allows preserving the original isolation context when + /// executing the operation in a different task. // https://github.com/swiftlang/swift-evolution/blob/main/proposals/0461-async-function-isolation.md // https://forums.swift.org/t/closure-isolation-control/70378 let operationIsolation = extractIsolation(operation) return await transform(operation, operationIsolation) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Sources/PrincipleConcurrency/TaskTimeLimit.swift(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
Sources/PrincipleConcurrency/TaskTimeLimit.swift (1)
Sources/PrincipleConcurrency/SingleUseTransfer.swift (1)
finalize(79-87)
🔇 Additional comments (4)
Sources/PrincipleConcurrency/TaskTimeLimit.swift (4)
12-15: Well-structured generic implementation.The change to make
TaskTimeLimitgeneric withSuccess: Sendableimproves type safety and makes the API more flexible. TheOperationtypealias clearly defines the closure requirements.
16-21: Good encapsulation for Event enum.Making the
Eventenum fileprivate and non-generic is appropriate since it relies on the outer generic parameter and only needs to be accessible within this file.
30-31: Improved parameter naming and type safety.Renaming
isolationtocallerIsolationmakes the purpose clearer. Using the defined typealias for the operation parameter provides better type safety and readability.
49-71:Details
✅ Verification successful
Verify the isolation handling approach.
The approach using
nonisolated(unsafe)with a deferred restore is complex but appears necessary for the isolation requirements. The comments about Swift's isolation behavior are helpful for understanding the implementation.
🏁 Script executed:
#!/bin/bash # Check if there are any similar patterns in the codebase rg -U "nonisolated\(unsafe\) var.*\ndefer \{ .* = consume .* \}" --type swiftLength of output: 79
Isolation handling approach approved
The
nonisolated(unsafe) var unsafeGroup = groupwithdefer { group = consume unsafeGroup }pattern is unique to this context (no other matches found in the codebase) and appropriately addresses Swift’s actor isolation rules. The inline comment and forum link clearly explain why this complexity is necessary.• File: Sources/PrincipleConcurrency/TaskTimeLimit.swift
• Lines: 49–71No further changes required.
| let operationIsolation = extractIsolation(operation) | ||
| return await transform(operation, operationIsolation) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify extractIsolation function implementation.
The code relies on an extractIsolation function that's not shown in this file. Ensure this function correctly extracts the actor isolation from the operation closure.
🏁 Script executed:
#!/bin/bash
# Look for the extractIsolation function implementation
rg -A 5 "func extractIsolation" --type swiftLength of output: 46
Let’s search the entire repo for any definition or reference of extractIsolation:
🏁 Script executed:
#!/bin/bash
# Search for any occurrences of extractIsolation in the codebase
rg -n "extractIsolation"Length of output: 134
Implement or Import extractIsolation
It looks like extractIsolation(_:) is called on line 101 of Sources/PrincipleConcurrency/TaskTimeLimit.swift but no implementation or import exists in the repo. You’ll need to:
• Add the missing func extractIsolation<T>(_ operation: @escaping () -> T) -> ActorIsolation (or appropriate signature)
• Or import the module/extension that provides it
Without this definition, the actor‐isolation logic in TaskTimeLimit.swift can’t compile or function correctly. Please include or reference the correct implementation.
| // NOTE: #isolation != operationIsolation | ||
| // https://forums.swift.org/t/explicitly-captured-isolated-parameter-does-not-change-isolation-of-sendable-sending-closures/79502 | ||
| _ = operationIsolation |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ensure isolation comment remains current.
The comment references a Swift forums discussion about isolation behavior. This is informative but Swift's concurrency model is evolving rapidly.
🌐 Web query:
What are the current limitations of Swift actor isolation with captured parameters?
💡 Result:
Current Limitations of Swift Actor Isolation with Captured Parameters
Swift’s actor isolation model, while robust, comes with certain limitations—particularly regarding how parameters, especially captured values like closures or references to self, interact with an actor’s isolation boundary. The following summarizes the main limitations as they stand in Swift 6.
Default Isolation and Capturing self
- By default, all instance methods and properties of an actor are actor-isolated, meaning they can only be accessed from within the actor's own context or via an
awaitboundary[1][3]. - When capturing parameters such as
selfin closures that escape the current actor context, Swift enforces strong restrictions. If the captured parameter is non-Sendable, this is flagged by the compiler because it risks being accessed from multiple isolation contexts, which could cause data races[6][8][9]. - You cannot refer to actor-isolated state (e.g., properties or methods) in a closure unless that closure is guaranteed to run on the same actor. Attempting to do so without proper isolation leads to compile-time errors:
error: actor-isolated property 'balance' cannot be referenced on non-isolated parameter 'self'[1][3][9].
Nonisolated Keyword and Sendability
- You can mark actor properties or methods as
nonisolated, which allows them to be accessed outside actor isolation. However, such methods can only refer to other non-isolated data; accessing actor-isolated state from a non-isolated context is not allowed[1][3]. - Any value passed across actor boundaries (including captured in closures or as parameters) must conform to
Sendable, or else you get a compile-time error[1][6][8]. - Returning a non-Sendable type from a
nonisolatedfunction is forbidden, as non-isolated declarations can be called from any concurrency domain[1].
Limitations with Isolated Parameters
- While Swift 6 introduces more flexible actor isolation with isolated parameters, there are syntactic and practical constraints:
- The pattern for inheriting actor isolation via isolated parameters is verbose and can't be consistently used everywhere, particularly in accessors or other contexts where you can't add parameters[5].
- When you pass a closure that captures an actor-isolated value (e.g.,
selfor a property), you must ensure that the closure only runs within the correct isolation context, otherwise the compiler warns that later accesses could race[9]. - The feature for inheriting caller’s actor isolation is under development, and current workarounds (like passing an
isolated (any Actor)?parameter) have performance or ergonomic downsides[4][5][7].
Inout Parameters
- Passing actor-isolated stored properties as
inoutparameters to synchronous functions is allowed within the actor, but is prohibited for asynchronous functions, as this could cause exclusivity violations if the async call suspends and other tasks try to access the same property[2].
Summary Table: Key Limitations
| Scenario | Limitation |
|---|---|
Capturing actor-isolated self |
Not allowed across isolation boundaries unless Sendable and handled correctly |
| Nonisolated methods/properties | Can only access non-isolated data; cannot reference actor-isolated state |
| Non-Sendable types in nonisolated context | Forbidden |
| Passing closures across isolation | Closure must not capture non-Sendable or actor-isolated values unless properly isolated |
| Inheriting actor isolation | Syntax is currently verbose and not universally applicable; more ergonomic solutions proposed |
| Inout parameters in async calls | Cannot pass actor-isolated properties as inout to async functions |
These constraints are all designed to prevent data races and ensure thread safety, but they do introduce complexity and sometimes ergonomic hurdles for developers dealing with actor isolation and captured parameters in Swift[1][2][3][5][6][9].
Citations:
- 1: https://github.com/apple/swift-evolution/blob/main/proposals/0313-actor-isolation-control.md
- 2: https://forums.swift.org/t/concurrency-actors-actor-isolation/41613
- 3: https://www.avanderlee.com/swift/nonisolated-isolated/
- 4: https://forums.swift.org/t/pitch-inheriting-the-callers-actor-isolation/68391
- 5: https://github.com/apple/swift-evolution/blob/main/proposals/0420-inheritance-of-actor-isolation.md
- 6: https://developer.apple.com/forums/thread/763665
- 7: https://forums.swift.org/t/isolated-any-function-types/70562
- 8: https://www.youtube.com/watch?v=vsPX4s8HqUs
- 9: https://forums.swift.org/t/unable-to-send-closure-with-an-isolated-parameter/73772
- 10: https://alexdremov.me/swift-actors-common-problems-and-tips/
Update isolation comment to reflect current Swift 6 semantics
The existing note points to an older forum discussion but doesn’t explain why the workaround is still necessary in Swift 6. Please update the inline comment for clarity and to reference the latest evolution proposal.
- File:
Sources/PrincipleConcurrency/TaskTimeLimit.swift
Lines: 59–61 - Replace the existing note with an updated explanation and link to SE-420.
Suggested diff:
- // NOTE: #isolation != operationIsolation
- // https://forums.swift.org/t/explicitly-captured-isolated-parameter-does-not-change-isolation-of-sendable-sending-closures/79502
- _ = operationIsolation
+ // As of Swift 6, capturing an `isolated` parameter does *not* propagate actor isolation into sendable closures.
+ // An explicit `ActorIsolation` token is required to maintain the actor’s isolation context.
+ // See SE-420 “Inheritance of Actor Isolation” for upcoming ergonomic improvements:
+ // https://github.com/apple/swift-evolution/blob/main/proposals/0420-inheritance-of-actor-isolation.md
+ _ = operationIsolation📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // NOTE: #isolation != operationIsolation | |
| // https://forums.swift.org/t/explicitly-captured-isolated-parameter-does-not-change-isolation-of-sendable-sending-closures/79502 | |
| _ = operationIsolation | |
| // As of Swift 6, capturing an `isolated` parameter does *not* propagate actor isolation into sendable closures. | |
| // An explicit `ActorIsolation` token is required to maintain the actor’s isolation context. | |
| // See SE-420 “Inheritance of Actor Isolation” for upcoming ergonomic improvements: | |
| // https://github.com/apple/swift-evolution/blob/main/proposals/0420-inheritance-of-actor-isolation.md | |
| _ = operationIsolation |
Summary by CodeRabbit
New Features
Improvements
Chores