Skip to content

Fix a race condition during the computation of uriToWorkspaceCache #1170

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

ahoppen
Copy link
Member

@ahoppen ahoppen commented Apr 8, 2024

See comment in workspaceForDocument that explains the race.

@ahoppen ahoppen requested a review from bnbarham April 8, 2024 20:16
@ahoppen ahoppen requested a review from benlangmuir as a code owner April 8, 2024 20:16
@ahoppen
Copy link
Member Author

ahoppen commented Apr 8, 2024

@swift-ci Please test

@@ -169,6 +169,15 @@ extension AsyncQueue where TaskMetadata == Serial {
return self.async(priority: priority, metadata: Serial(), operation: operation)
}

/// Executes the given `operation` and awaits its result.
@discardableResult
public func sync<Success: Sendable>(
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
public func sync<Success: Sendable>(
public func asyncResult<Success: Sendable>(

Maybe? Seems weird to call it sync since it is really still async.

Copy link
Member Author

Choose a reason for hiding this comment

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

I removed it and added await <xxx>.value instead. Turns out that one of the three calls benefited from using await <xxx>.valuePropagatingCancellation

See comment in `workspaceForDocument` that explains the race.
@ahoppen ahoppen force-pushed the uri-to-workspace-cache-race-condition branch from 2f9e211 to a6f74fc Compare April 18, 2024 04:09
@ahoppen
Copy link
Member Author

ahoppen commented Apr 18, 2024

@swift-ci Please test

@ahoppen
Copy link
Member Author

ahoppen commented Apr 18, 2024

@swift-ci Please test Windows

@ahoppen ahoppen enabled auto-merge April 18, 2024 22:50
@ahoppen ahoppen merged commit 0d25ce3 into swiftlang:main Apr 19, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants