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

Segmentation fault due to Surface::configure not caring about Device lifetime #5781

Closed
KaarelKurik opened this issue Jun 8, 2024 · 2 comments

Comments

@KaarelKurik
Copy link

Description
Surface::configure(..) does not enforce lifetimes on its parameters, which allows user code to drop a device
before the surface is done using it, resulting in a segfault.

Repro steps
The following main.rs file in a project with winit 0.20, wgpu 0.30 and pollster 0.3 (just for blocking on futures) will
reproduce the effect upon closing the spawned window.

use std::sync::Arc;

struct App<'a> {
    // Swapping the order of the device and surface members changes whether segfault happens, due to drop order.
    device: Option<wgpu::Device>,
    surface: Option<wgpu::Surface<'a>>,
    window: Option<Arc<winit::window::Window>>,
}

impl<'a> winit::application::ApplicationHandler for App<'a> {
    fn resumed(&mut self, event_loop: &winit::event_loop::ActiveEventLoop) {
        let window = Arc::new(
            event_loop
                .create_window(winit::window::WindowAttributes::default())
                .unwrap(),
        );
        let size = window.inner_size();

        let instance = wgpu::Instance::new(wgpu::InstanceDescriptor::default());
        let surface = instance.create_surface(window.clone()).unwrap();
        let adapter = pollster::block_on(instance.request_adapter(&wgpu::RequestAdapterOptions {
            power_preference: wgpu::PowerPreference::default(),
            force_fallback_adapter: false,
            compatible_surface: Some(&surface),
        }))
        .unwrap();

        let surface_config = surface
            .get_default_config(&adapter, size.width, size.height)
            .unwrap();

        let (device, queue) = pollster::block_on(adapter.request_device(
            &wgpu::DeviceDescriptor {
                label: None,
                required_features: wgpu::Features::empty(),
                required_limits: wgpu::Limits::default(),
            },
            None,
        ))
        .unwrap();

        surface.configure(&device, &surface_config); // Removing this call also fixes the segfault.
        self.surface = Some(surface);
        self.window = Some(window);
        self.device = Some(device);
    }

    fn window_event(
        &mut self,
        event_loop: &winit::event_loop::ActiveEventLoop,
        window_id: winit::window::WindowId,
        event: winit::event::WindowEvent,
    ) {
        match event {
            winit::event::WindowEvent::CloseRequested => event_loop.exit(),
            _ => {}
        }
    }
}

fn main() {
    let event_loop = winit::event_loop::EventLoop::new().unwrap();
    let mut app = App {
        device: None,
        surface: None,
        window: None,
    };

    event_loop.run_app(&mut app).unwrap();
}

Expected vs observed behavior
Expected: a compile-time error preventing the use of Surface::configure(..) with a reference that does not live long enough. Alternatively, explicit annotation of Surface::configure(..) as being unsafe.

Observed: code appears to be safe, but causes segfault.

Platform
This is being done on Linux with X11.

All that said, this is a deliberately pathological construction.

@cwfitzgerald
Copy link
Member

Does this happen on trunk as well? I think this is already fixed

@KaarelKurik
Copy link
Author

Does this happen on trunk as well? I think this is already fixed

Appears to be fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

No branches or pull requests

2 participants