Add PhantomData marker to Context to make Context !Send and !Sync

[jihiggins]

Adds PhantomData<*mut ()> to Context in order to allow for future !Send or !Sync additions to Context's fields. This would allow for things like future single threaded async executor optimizations, or (re)adding LocalWaker, etc.

Closes #66481.

Per https://github.com/rust-lang/rust/issues/66481#issuecomment-561289725, this is a breaking change that needs a Crater run as the next step.

(So far have tested the change locally with cargotest on WSL)

[rust-highfive]

Thank you for submitting a new PR for the library teams! If this PR contains a stabilization of a library feature that has not already completed FCP in its tracking issue, introduces new or changes existing unstable library APIs, or changes our public documentation in ways that create new stability guarantees then please comment with r? rust-lang/libs-api @rustbot label +T-libs-api to request review from a libs-api team reviewer. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Mark-Simulacrum (or someone else) soon.

Please see the contribution instructions for more information.

[compiler-errors]

I believe this is T-libs-api since it affects an (auto) impl of a public type in std

r? rust-lang/libs-api @rustbot label +T-libs-api

[bors]

:umbrella: The latest upstream changes (presumably #102022) made this pull request unmergeable. Please resolve the merge conflicts.

[RalfJung]

@jihiggins sorry, this seems to have fallen through the cracks. Can you rebase on top of current master and resolve the conflicts? Then I can start a crater run.

Cc @rust-lang/wg-async

[rustbot]

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

• Stabilizing library features
• Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
• Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
• Changing public documentation in ways that create new stability guarantees
• Changing observable runtime behavior of library APIs

[rust-log-analyzer]

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

    Checking rand v0.7.3
    Checking std v0.0.0 (/checkout/library/std)
    Checking core v0.0.0 (/checkout/library/core)
    Checking alloc v0.0.0 (/checkout/library/alloc)
error[E0277]: `*mut ()` cannot be shared between threads safely
   |
   |
24 |     static CONTEXT: Context<'static> = Context::from_waker(&WAKER);
   |                     ^^^^^^^^^^^^^^^^ `*mut ()` cannot be shared between threads safely
   |
   = help: within `Context<'static>`, the trait `Sync` is not implemented for `*mut ()`
   = note: required because it appears within the type `PhantomData<*mut ()>`
   = note: required because it appears within the type `Context<'static>`
   = note: shared static variables must have a type that implements `Sync`
For more information about this error, try `rustc --explain E0277`.
error: could not compile `core` due to previous error
warning: build failed, waiting for other jobs to finish...
Build completed unsuccessfully in 0:01:55

[RalfJung]

Looks like in library/core/tests/task.rs:24:21 we already have a test relying on Context being Sync... that test case can probably be fixed but this is still not a good sign.

@bors try

[bors]

:hourglass: Trying commit f4c110319db9db6cd4a353d81c6259ee1e7a6c0a with merge 7caa343c25a692951246ea2f0664520b9c3f46da...

[bors]

:sunny: Try build successful - checks-actions Build commit: 7caa343c25a692951246ea2f0664520b9c3f46da (7caa343c25a692951246ea2f0664520b9c3f46da)

[RalfJung]

@craterbot check

[craterbot]

:ok_hand: Experiment pr-95985 created and queued. :robot: Automatically detected try build 7caa343c25a692951246ea2f0664520b9c3f46da :mag: You can check out the queue and this experiment's details.

:information_source: Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

:construction: Experiment pr-95985 is now running

:information_source: Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

:tada: Experiment pr-95985 is completed! :bar_chart: 59 regressed and 8 fixed (245696 total) :newspaper: Open the full report.

:warning: If you notice any spurious failure please add them to the blacklist! :information_source: Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[Ralith]

Reviewing regressions:

Genuine failures:

• Fedomn.rust-knowledge.e2c4007d01b504d4304ec0ed776b26125ffdbf87 is due to futures-async-stream (nightly-only) apparently capturing a Context inside a generator in a macroexpansion. Did not dig further due to nightly-only.
• desync-0.8.1 is an interesting case: In src/pipe.rs it has a pattern of capturing a context constructed via from_waker inside async blocks to later be used when polling an internal future, rather than propagating the context of the enclosing block. I haven't investigated why this approach is used, but it could conceivably be made compatible by creating Contexts just in time from captured Wakers rather than capturing them directly. I'd be happy to work with the maintainer on this if needed.

Spurious failures

Mostly due to failure to determine rustc version at build time, most commonly in libc.

gclark916.vulkano_examples.20dcf2760255f71bf4fdea0ca0f1e9d9eaea221f gclark916.vulkano_tri1.b3cc87d01ff8e6387c79b8a1942af0d966a6fa74 Ayrx.binja-rs-hello-world.a0f5fb90a3544b233856cddbdac059da21ce27ee Zoeycode.zoeycode.github.io.74a38dfea8ba84da15b4cb20ce7a38a2fa9ea2e0 etke.bff.2b0c0fb5941f2f751afcdeb15777128afec7e791 maxco2.pdfwc.5cb4bff8a9f4d3e78b4d64db7788533561c3dce6 stuartZhang.scaffold-wizard.afa3f1ff91917d552310e43422013da28e9f05c5 catboost2-sys-0.1.1+catboost.1.0.5 catboost2-0.1.1+catboost.1.0.5 promptconv-0.1.3 ps2-mouse-0.1.4 qt_ritual-0.0.0 readline-async-0.1.0 resctl-bench-2.2.3 ritual_common-0.4.0 rmake-0.1.38 rtsam-0.1.4 rune-0.12.0 rusoto_sns-0.48.0 rust-ini-0.18.0 rusted_cypher-1.1.0 rusty-menu-0.1.3 rvm-0.0.2 sdoc-0.8.11 senpy-cli-0.1.1 serde_pipe-0.1.3 singletonum-derive-0.2.0 sjq-crate-publish-demo-0.1.0 sketch-duplicates-0.1.0 smafa-0.5.0 solana-base58-json-converter-0.1.0 sortnet-0.1.0 spade-2.0.0 spglib-sys-1.16.1 split-csv-0.1.2 ssandbox-0.1.0 stable-swap-1.8.1 stats-cli-3.0.1

[Ralith]

The worst case scenario is if the handling of Contexts is part of a crates public API.

This does not appear to be the case for desync, fortunately.

[tvallotton]

Do you reckon, If we made a PR for every broken version of these two crates could this get merged?

[jihiggins]

How would the order of operations on that work? (Also, what are next steps here? Just reaching out to the maintainers for those crates?)

[guswynn]

desync appears to only be used by crates with the same maintainer: https://crates.io/crates/desync/reverse_dependencies

[m-ou-se]

Hey @rust-lang/wg-async, we (libs-api) would like your input on this! (There's some context in this issue from 2019: https://github.com/rust-lang/rust/issues/66481)

[eholk]

We've started a zulip thread on this and also tagged the issue so it will come up in our next triage meeting. We'll report back once we've had the chance to consider it and form some opinions.

[tmandry]

wg-async is in favor of this change. Context was never intended to be Send and Sync. If we can get away with fixing this now, we should do it.

[tvallotton]

Will this be able to be merged for version 1.65 or will it have to wait for 1.66?

[RalfJung]

If it was merged now (or any time before Dec 9th) it would land for 1.67, and might be backported for 1.66. It's definitely not going to be in 1.65 which will be released today.