Skip to content

Synchronize adding a child task to a TaskGroup with parent task #40520

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

Conversation

rokhinip
Copy link
Contributor

@rokhinip rokhinip commented Dec 11, 2021

When a task is adding adding new children to a task group, we need to synchronize with the task status record lock of the parent task that has the task group, to prevent races with concurrent cancellation and escalation.

In the future, we'd probably want a lock for the TaskGroup itself and not have it be the group's parent task's status record lock but for now, this will get us the right functionality.

rdar://86311782

when a task is adding adding new children to a task group, we need to
synchronize with the task status record lock of the parent task that has the
task group, to prevent races with concurrent cancellation and escalation.

Radar-Id: rdar://problem/86311782
@rokhinip rokhinip requested review from ktoso and rjmccall December 11, 2021 04:25
Copy link
Contributor

@rjmccall rjmccall left a comment

Choose a reason for hiding this comment

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

LGTM

@ktoso
Copy link
Contributor

ktoso commented Dec 11, 2021

Resolves rdar://86311782

@ktoso
Copy link
Contributor

ktoso commented Dec 11, 2021

@swift-ci please smoke test

Copy link
Contributor

@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, thanks for spotting this!

Feel free to merge once CI passes, removing the double-log can be done any time.

@ktoso ktoso added the concurrency Feature: umbrella label for concurrency language features label Dec 11, 2021
@ktoso
Copy link
Contributor

ktoso commented Dec 13, 2021

Failure was the unrelated

/home/buildnode/jenkins/workspace/swift-PR-Linux-smoke-test/branch-main/swiftpm/Tests/WorkspaceTests/RegistryPackageContainerTests.swift:421:39: error: type 'FingerprintCheckingMode' has no member 'none'
20:09:38             fingerprintCheckingMode: .none,
20:09:38                                      ~^~~~

from swiftpm that we fixed; rebuilding

@ktoso
Copy link
Contributor

ktoso commented Dec 13, 2021

@swift-ci please smoke test

Co-authored-by: Konrad `ktoso` Malawski <[email protected]>
@rokhinip
Copy link
Contributor Author

@swift-ci please smoke test

@ktoso
Copy link
Contributor

ktoso commented Dec 14, 2021

rdar://86311782

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants