Skip to content

Concurrency: Guard TG state with a mutex even with cooperative executor #75008

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

kateinoigakukun
Copy link
Member

SWIFT_STDLIB_SINGLE_THREADED_CONCURRENCY just means that the global executor is cooperative, but it doesn't mean that the target platform is always single-threaded. For example, on wasm32-unknown-wasip1-threads, the global executor is cooperative, but users can still set up their own TaskExecutor with multiple threads.

This patch guards the TaskGroup state with a mutex even with a cooperative executor by respecting threading package instead. This change effectively affects only wasm32-unknown-wasip1-threads.

`SWIFT_STDLIB_SINGLE_THREADED_CONCURRENCY` just means that the global
executor is cooperative, but it doesn't mean that the target platform is
always single-threaded. For example, on wasm32-unknown-wasip1-threads,
the global executor is cooperative, but users can still set up their own
TaskExecutor with multiple threads.

This patch guards the `TaskGroup` state with a mutex even with a
cooperative executor by respecting threading package instead. This
change effectively affects only wasm32-unknown-wasip1-threads.
@kateinoigakukun
Copy link
Member Author

@swift-ci smoke test

@kateinoigakukun
Copy link
Member Author

@swift-ci test WebAssembly

@kateinoigakukun kateinoigakukun marked this pull request as ready for review July 6, 2024 11:16
@kateinoigakukun kateinoigakukun requested a review from ktoso as a code owner July 6, 2024 11:16
@MaxDesiatov MaxDesiatov requested review from etcwilde and al45tair July 7, 2024 07:44
@ktoso ktoso added the concurrency Feature: umbrella label for concurrency language features label Jul 8, 2024
@ktoso
Copy link
Contributor

ktoso commented Jul 8, 2024

I added your comment in the PR as a source comment, keeping track of the "why" of those #if is very tricky so we need to document such in source IMHO, pending some other improved refactoring of the runtime that would make this less difficult to manage/understand.

LGTM though, merging

@ktoso ktoso enabled auto-merge July 8, 2024 01:51
@ktoso
Copy link
Contributor

ktoso commented Jul 8, 2024

@kateinoigakukun do we want this on 6.0 as well? If so please prepare a PR and ping me :)

@kateinoigakukun
Copy link
Member Author

Thanks! Yes, we need this in 6.0 too

@kateinoigakukun
Copy link
Member Author

@swift-ci smoke test

@ktoso ktoso merged commit a8f68aa into swiftlang:main Jul 8, 2024
3 checks passed
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.

2 participants