-
Notifications
You must be signed in to change notification settings - Fork 314
Implement request cancellation #860
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
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
3d17cad
Implement request cancellation
ahoppen 71dfd48
Remove `CancellationToken`
ahoppen b41f6af
Avoid a potential race condition in which a sourcekitd/clangd request…
ahoppen f7572c4
Factor `withCancellableCheckedThrowingContinuation` into a separate f…
ahoppen f29c97f
Update the document after cancelling the completion request to unbloc…
ahoppen 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
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
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
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
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
This file was deleted.
Oops, something went wrong.
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
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
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,64 @@ | ||
//===----------------------------------------------------------------------===// | ||
// | ||
// This source file is part of the Swift.org open source project | ||
// | ||
// Copyright (c) 2014 - 2023 Apple Inc. and the Swift project authors | ||
// Licensed under Apache License v2.0 with Runtime Library Exception | ||
// | ||
// See https://swift.org/LICENSE.txt for license information | ||
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors | ||
// | ||
//===----------------------------------------------------------------------===// | ||
|
||
public extension Task { | ||
/// Awaits the value of the result. | ||
/// | ||
/// If the current task is cancelled, this will cancel the subtask as well. | ||
var valuePropagatingCancellation: Success { | ||
get async throws { | ||
try await withTaskCancellationHandler { | ||
return try await self.value | ||
} onCancel: { | ||
self.cancel() | ||
} | ||
} | ||
} | ||
} | ||
|
||
/// Allows the execution of a cancellable operation that returns the results | ||
/// via a completion handler. | ||
/// | ||
/// `operation` must invoke the continuation's `resume` method exactly once. | ||
/// | ||
/// If the task executing `withCancellableCheckedThrowingContinuation` gets | ||
/// cancelled, `cancel` is invoked with the handle that `operation` provided. | ||
public func withCancellableCheckedThrowingContinuation<Handle, Result>( | ||
_ operation: (_ continuation: CheckedContinuation<Result, any Error>) -> Handle, | ||
cancel: (Handle) -> Void | ||
) async throws -> Result { | ||
let handleWrapper = ThreadSafeBox<Handle?>(initialValue: nil) | ||
|
||
@Sendable | ||
func callCancel() { | ||
/// Take the request ID out of the box. This ensures that we only send the | ||
/// cancel notification once in case the `Task.isCancelled` and the | ||
/// `onCancel` check race. | ||
if let handle = handleWrapper.takeValue() { | ||
cancel(handle) | ||
} | ||
} | ||
|
||
return try await withTaskCancellationHandler(operation: { | ||
try Task.checkCancellation() | ||
return try await withCheckedThrowingContinuation { continuation in | ||
handleWrapper.value = operation(continuation) | ||
|
||
// Check if the task was cancelled. This ensures we send a | ||
// CancelNotification even if the task gets cancelled after we register | ||
// the cancellation handler but before we set the `requestID`. | ||
if Task.isCancelled { | ||
callCancel() | ||
} | ||
} | ||
}, onCancel: callCancel) | ||
} |
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
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,57 @@ | ||
//===----------------------------------------------------------------------===// | ||
// | ||
// This source file is part of the Swift.org open source project | ||
// | ||
// Copyright (c) 2014 - 2023 Apple Inc. and the Swift project authors | ||
// Licensed under Apache License v2.0 with Runtime Library Exception | ||
// | ||
// See https://swift.org/LICENSE.txt for license information | ||
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors | ||
// | ||
//===----------------------------------------------------------------------===// | ||
|
||
import Foundation | ||
|
||
extension NSLock { | ||
/// NOTE: Keep in sync with SwiftPM's 'Sources/Basics/NSLock+Extensions.swift' | ||
fileprivate func withLock<T>(_ body: () throws -> T) rethrows -> T { | ||
lock() | ||
defer { unlock() } | ||
return try body() | ||
} | ||
} | ||
|
||
/// A thread safe container that contains a value of type `T`. | ||
public class ThreadSafeBox<T> { | ||
/// Lock guarding `_value`. | ||
private let lock = NSLock() | ||
|
||
private var _value: T | ||
|
||
public var value: T { | ||
get { | ||
return lock.withLock { | ||
return _value | ||
} | ||
} | ||
set { | ||
lock.withLock { | ||
_value = newValue | ||
} | ||
} | ||
} | ||
|
||
public init(initialValue: T) { | ||
_value = initialValue | ||
} | ||
|
||
/// If the value in the box is an optional, return it and reset it to `nil` | ||
/// in an atomic operation. | ||
public func takeValue<U>() -> T where U? == T { | ||
lock.withLock { | ||
guard let value = self._value else { return nil } | ||
self._value = nil | ||
return value | ||
} | ||
} | ||
} |
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
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
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.
I think this probably ought to be marked
@Sendable
(I believe we'll warn under strict concurrency)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.
Talking of strict concurrency checking, are we planning on enabling that for this repo? I don't think it's a proper package option yet, but I think you can add
.enableExperimentalFeature("StrictConcurrency")
as a build settingThere 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.
I was planning to try and enable strict concurrency checking once I don’t have any further async migrations in my backlog. Until I do that, I don’t think I care about sendable annotations too much.
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.
Fair enough