Skip to content

[Concurrency] Prevent task re-enqueues in continuation APIs #84944

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

[Concurrency] Prevent task re-enqueues in continuation APIs #84944

wants to merge 6 commits into from

Conversation

@ktoso
Copy link
Contributor

@ktoso ktoso commented Oct 16, 2025 •
edited
Loading

This is the minimal set of changes from
#80753 to specifically address the with...Continuation APIs re-enqueueing tasks when they need not have to.

Resolves rdar://162192512

Before, 10 enqueues in total in the task executor case:

         1: === foo() async
         2: ---------------------------------------
         3: [executor][task-executor] Enqueue (1)
         4: foo - withTaskExecutorPreference
         5: [executor][task-executor] Enqueue (2)
         6: foo - withTaskExecutorPreference - withCheckedContinuation
         7: [executor][task-executor] Enqueue (3)
         8: foo - withTaskExecutorPreference - withCheckedContinuation done
         9: [executor][task-executor] Enqueue (4)
        10: foo - withTaskExecutorPreference - withUnsafeContinuation
        11: [executor][task-executor] Enqueue (5)
        12: foo - withTaskExecutorPreference - withUnsafeContinuation done
        13: [executor][task-executor] Enqueue (6)
        14: foo - withTaskExecutorPreference - withCheckedThrowingContinuation
        15: [executor][task-executor] Enqueue (7)
        16: foo - withTaskExecutorPreference - withCheckedThrowingContinuation done
        17: [executor][task-executor] Enqueue (8)
        18: foo - withTaskExecutorPreference - withUnsafeThrowingContinuation
        19: [executor][task-executor] Enqueue (9)
        20: foo - withTaskExecutorPreference - withUnsafeThrowingContinuation done
        21: [executor][task-executor] Enqueue (10)
        22: foo - withTaskExecutorPreference done
        23: == Make: actor Foo
        24: ---------------------------------------
        25: [executor][actor-executor] Enqueue (1)
        26: actor.foo
        27: actor.foo - withCheckedContinuation
        28: actor.foo - withCheckedContinuation done
        29: actor.foo - withUnsafeContinuation
        30: actor.foo - withUnsafeContinuation done
        31: actor.foo - withCheckedThrowingContinuation
        32: actor.foo - withCheckedThrowingContinuation done
        33: actor.foo - withUnsafeThrowingContinuation
        34: actor.foo - withUnsafeThrowingContinuation done
        35: actor.foo done
        36: done

After, two total enqueues in the task executor:

    1: === foo() async
    2: ---------------------------------------
    3: [executor][task-executor] Enqueue (1)
    4: foo - withTaskExecutorPreference
    5: foo - withTaskExecutorPreference - withCheckedContinuation
    6: foo - withTaskExecutorPreference - withCheckedContinuation done
    7: foo - withTaskExecutorPreference - withUnsafeContinuation
    8: foo - withTaskExecutorPreference - withUnsafeContinuation done
    9: foo - withTaskExecutorPreference - withCheckedThrowingContinuation
   10: foo - withTaskExecutorPreference - withCheckedThrowingContinuation done
   11: foo - withTaskExecutorPreference - withUnsafeThrowingContinuation
   12: foo - withTaskExecutorPreference - withUnsafeThrowingContinuation done
   13: [executor][task-executor] Enqueue (2)
   14: foo - withTaskExecutorPreference done
   15: == Make: actor Foo
   16: ---------------------------------------
   17: [executor][actor-executor] Enqueue (1)
   18: actor.foo
   19: actor.foo - withCheckedContinuation
   20: actor.foo - withCheckedContinuation done
   21: actor.foo - withUnsafeContinuation
   22: actor.foo - withUnsafeContinuation done
   23: actor.foo - withCheckedThrowingContinuation
   24: actor.foo - withCheckedThrowingContinuation done
   25: actor.foo - withUnsafeThrowingContinuation
   26: actor.foo - withUnsafeThrowingContinuation done
   27: actor.foo done
   28: done

@ktoso ktoso requested a review from phausler as a code owner October 16, 2025 03:54
ktoso
ktoso commented Oct 16, 2025
View reviewed changes
@ktoso
Copy link
Contributor Author

ktoso commented Oct 16, 2025

@swift-ci please smoke test

@ktoso
Copy link
Contributor Author

ktoso commented Oct 20, 2025

The release/debug modes was a bit misleading and we should fix that as well tbh, since it did complete hop elimination. but it did HIDE the real issue actually.

The real issue is in the enqueue and run methods, and I feel like we talked about this but we never fixed it... I'll fix up the tests tomorrow.

@ktoso ktoso force-pushed the wip-withContinuation-nonisolated-nonsending branch 3 times, most recently from ea36b2d to 50f7d5b Compare October 21, 2025 07:36
@ktoso ktoso requested a review from eeckstein as a code owner October 21, 2025 07:36
@ktoso ktoso force-pushed the wip-withContinuation-nonisolated-nonsending branch from 50f7d5b to 9c96b6c Compare October 21, 2025 07:44
@ktoso
[Concurrency] Prevent task re-enqueues in continuation APIs
13c453b 
This explicitly adopts nonisolated nonsending funcs for the with
continuation APIs. This guarantees there are not spurious enqueues
between the caller and the closure executing, which is a requirement for
these methods.

This takes a risk on removing the bad #isolation based API from source,
however explicitly passing the isolation as something else than the
actual isolation would always have been incorrect, so we should try to
get this through.

There are further re-enqueue fixes to be done, but this specific API
should adopt the nonisolated nonsending approach.

Resolves rdar://162192512
@ktoso ktoso force-pushed the wip-withContinuation-nonisolated-nonsending branch from 9c96b6c to 13c453b Compare October 21, 2025 07:48
ktoso
ktoso commented Oct 21, 2025
View reviewed changes
stdlib/public/Concurrency/PartialAsyncTask.swift
@unsafe
public func withUnsafeThrowingContinuation<T>(
isolation: isolated (any Actor)? = #isolation,
public nonisolated(nonsending) func withUnsafeThrowingContinuation<T>(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope we can get away with this "technically" source break... Let's see what the source compat suite says.

@ktoso ktoso requested a review from hborla October 21, 2025 07:49
@ktoso
Copy link
Contributor Author

ktoso commented Oct 21, 2025

@swift-ci please smoke test

@ktoso
Copy link
Contributor Author

ktoso commented Oct 21, 2025

@swift-ci please smoke test

@ktoso
Copy link
Contributor Author

ktoso commented Oct 21, 2025

@swift-ci please test source compatibility

@ktoso ktoso mentioned this pull request Oct 21, 2025
Closed
@ktoso
Copy link
Contributor Author

ktoso commented Oct 21, 2025

@swift-ci please test source compatibility

@ktoso
Copy link
Contributor Author

ktoso commented Oct 21, 2025

@swift-ci please smoke test

@ktoso
Copy link
Contributor Author

ktoso commented Oct 22, 2025

@swift-ci please smoke test

@ktoso
Copy link
Contributor Author

ktoso commented Oct 22, 2025

@swift-ci please smoke test macOS

@ktoso
Copy link
Contributor Author

ktoso commented Oct 22, 2025

@swift-ci please smoke test Windows

@ktoso
Copy link
Contributor Author

ktoso commented Oct 22, 2025

@swift-ci please test source compatibility

@ktoso
Copy link
Contributor Author

ktoso commented Oct 23, 2025

The hummingbird issue in release mode seems unrelated to this change.

@hamishknight
Copy link
Contributor

#85062 has landed, you should be able to drop the workaround commits now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@phausler phausler Awaiting requested review from phausler phausler is a code owner

@eeckstein eeckstein Awaiting requested review from eeckstein

@hborla hborla Awaiting requested review from hborla

@bnbarham bnbarham Awaiting requested review from bnbarham bnbarham is a code owner

@hamishknight hamishknight Awaiting requested review from hamishknight hamishknight is a code owner

@rintaro rintaro Awaiting requested review from rintaro rintaro is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ktoso @hamishknight