madsmtm
commented
8 months ago The example doesn't actually seem to be using tiny-skia? And in any case, I can't seem to reproduce the issue, are you sure you posted the correct code?
Closed apps4uco closed 8 months ago
madsmtm
commented
8 months ago The example doesn't actually seem to be using tiny-skia? And in any case, I can't seem to reproduce the issue, are you sure you posted the correct code?
apps4uco
commented
8 months ago The example doesn't actually seem to be using
tiny-skia? And in any case, I can't seem to reproduce the issue, are you sure you posted the correct code?
Sorry for the delay in replying....
Yes it is the code that produces the Exception
Just re downloaded it to check and also changed to stable, just in case it was nightly that caused the problem. The segmentation fault occurs with both nightly and stable
You are correct that the example does not use tiny-skia. It must have been late... (or too early). Removing the unused dependency in Cargo.toml Im attaching the revised, minimal test case.
zsh: segmentation fault cargo run
% rustc -V
rustc 1.75.0 (82e1608df 2023-12-21)zsh: segmentation fault cargo run
% rustc -V
rustc 1.77.0-nightly (3cdd004e5 2023-12-29)Commenting out softbuffer line
//let mut surface = softbuffer::Surface::new(&context, window.clone()).unwrap();Does not produce the segmentation fault on exit at least not on a Mac Air m1. So it could be UB in SoftBuffer, and/or some MacOs specific bug
kchibisov
commented
8 months ago The general code in the example is unsafe, so it might not be an UB at the end of the day, the reason why it's unsafe, because people just hacked up something, so it could be not a real world problem, but just some unsound code in the example. Make sure to drop the Surface and the Context from softbuffer inside the loop before exiting it.
kchibisov
commented
8 months ago This is related to this issue https://github.com/rust-windowing/winit/issues/3317 , however since softbuffer is using raw-window-handle it's not a soundness problem in winit/softbuffer, but rather the code user has since raw-window-handle is unsafe in general.
madsmtm
commented
8 months ago Tried again, still cannot reproduce on Macbook Pro w. M2 Pro running macOS Sonoma 14.2.1.
While I agree with @kchibisov, I think this kind of thing still shouldn't cause segmentation faults, especially not while releasing objects; to me, that hints more towards a double-free somewhere in Winit or softbuffer.
@apps4uco perhaps you can try to wrap the surface creation in an autorelease pool? Something like (using the objc2 crate):
let mut surface = objc2::rc::autoreleasepool(|_| {
softbuffer::Surface::new(&context, window.clone()).unwrap()
});Hi,
If I try to drop the surface and context inside the loop
if window_id == window.id() => {
//println!("Dropping");
mem::drop(surface);
mem::drop(context);
elwt.exit();I get a compile error
cannot move out of `surface`, a captured variable in an `FnMut` closureUsing @madsmtm idea of using objc2 pool
https://github.com/rust-windowing/winit/issues/3379#issuecomment-1892586177
gives the same segmentation fault
adding these lines right at the end of main.
println!("Sleep 1");
thread::sleep(Duration::from_secs(1));
println!("Dropping surface");
mem::drop(surface);
println!("Dropping context");
mem::drop(context);
println!("Sleep 2");
thread::sleep(Duration::from_secs(2));I get the output
Sleep 1
Dropping surface
Dropping context
zsh: segmentation fault cargo runSo it appears (to me) to be that the drop of the context is causing the segmentation fault.
Its weird that it cant be reproduced on another Mac, on mine it consistently fails.
Attaching the current source code
kchibisov
commented
8 months ago You can use the Option to achieve dropping inside the loop, so once you drop you assign None.
apps4uco
commented
8 months ago Thanks for looking into this....
I tried @kchibisov 's suggestion. New code attached.
winit-softbuffer-objc2-option.zip
I also set the following variables in the shell, as indicated by https://developer.apple.com/library/archive/documentation/Performance/Conceptual/ManagingMemory/Articles/MallocDebug.html
MallocStackLogging=1 MallocCheckHeapStart=1000 MallocCheckHeapEach=100 MallocStackLoggingNoCompact=1 MallocScribble=1
Running `target/debug/winit-softbuffer`
Set surface to None
Set context to None
Calling exit
zsh: segmentation fault cargo runNote the println of Sleep 1 is never printed
I also tried
lldb target/debug/winit-softbuffer
(lldb) target create "target/debug/winit-softbuffer"
Current executable set to '/tmp/target/debug/winit-softbuffer' (arm64).
(lldb) run
Process 17456 launched: '/tmp/target/debug/winit-softbuffer' (arm64)
2024-01-15 14:10:35.208451-0500 winit-softbuffer[17456:16840330] [Window] Warning: Window WinitWindow 0x1005066f0 ordered front from a non-active application and may order beneath the active application's windows.
2024-01-15 14:10:35.248207-0500 winit-softbuffer[17456:16840330] [Window] Warning: Window WinitWindow 0x1005066f0 ordered front from a non-active application and may order beneath the active application's windows.
2024-01-15 14:10:38.903335-0500 winit-softbuffer[17456:16840368] [client] No error handler for XPC error: Connection invalid
Process 17456 exited with status = 0 (0x00000000)Hmm, just to explore that path, perhaps you could try creating the window, context and surface inside of Event::NewEvents(StartCause::Init)?
apps4uco
commented
8 months ago ... sure. I delayed the init of the surface as shown:
Event::NewEvents(StartCause::Init) => {
context = Some(softbuffer::Context::new(window.clone()).unwrap());
surface = Some(softbuffer::Surface::new(context.as_mut().unwrap(), window.clone()).unwrap());
// let s = objc2::rc::autoreleasepool(|_| {
// softbuffer::Surface::new(context.as_mut().unwrap(), window.clone()).unwrap()
// });
// surface = Some(s);
}Still get a segmentation fault.
Also using the code shown in comments also produces the same result.
Set surface to None
Set context to None
Calling exit
zsh: segmentation fault cargo runNote: println Sleep is not called.
madsmtm
commented
8 months ago Huh, with that example I get a segmentation fault too!
Hold on, let me try to figure it out.
apps4uco
commented
8 months ago Just realized that you can get a report on the process exiting on a Mac, (Im more of a Linux developer ...)
madsmtm
commented
8 months ago Ugh, stupid me, I already fixed this in https://github.com/rust-windowing/softbuffer/pull/180, which was why I couldn't seem to find any problem with the code - and then I probably had trouble reproducing because my editor updated the lockfile or something...
Run cargo update to get softbuffer v0.4.1 ;)
apps4uco
commented
8 months ago Yep that solved it.... Cool thanks for looking into it ... And fixing it
In a small project that only has 3 direct dependencies, on MacOs M1 there is a segmentation fault on program exit (click on close button) Im not exactly sure what crate is causing the UB so I figured Id report it in case it was winit.
Cargo.toml has
winit-softbuffer-tinyskia.zip
rustc -V
rustc 1.77.0-nightly (3cdd004e5 2023-12-29)
uname -a
Darwin .....-mac-m1 22.5.0 Darwin Kernel Version 22.5.0: Thu Jun 8 22:22:19 PDT 2023; root:xnu-8796.121.3~7/RELEASE_ARM64_T8103 arm64