Skip to content

Avoid reassigning task-local runtime state in a Runner's event handler unless it's nested #339

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 2 commits into from
Apr 10, 2024

Conversation

stmontgomery
Copy link
Contributor

This is a mitigation for a bug in which an expectation inside a withTaskGroup closure can cause a crash if it fails.

Motivation:

It can sometimes be useful to perform expectations (e.g. #expect) within the body of the closure passed to withTaskGroup or similar APIs. However, doing so currently causes a crash if the expectation fails, illustrated by this small example:

@Test func example() async {
  await withTaskGroup(of: Void.self) { _ in
    #expect("abc" == "123") // 💥
  }
}

Modifications:

This change avoids wrapping the event handler of a runner's configuration with one that reassigns a TaskLocal (potentially triggering the crash) unless that runner is running during the run operation of another runner. In other words, only mutate the TaskLocal for nested runners. Nesting, or running one runner in the context of another, is something which is only done in practice when performing tests of the testing library itself. Ordinary usage of the testing library only has one “outer“ runner.

Result:

The above example code no longer crashes, verified by the new unit test.

Checklist:

  • Code and documentation should follow the style of the Style Guide.
  • If public symbols are renamed or modified, DocC references should be updated.

Resolves rdar://126132392

…r unless it's nested

Resolves rdar://126132392
@stmontgomery stmontgomery added the bug 🪲 Something isn't working label Apr 10, 2024
@stmontgomery stmontgomery self-assigned this Apr 10, 2024
@stmontgomery
Copy link
Contributor Author

@swift-ci please test

@stmontgomery
Copy link
Contributor Author

@swift-ci please test

@stmontgomery stmontgomery merged commit 3c0a692 into main Apr 10, 2024
@stmontgomery stmontgomery deleted the runtime-state-task-local branch April 10, 2024 19:13
Copy link

@ktoso ktoso left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🪲 Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants