Crash writing PNG from a C++ background thread on macOS Rust 1.84.0 #136043
Comments
|
I can reproduce this on my computer. The crash is happening inside |
|
If you look at the disassembly, you'll see that the failure is in a stack probe that tries to allocate Compiling under Usually you'd see an error like |
|
#131460 seems suspicious. (Although it presumably makes this use less stack space in release builds.) |
|
It seems like that PR increased the number of locals of type @rustbot label -I-prioritize -needs-triage -regression-untriaged -C-bug +C-external-bug |
|
cc @jwong101 (author of the PR I linked.) |
|
Minimized (with some differences from the original code, in particular, I'm using a much larger allocation (the original miniz_oxide code created a 164098-byte struct on the heap)): #[allow(dead_code)]
struct Thing([u8; 1000000]);
impl Default for Thing {
fn default() -> Self {
Thing([0; 1000000])
}
}
fn main() {
std::thread::Builder::new()
.stack_size(2500000)
.spawn(|| {
let _x: Box<Thing> = Box::default();
})
.unwrap()
.join()
.unwrap();
}On my computer (mac), in debug mode, this runs fine on version 1.83.0. It crashes with This code also reproduces the issue on godbolt, which I believe is on linux. It appears that version 1.84.0 puts three copies of the array onto the stack, for some reason. |
|
The code in miniz_oxide first used Box::default in this manner at Frommi/miniz_oxide@33585a6, saying that it uses the "default trick" to avoid excessive stack copies. I don't know where this supposed trick is from. |
|
It looks like most of the stack growth in debug mode is due to the fact that |
|
Manually inlining fn new_default<T: Default>() -> Box<T> {
let mut x: Box<MaybeUninit<T>>= Box::new_uninit();
unsafe {
x.as_mut_ptr().cast::<T>().write(T::default());
x.assume_init()
}
} |
It stems from back when rust didn't manage optimize That hasn't been relevant for quite some time now though as the compiler is smart enough to optimized it properly in most cases now since and also as |
This is not true, starting in 1.84.0 (changed in 131460). And I suspect that this change actually caused the regression. |
|
The most foolproof fix in miniz_oxide would probably be to use something like zeroed_box from bytemuck to allocate the I think that, on the rust side though, having 3 copies of the value on the stack isn't exactly ideal. |
Ah sorry, I' haven't fully kept up with all changes. At least using Box::new should make it act like before this change as I would rather avoid introducing a dependency like that if I can avoid it. Ideally there would be a safe way to directly heap allocate objects in standard safe rust by now that didn't result in excess stack usage in debug mode but I guess that's still a tricky issue to solve. |
|
We should be able to reduce the number of copies of However, there's still a couple of extra pointers on the stack in debug mode, but the overhead in terms of I'll make a PR later tonight once I get back home. Edit: |
…sage, r=Amanieu Reduce `Box::default` stack copies in debug mode The `Box::new(T::default())` implementation of `Box::default` only had two stack copies in debug mode, compared to the current version, which has four. By avoiding creating any `MaybeUninit<T>`'s and just writing `T` directly to the `Box` pointer, the stack usage in debug mode remains the same as the old version. Another option would be to mark `Box::write` as `#[inline(always)]`, and change it's implementation to to avoid calling `MaybeUninit::write` (which creates a `MaybeUninit<T>` on the stack) and to use `ptr::write` instead. Fixes: rust-lang#136043
…sage, r=Amanieu Reduce `Box::default` stack copies in debug mode The `Box::new(T::default())` implementation of `Box::default` only had two stack copies in debug mode, compared to the current version, which has four. By avoiding creating any `MaybeUninit<T>`'s and just writing `T` directly to the `Box` pointer, the stack usage in debug mode remains the same as the old version. Another option would be to mark `Box::write` as `#[inline(always)]`, and change it's implementation to to avoid calling `MaybeUninit::write` (which creates a `MaybeUninit<T>` on the stack) and to use `ptr::write` instead. Fixes: rust-lang#136043
Rollup merge of rust-lang#136089 - jwong101:box-default-debug-stack-usage, r=Amanieu Reduce `Box::default` stack copies in debug mode The `Box::new(T::default())` implementation of `Box::default` only had two stack copies in debug mode, compared to the current version, which has four. By avoiding creating any `MaybeUninit<T>`'s and just writing `T` directly to the `Box` pointer, the stack usage in debug mode remains the same as the old version. Another option would be to mark `Box::write` as `#[inline(always)]`, and change it's implementation to to avoid calling `MaybeUninit::write` (which creates a `MaybeUninit<T>` on the stack) and to use `ptr::write` instead. Fixes: rust-lang#136043
I'm writing PNG files from a C++ background thread. It used to work on Rust stable 1.83.0, but it crashes on Rust stable 1.84.0 and nightly. I'm using the
pngcrate, but it or its dependencies shouldn't have any code, which would cause the crash.The crash happens on macOS and Rust 1.84.0 (I tested on arm and intel macs). The program runs without issues on Ubuntu linux Rust 1.84.0. I didn't test Windows. There are no issues writing PNGs from the C++ main thread, but PNG writing crashes on the C++ background thread. If I write the same program in pure Rust without ffi and using Rust threads, then everything works without issues.
Could this be a regression in the Rust standard library? Or is the Rust function called from the C++ background thread in a way that is incompatible with Rust 1.84.0?
Code
Here's a minimal example project as a zip file so that you can see the code and try it yourself: test-cthread-png.zip
I expected to see this happen: Two PNG files should be written to the current folder: 'out-png-1.png' and 'out-png-2.png'
Instead, this happened: Only one PNG file is written: 'out-png-1.png' (on the main thread) and then the program crashes:
Version it worked on
It most recently worked on: Rust 1.83
Version with regression
rustc --version --verbose:LLVM Stack Trace
Stack trace of the example project
The text was updated successfully, but these errors were encountered: