Skip to content

[6.0][Concurrency] Remove last usages of @_unsafeInheritExecutor #73569

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

Conversation

ktoso
Copy link
Contributor

@ktoso ktoso commented May 10, 2024

Description: We still had a few stray uses of @_unsafeInheritExecutor which we are intending to stop using entirely. The uses were in task cancellation handler (which I noticed), and withTaskGroup*.
Scope/Impact: Use the official way of inheriting isolation, rather than the fragile unofficial attribute. Impact on users is more correct inheritance of context, making APIs usable in actors in Swift 6 mode.
Risk: Low, Verified ABI method remain and only adds #isolation new API
Testing: CI testing, added tests to verify behavior of task cancellation handler inside actor
Reviewed by: @hborla

Original PR: #73568
Radar: rdar://127874129

--

depends on #74563

@ktoso ktoso requested a review from a team as a code owner May 10, 2024 12:11
@ktoso
Copy link
Contributor Author

ktoso commented May 10, 2024

@swift-ci please test

Copy link
Member

@DougGregor DougGregor left a comment

Choose a reason for hiding this comment

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

Looks great, thank you!

@ktoso
Copy link
Contributor Author

ktoso commented May 13, 2024

Hmmm, there's one problem we need to discuss and solve about isolation of the closure still.
I.e. how should we properly handle:

actor A {
  func g() { }
  var num: Int = 0

  func f() async throws {
    await withTaskGroup(...) { group in
      g()
      num += 1
    }
  }
}

Blocking on rdar://125078448

@ktoso ktoso marked this pull request as draft May 13, 2024 04:38
@ktoso ktoso added concurrency Feature: umbrella label for concurrency language features 🍒 release cherry pick Flag: Release branch cherry picks swift 6.0 labels May 27, 2024
@ktoso ktoso marked this pull request as ready for review June 20, 2024 08:03
@ktoso
Copy link
Contributor Author

ktoso commented Jun 20, 2024

Unblocked and cleaned up 🎉

@ktoso ktoso force-pushed the pick-wip-fix-last_unsafeInheritExecutor-usage branch from b492538 to b44afcf Compare June 20, 2024 08:06
@ktoso
Copy link
Contributor Author

ktoso commented Jun 20, 2024

@swift-ci please test

@ktoso
Copy link
Contributor Author

ktoso commented Jun 20, 2024

Meh, depends on #74563 so this will fail this run.

@ktoso
Copy link
Contributor Author

ktoso commented Jun 21, 2024

@swift-ci please test

@ktoso ktoso force-pushed the pick-wip-fix-last_unsafeInheritExecutor-usage branch from b44afcf to 9b8b5eb Compare June 26, 2024 13:32
@ktoso ktoso requested a review from a team as a code owner June 26, 2024 13:32
@ktoso
Copy link
Contributor Author

ktoso commented Jun 26, 2024

@swift-ci please test

This should have been a compile time error but seems it wasn't caught. Remove the * from the backDeployed as this is not something which makes sense and would break adopter projects.
@ktoso
Copy link
Contributor Author

ktoso commented Jun 26, 2024

@swift-ci please test

@ktoso
Copy link
Contributor Author

ktoso commented Jun 27, 2024

@swift-ci please test macOS

@ktoso ktoso enabled auto-merge (squash) June 27, 2024 08:40
@ktoso ktoso disabled auto-merge June 27, 2024 08:40
@ktoso ktoso merged commit fc2be5e into swiftlang:release/6.0 Jun 27, 2024
5 checks passed
@ktoso ktoso deleted the pick-wip-fix-last_unsafeInheritExecutor-usage branch June 27, 2024 10:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
concurrency Feature: umbrella label for concurrency language features 🍒 release cherry pick Flag: Release branch cherry picks swift 6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants