Skip to content

Conversation

@purplesyringa
Copy link
Contributor

This is a mistake I've found in many projects on GitHub and made myself. It's obvious that Error::new/Error::other allocate when you look at their signatures, and it's obvious that the pre_exec closure shouldn't allocate, but when you're asked to write a closure that returns a specific type, you don't expect its main constructor to be problematic.

I've included the list of 71 affected projects in a spoiler, though I don't expect anyone to look through it closely. It includes popular projects like alacritty, pika-backup, rio, rpm-ostree, and headcrab, as well as many smaller ones. For the most part, the snippets I've found are careful to only call functions from libc, nix, or rustix, so it's not a case of general incompetence, but something that just slipped through the cracks.

This docs section is already cluttered, so I've kept it terse.

If this is merged, I'd like to mass-post issues to the affected projects linking to this PR as a centralized source of information and place for discussion. Are there any objections about that?

@rustbot label +A-docs +A-process +O-unix +T-libs-api

List of affected projects

Format: links (witness of multi-threading)

@rustbot rustbot added O-unix Operating system: Unix-like 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 Nov 15, 2025
@rustbot
Copy link
Collaborator

rustbot commented Nov 15, 2025

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

@rustbot rustbot added A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools A-process Area: `std::process` and `std::env` T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Nov 15, 2025
/// other threads perhaps still running when the `fork` was run).
///
/// Note that the list of allocating functions includes [`Error::new`] and
/// [`Error::other`]. To signal a non-trivial error, prefer [`panic!`].
Copy link
Member

Choose a reason for hiding this comment

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

Is panic safe to call? Won't it allocate the payload?

Copy link
Contributor Author

@purplesyringa purplesyringa Nov 15, 2025

Choose a reason for hiding this comment

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

This is answered by the existing documentation slightly below. TL;DR is that the message is printed, but without traceback; the panic hook is not invoked; and unwinding doesn't occur. So the payload doesn't need to be allocated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools A-process Area: `std::process` and `std::env` O-unix Operating system: Unix-like 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. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants