From bdb28f33dc111d5acc471a1342e27dbb3ffe8429 Mon Sep 17 00:00:00 2001 From: Dominik Behr Date: Tue, 8 Sep 2026 08:48:23 -0700 Subject: [PATCH] ui: create the GL window surface + first make_current on the main thread issue #94: on some AMD machines the CLI died during startup with a null read inside atio6axx.dll (the AMD OpenGL ICD) on the main thread, inside a window procedure. The ICD subclasses the GL window when SetPixelFormat is called; its wndproc hook then runs on the window-owning (main) thread. iris was doing SetPixelFormat + the first wglMakeCurrent lazily on the REX3 refresh thread, in GlRenderer::ensure_init - so a startup resize could reach the freshly-installed subclass before wglMakeCurrent had populated the driver's per-HWND state. Ui::new() now builds the Surface and binds the context once (make_current -> make_not_current) on the main thread, before Ui::run starts pumping messages, and hands both the NotCurrentContext and the Surface (initial_surface) to GlRenderer. ensure_init() consumes the surface on the true first frame and only make_current()s it on the refresh thread - the "move a context between threads" handoff. Rendering stays entirely on REX3; only the one-time bind moved. After a stop()/start() cycle (jitcheck checkpoint restore, reset, snapshot load) initial_surface is None and ensure_init makes a fresh surface on the refresh thread as before - safe by then, the pixel format is set and the driver's window state exists. mac/Linux unaffected (macOS window_handle() is already captured on the main thread; Linux/GLX benefits - the drawable calls now happen on the window-creating thread, matching the proprietary-NVIDIA note on not_current_context). Verified on Windows: boots clean, and `reset` (exercises the fresh-surface path) does not crash. Co-Authored-By: Claude Sonnet 5 --- rules/gui/windows-silent-exit-0xc000041d.md | 41 ++++++++---- src/ui.rs | 74 ++++++++++++++++++--- 2 files changed, 91 insertions(+), 24 deletions(-) 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(),