std: use nonpoison::Mutex for all internal mutexes

[joboet]

Under the hopefully reasonable assumption that std's code is bug-free, all of the mutexes switched here will never be poisoned. The only poisoning mutex left is the one used for output capturing, as that has subtle effects on what gets captured.

[rustbot]

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

[joboet]

The manual implementation is necessary to avoid a situation like #146087.

[bjorn3]

If a panic happens anyway, do all these cases preserve correct internal state? If not, then we should actually keep propagating the panic through poisoning IMO.

[Mark-Simulacrum]

Maybe worth starting with the ones that we always unwrap_or_else(|e| e.into_inner()) on anyway? That seems easy to land.

I think for the ones that we sometimes just unwrap() on I'd lean towards keeping poisoning for the same reason outlined by @bjorn3 -- I'm not that convinced we lack bugs, and it seems like a cheap (good!) defense against those.

[joboet]

If a panic happens anyway, do all these cases preserve correct internal state? If not, then we should actually keep propagating the panic through poisoning IMO.

They do.

I think for the ones that we sometimes just unwrap() on I'd lean towards keeping poisoning for the same reason outlined by @bjorn3 -- I'm not that convinced we lack bugs, and it seems like a cheap (good!) defense against those.

I think it's important to keep in mind that such bugs would still result in a panic, and would hence need to corrupt the internal state in a way that leads to unsoundness before this would make a meaningful difference.

[bjorn3]

I think it's important to keep in mind that such bugs would still result in a panic

Only on the thread where the buggy method was called. If this thread is never joined, without poisoning the panic will never propagate to any other threads.

[joboet]

I'd like to point out that T-libs-api wants to switch std::sync::Mutex to the non-poisoning variant at some point. I'd be very confused if std set such a public standard, but then didn't follow it internally. So while I am personally of the opinion that poisoning is far too cautious and rarely helpful, I think your pushback here illustrates the need for a larger discussion on std's policy.

@rustbot label +I-libs-api-nominated

[Mark-Simulacrum]

I'd be very confused if std set such a public standard, but then didn't follow it internally.

I think the default matters (and non-poison is probably the right default). But I'm not sure that means std can't use poisoned Mutexes anywhere -- that seems like a strange position to take. In ~95% of cases I'd tend to agree that they don't add value, but sometimes they do, and then we should aim to use them :)