Create CancellableQueue #63
Merged
+801
−2
Conversation
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
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #63 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 11 13 +2
Lines 1015 1293 +278
==========================================
+ Hits 1015 1293 +278
🚀 New features to boost your workflow:
|
dfed
force-pushed
the
dfed--cancellation
branch
from
d7ee222 to
3ec737f
Compare
dfed
commented
Nov 28, 2025
| // MARK: Public | ||
|
|
||
| /// Cancels the currently executing task, as well as any task currently pending in the queue. | ||
| public func cancelTasks() { |
Owner
Author
There was a problem hiding this comment.
cancelAll is a reasonable name too. I like cancelTasks a little bit better because it doesn't imply that future tasks will be cancelled as well.
* Ignore agent configuration * Add CancellableQueueTests for 100% test coverage Test cancelTasks() behavior across all queue types: - FIFOQueue (with isolated and throwing variants) - ActorQueue - MainActor queue Each queue type tests four scenarios: - doesNotCancelCompletedTask - cancelsCurrentlyExecutingTask - cancelsCurrentlyExecutingAndPendingTasks - doesNotCancelFutureTasks * Use semaphores instead of sleep for test synchronization Replace brittle Task.sleep timing with proper semaphore-based synchronization to ensure tasks have started before cancellation. * Simplify tests by returning Task.isCancelled directly Remove Expectation usage in favor of checking task.value directly. This eliminates timeouts and makes tests cleaner. * Use task.isCancelled property instead of returning from closure Check cancellation status directly on the Task instance rather than returning Task.isCancelled from within the closure. * Remove unnecessary task awaits Check task.isCancelled immediately after cancelTasks() without waiting for task completion. Only await task.result where needed to ensure task completes before calling cancelTasks(). * Fix some of the 'didn't wait long enough' problem * Add taskAllowedToEnd semaphore to ActorQueue and MainActor tests Ensure tasks are still executing when cancelTasks() is called by having them wait on a semaphore that is signaled after cancellation. * Fix misleading comments about ActorQueue task ordering ActorQueues are re-entrant, not FIFO, so tasks don't wait for earlier tasks to complete. * Fix flaky ActorQueue and MainActor cancellation tests ActorQueues are re-entrant, so task2 and task3 would complete immediately when task1 suspends, potentially before cancelTasks() is called. Now all three tasks signal when started and wait on semaphores, ensuring they're all still executing when cancelled. * Rename ActorQueue/MainActor tests to reflect concurrent execution ActorQueues are re-entrant, so tasks execute concurrently rather than pending. Renamed tests from 'cancelsCurrentlyExecutingAndPendingTasks' to 'cancelsAllConcurrentlyExecutingTasks' to accurately describe behavior. * Revert "Rename ActorQueue/MainActor tests to reflect concurrent execution" This reverts commit 88f7fd0. * mark * formatting * Simplify ActorQueue and MainActor multi-task cancellation tests We only need to wait for task1 to start. Whether task2 and task3 are pending or already executing when cancelTasks() is called, they will be cancelled either way. * Ensure task2 and task3 wait before cancelTasks() is called Tasks need to wait on the semaphore to ensure they haven't completed and been removed from the cancel map before cancelTasks() is called. * Ensure task3 is pending in ActorQueue/MainActor cancellation tests Task2 now spins with try Task.checkCancellation() without suspension points, preventing task3 from starting on the re-entrant queue. This guarantees task3 is truly pending when cancelTasks() is called. --------- Co-authored-by: Claude <noreply@anthropic.com>
dfed
commented
Nov 28, 2025
* Improve CancellableQueue documentation - Expand class-level doc comment to explain behavior - Add missing priority parameter documentation - Fix "priority of the queue" typo (should be "priority of the task") - Add CancellableQueue section to README with usage examples * Apply suggestions from code review --------- Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Addresses #62 by creating a new
CancellableQueuetype that wraps eitherFIFOQueueorActorQueueand allows tasks enqueued on the cancellable queue to be cancelled.I created a separate queue type to avoid making every
FIFOQueueandActorQueuecreate yet another queue – this feels like a specialized use case, and so a specialized queue type seemed like a reasonable solution.