-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Always check the result of pthread_mutex_lock
#120238
Conversation
(rustbot has picked a reviewer for you, use r? to override) |
Wow, thanks for beating me to this! |
r=me with commits squashed |
64ce665
to
8d708e5
Compare
@bors r+ |
…, r=Mark-Simulacrum Always check the result of `pthread_mutex_lock` Fixes rust-lang#120147. Instead of manually adding a list of "good" platforms, I've simply made the check unconditional. pthread's mutex is already quite slow on most platforms, so one single well-predictable branch shouldn't hurt performance too much.
…, r=Mark-Simulacrum Always check the result of `pthread_mutex_lock` Fixes rust-lang#120147. Instead of manually adding a list of "good" platforms, I've simply made the check unconditional. pthread's mutex is already quite slow on most platforms, so one single well-predictable branch shouldn't hurt performance too much.
…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#116677 (References refer to allocated objects) - rust-lang#118533 (Suppress unhelpful diagnostics for unresolved top level attributes) - rust-lang#120232 (Add support for custom JSON targets when using build-std.) - rust-lang#120238 (Always check the result of `pthread_mutex_lock`) - rust-lang#120266 (Improve documentation for [A]Rc::into_inner) - rust-lang#120373 (Adjust Behaviour of `read_dir` and `ReadDir` in Windows Implementation: Check Whether Path to Search In Exists) r? `@ghost` `@rustbot` modify labels: rollup
@bors rollup=iffy |
@matthiaskrgr I think that rollup failed because of #120232. This PR doesn't modify anything that would cause the errors seen in the rollup. |
Hmm well I have been scratching my head at that failure for a bit, guess we can just leave both prs on iffy, just in case 😅 |
I've seen these issues before (e.g. #103193). They only occur on some targets, while others using the same code work just fine... I suspect that this is a bug in the const-checking logic... Either way, this PR should be fine (it really does not touch anything related to |
…r=Mark-Simulacrum Always check the result of `pthread_mutex_lock` Fixes rust-lang#120147. Instead of manually adding a list of "good" platforms, I've simply made the check unconditional. pthread's mutex is already quite slow on most platforms, so one single well-predictable branch shouldn't hurt performance too much.
💥 Test timed out |
This comment has been minimized.
This comment has been minimized.
@bors retry macos timeout again |
Okay, so this PR is apparently cursed... I don't think that this failure has anything to do with the changes here. The code in question does not use Could we retry please 😉? |
@bors retry |
…r=Mark-Simulacrum Always check the result of `pthread_mutex_lock` Fixes rust-lang#120147. Instead of manually adding a list of "good" platforms, I've simply made the check unconditional. pthread's mutex is already quite slow on most platforms, so one single well-predictable branch shouldn't hurt performance too much.
The job Click to see the possible cause of the failure (guessed by this bot)
|
💔 Test failed - checks-actions |
Ok, so the failure is real. I have access to a macOS machine, but I can't test this locally as trying to run the tests fails with a different error (some relocation that isn't implemented). CC @bjorn3 (I couldn't find a ping group for cranelift) |
This issue has been randomly happening on several PR's ever since am update of the apple runner image. It is currently being investigated on zulip. (on mobile, so I can't easily link it, but it is currently the last topic in the t-infra stream) @bors retry spurious apple r_symbolnum cranelift linker failure |
…r=Mark-Simulacrum Always check the result of `pthread_mutex_lock` Fixes rust-lang#120147. Instead of manually adding a list of "good" platforms, I've simply made the check unconditional. pthread's mutex is already quite slow on most platforms, so one single well-predictable branch shouldn't hurt performance too much.
💔 Test failed - checks-actions |
@bors retry spurious network error |
☀️ Test successful - checks-actions |
Finished benchmarking commit (972452c): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 661.327s -> 662.964s (0.25%) |
Fixes #120147.
Instead of manually adding a list of "good" platforms, I've simply made the check unconditional. pthread's mutex is already quite slow on most platforms, so one single well-predictable branch shouldn't hurt performance too much.