diff --git a/rules/gui/windows-silent-exit-0xc000041d.md b/rules/gui/windows-silent-exit-0xc000041d.md index 32461b2..37031aa 100644 --- a/rules/gui/windows-silent-exit-0xc000041d.md +++ b/rules/gui/windows-silent-exit-0xc000041d.md @@ -116,16 +116,31 @@ iris does `SetPixelFormat` + the first `wglMakeCurrent` (via glutin's state is populated → null deref. AMD's ICD is historically the worst offender for this cross-thread setup race. -## Fix direction - -Establish the drawable fully on the **main thread, before `Ui::run` starts the -event loop** (no messages are being pumped yet, so the AMD hook can't fire -mid-setup): create the `Surface` and do the first `make_current` in `Ui::new`, -then `make_not_current()` and hand both the `NotCurrentContext` and the -`Surface` to `GlRenderer`. `ensure_init` on the REX3 thread then only -re-`make_current`s on its own thread — the "moving a context between threads" -pattern, which WGL explicitly supports. Rendering stays on REX3; only the -one-time pixel-format + initial bind moves. (The macOS main-thread -`window_handle()` constraint is already satisfied — the handle is captured in -`Ui::new` — so this is compatible with -`rules/macos/winit-030-window-handle-main-thread-only.md`.) +## Fix + +`Ui::new()` (main thread, before `Ui::run` starts pumping messages) now builds +the window `Surface` and binds the context to it once — `make_current` then +`make_not_current` — and hands both the `NotCurrentContext` and the `Surface` +to `GlRenderer` (`initial_surface: Option>`). That first +`make_current` is what installs the ICD's window subclass and populates its +per-HWND state, so by the time the event loop runs and a resize can reach the +subclass, the driver state exists. + +`ensure_init()` on the REX3 thread consumes `initial_surface` on the true first +frame and only `make_current`s it on its own thread — the "move a context +between threads" handoff, which WGL supports. Rendering stays entirely on REX3; +only the one-time pixel-format + initial bind moved. + +After a `stop()`/`start()` cycle (jitcheck checkpoint restore, `reset`, +snapshot load) `initial_surface` is already `None` — `ensure_init` then makes a +fresh surface on the refresh thread, as before. Safe by then: the pixel format +is already set on the HWND and the driver's window state was established at +startup, so a resize hitting the subclass is a read of valid state, not a null +deref. + +macOS/Linux: unaffected or improved. The macOS `window_handle()` main-thread +constraint was already satisfied (handle captured in `Ui::new` — +`rules/macos/winit-030-window-handle-main-thread-only.md`); Linux/GLX moving +`create_window_surface` + first `make_current` onto the window-creating thread +lines up with the proprietary-NVIDIA concern noted on the +`not_current_context` field. diff --git a/src/ui.rs b/src/ui.rs index 8ba2995..b9b9e15 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -96,6 +96,17 @@ struct GlRenderer { // that validate against the drawable/FBConfig being issued from a thread // other than the one that created the X11 connection/window. not_current_context: Option, + // The window surface, also built on the main thread in Ui::new() and bound + // once there (make_current → make_not_current) before the event loop runs. + // On Windows that first bind is what installs the GL ICD's window subclass + // and populates its per-HWND state; doing it lazily on the refresh thread + // while the event loop is already dispatching a startup resize raced the + // half-initialised driver (AMD atio6axx.dll null read — issue #94). + // `ensure_init()` consumes this on the true first frame; on a stop()/start() + // respawn it is already `None` and a fresh surface is made on the refresh + // thread (safe by then — the driver's window state exists). + // See rules/gui/windows-silent-exit-0xc000041d.md. + initial_surface: Option>, // Captured on the main thread in Ui::new(): winit 0.30 macOS returns // HandleError::Unavailable from window_handle() on any other thread, and // init_gl() runs on the refresh thread. @@ -170,17 +181,24 @@ impl GlRenderer { let not_current_gl_context = self.not_current_context.take() .expect("GL context missing on the true first ensure_init() call — construction bug in Ui::new()"); - let size = self.window.inner_size(); - let attrs = SurfaceAttributesBuilder::::new().build( - self.raw_window_handle, - NonZeroU32::new(size.width.max(1)).unwrap(), - NonZeroU32::new(size.height.max(1)).unwrap(), - ); - let gl_surface = unsafe { - gl_display - .create_window_surface(&self.gl_config, &attrs) - .unwrap() - }; + // Normal boot: the surface was created and bound once on the main + // thread in Ui::new() — reuse it. Only after a stop()/start() cycle + // (jitcheck checkpoint restore, `reset`, snapshot load) is it already + // gone, and then a fresh one on this thread is fine: the driver's + // per-HWND GL state was established at startup. + let gl_surface = self.initial_surface.take().unwrap_or_else(|| { + let size = self.window.inner_size(); + let attrs = SurfaceAttributesBuilder::::new().build( + self.raw_window_handle, + NonZeroU32::new(size.width.max(1)).unwrap(), + NonZeroU32::new(size.height.max(1)).unwrap(), + ); + unsafe { + gl_display + .create_window_surface(&self.gl_config, &attrs) + .unwrap() + } + }); let gl_context = not_current_gl_context.make_current(&gl_surface).unwrap(); @@ -801,6 +819,39 @@ impl Ui { .expect("failed to create a GL context under any requested version/profile"); eprintln!("iris: using GL tier {:?}", gl_tier); + // Build the window surface and bind the context to it *once, here* — on + // the main thread that owns the window, before `Ui::run` starts pumping + // messages. This is the one GL step that must not happen lazily on the + // refresh thread: `create_window_surface` sets the pixel format (which + // makes the Windows GL ICD subclass the window) and the first + // `make_current` populates the driver's per-HWND state. Doing that while + // the event loop is already delivering a startup resize left the AMD ICD + // dereferencing a null pointer inside its wndproc hook (issue #94). + // Rendering stays entirely on the refresh thread — only this bind moves. + let (not_current_context, initial_surface) = { + let sz = window.inner_size(); + let attrs = SurfaceAttributesBuilder::::new().build( + raw_window_handle, + NonZeroU32::new(sz.width.max(1)).unwrap(), + NonZeroU32::new(sz.height.max(1)).unwrap(), + ); + let surface = unsafe { + gl_display + .create_window_surface(&gl_config, &attrs) + .expect("failed to create the GL window surface on the main thread") + }; + let current = not_current_context + .make_current(&surface) + .expect("failed to make the GL context current during init"); + // Hand the context straight back to `NotCurrentContext` so the + // refresh thread can `make_current` it on its own thread (a GL + // context is current on at most one thread at a time). + let not_current = current + .make_not_current() + .expect("failed to release the GL context after init"); + (not_current, surface) + }; + let window_size = Arc::new(Mutex::new(None)); let resize_request = Arc::new(Mutex::new(None)); // Seed with the Indy's default 1280×1024; the render thread republishes @@ -820,6 +871,7 @@ impl Ui { window: window.clone(), gl_config, not_current_context: Some(not_current_context), + initial_surface: Some(initial_surface), raw_window_handle, gl_tier, window_size: window_size.clone(),