Skip to content

Make std::fs::File Send on UEFI#154003

Open
ginnyTheCat wants to merge 1 commit intorust-lang:mainfrom
ginnyTheCat:uefi-file-send
Open

Make std::fs::File Send on UEFI#154003
ginnyTheCat wants to merge 1 commit intorust-lang:mainfrom
ginnyTheCat:uefi-file-send

Conversation

@ginnyTheCat
Copy link
Contributor

@ginnyTheCat ginnyTheCat commented Mar 17, 2026

Similarly to #150990 since UEFI has no threads, this should be safe.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Mar 17, 2026
@rustbot
Copy link
Collaborator

rustbot commented Mar 17, 2026

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: @ChrisDenton, libs
  • @ChrisDenton, libs expanded to 8 candidates
  • Random selection from Mark-Simulacrum, joboet

@Mark-Simulacrum
Copy link
Member

Mark-Simulacrum commented Mar 23, 2026

Hm, I'm wondering if this justification is really enough. If it is, it seems like this is something we should teach the compiler about, rather than putting it in code where it might be left in by accident / copied into the wrong place / etc. I'll start a thread on Zulip about that (edit: https://rust-lang.zulipchat.com/#narrow/channel/136281-t-opsem/topic/Send.20on.20thread-free.20targets/with/580986733).

I guess this is technically also making a new stable guarantee (although on a tier 2 target), so it probably needs FCP? I see that wasn't applied in #150990, perhaps because it seems like this is 'obvious', but it seems like we should at least raise it with libs-api.

@Mark-Simulacrum Mark-Simulacrum added the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Mar 23, 2026
@Mark-Simulacrum
Copy link
Member

Also, can you say a bit more about the rationale for this? What specifically is needing a Send bound that's making this needed on the target? I see the other PR mentions remote-test-server, but I'm not seeing any merged PRs there recently that seem related.

@ginnyTheCat
Copy link
Contributor Author

ginnyTheCat commented Mar 23, 2026

this is something we should teach the compiler about

I agree. Especially on wasm there is a lot of unsafe code that relies on there only being one thread (in wasm-bindgen for example if i remember correctly). This could be avoided by everything implementing Send on targets without thread spawn support (UEFI or wasm32 in the browser).

can you say a bit more about the rationale for this?

This isn't really related to any core Rust things like the other PR is, but me having code that kind of looks like this:

let file = File::open("data")?;
thread::scope(|s| {
    for _ in 1..available_parallelism().map_or(2, NonZero::get) {
        if Builder::new().spawn_scoped(s, || worker(&file)).is_err() {
            break;
        }
    }
    worker(&file);
});

This would only use a single thread on UEFI while being multithreaded everywhere else. Currently this has to be done using #[cfg] since File is not Send on UEFI (while being on all other targets).

@Amanieu
Copy link
Member

Amanieu commented Mar 24, 2026

We discussed this in the @rust-lang/libs-api meeting. We agree that File should always implement Send for several reasons:

  • It's documented as such in the standard library docs, which make no mention of this being dependent on the target.
  • As a general rule, the std API (unlike core) isn't async-signal-safe. This means that it cannot be used in signal handlers or interrupt handlers, even for types that implement Send.
  • Finally, the standard library on UEFI only works on the boot CPU. While it is possible to run code on secondary CPUs with asm, this should be treated the same way as interrupt handlers and be restricted to core.

As a side note, #150990 really should have gone through a libs-api FCP since it is making an API change, even if only a tier 2 target.

@rfcbot merge libs-api

@Amanieu Amanieu removed the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Mar 24, 2026
@rust-rfcbot
Copy link
Collaborator

rust-rfcbot commented Mar 24, 2026

Team member @Amanieu has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rust-rfcbot rust-rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Mar 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants