[Linux] Cancel active frame wait and defer GTK window destruction during multi-window teardown by jtmcdole · Pull Request #193126 · flutter/flutter

jtmcdole · 2026-09-21T21:44:48Z

When tearing down an active secondary window on Linux:

Calling gtk_widget_destroy() synchronously from a Dart platform task running inside fl_view_renderer_opengl_draw() -> wait_for_frame() -> fl_task_runner_wait() unrealized the GdkWindow and freed the GtkWindow while gtk_widget_draw_internal() was still active on the C call stack. Add fl_view_begin_destroy() and fl_view_renderer_opengl_cancel_wait() to abort wait_for_frame() immediately, and schedule gtk_widget_destroy() on the GLib main loop in fl_view_monitor_dispose() once the draw stack unwinds.
In BaseWindowControllerLinux.destroy(), unreference FlViewMonitor and FlWindowMonitor before closing their Dart NativeCallables so that GTK signals emitted during teardown do not invoke closed callbacks.
Suppress listener notifications in _ToplevelWindowControllerLinux once _destroyed is true to prevent window manager signals (such as notify::is-active) from triggering rebuilds on destroyed views.

sequenceDiagram
    participant Dart as Dart Isolate (GTK Thread)
    participant GTK as GTK3 / GLib Main Loop (GTK Thread)
    participant Raster as Flutter Engine Raster Thread

    rect
    Note over GTK,Raster: Phase 2: Frame Presentation & Re-entrant Draw Wait
    Raster->>Raster: get_renderable(view_id) [acquires g_object_ref(FlView)]
    Raster->>Raster: present_layers(): lock frame_mutex, composite into self->frame
    Raster->>GTK: fl_task_runner_stop_wait() + g_idle_add(redraw_cb)
    Raster->>Raster: g_object_unref(FlView)
    GTK->>GTK: gtk_widget_draw_internal() -> fl_view_renderer_opengl_draw()
    GTK->>GTK: wait_for_frame() -> fl_task_runner_wait() [pumps Dart tasks while draw is on stack!]
    end

    rect
    Note over Dart,GTK: Phase 3: Window Destroy Called Inside fl_task_runner_wait()
    Dart->>GTK: controller.destroy() -> _viewMonitor.unref()
    GTK->>GTK: fl_view_monitor_dispose() -> fl_view_begin_destroy()
    GTK->>GTK: fl_view_renderer_opengl_cancel_wait(): sets self->destroyed = TRUE & wakes task_runner
    GTK->>GTK: Queues g_idle_add_full(G_PRIORITY_HIGH, destroy_toplevel_cb, g_object_ref(toplevel))
    GTK->>GTK: fl_window_monitor_dispose(): disconnects GTK signals
    Dart->>Dart: _notifyWindowDestroyed() -> dialogController.value = null
    GTK->>GTK: wait_for_frame() resumes, sees self->destroyed == TRUE, breaks immediately!
    GTK->>GTK: fl_view_renderer_opengl_draw() returns FALSE cleanly to gtk_widget_draw_internal()
    end

    rect
    Note over GTK,Raster: Phase 4: Deferred Unrealize, Dispose & Finalize
    GTK->>GTK: destroy_toplevel_cb() runs on clean main loop stack -> gtk_widget_destroy(toplevel)
    GTK->>GTK: cleanup_opengl_resources(): locks frame_mutex, clears render_context (if current), frees compositor & frame
    GTK->>GTK: fl_view_finalize() & fl_view_renderer_opengl_finalize(): frees task_runner & frame_mutex
    end

fixes: #192653
Part of stack #193534.

Pre-launch Checklist

gemini-code-assist

Code Review

This pull request updates the .ci.yaml configuration file by adding the xvfb: "1" property to a target's properties. I have no feedback to provide.

flutter-dashboard · 2026-09-21T21:56:02Z

This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

jtmcdole · 2026-09-22T15:34:28Z

Gemini suggested changes (investigating):

Linux windowing_test in .ci.yaml was missing xvfb: "1", causing the GTK application to abort immediately on startup in headless CI because DISPLAY was unset.
With xvfb: "1" enabled, creating a secondary FlView (WindowControllerLinux) while an existing FlView is rendering on X11 crashes with:

Gdk-ERROR **: The program 'com.example.windowing_test' received an X Window System error.
The error was 'BadAccess (attempt to access private resource denied)'.
(Details: serial 321 error_code 10 request_code 150 (GLX) minor_code 26)

After #192137 (45f4e3db32b), fl_view_renderer_opengl_draw() deliberately leaves the view's GdkGLContext (which uses GLX on X11) current on the GTK main thread. When a secondary FlView is realized on the GTK main thread, fl_view_renderer_opengl_realize() calls fl_compositor_opengl_new(), which binds the engine's EGL context via eglMakeCurrent().

Under libglvnd, calling eglMakeCurrent on the GTK main thread while a GLX context (GdkGLContext) is current overwrites libglvnd's thread-local current context pointer with the EGL context without unbinding the GLX context or sending an X_GLXMakeContextCurrent(None) request to the X server, causing client-server context state desynchronization. When GTK subsequently calls gdk_gl_context_make_current() (glXMakeContextCurrent, GLX minor_code 26), libGLX sends oldContextTag = 0 for a context the X server still considers active, triggering BadAccess.

Additionally, fl_compositor_opengl_new() called fl_opengl_manager_make_current() (which binds the raster thread's render_context rather than platform_context) and did not release the EGL context on completion, leaving an EGL context bound on the GTK main thread for secondary views (view_id != kFlutterImplicitViewId).

API References

gdk_gl_context_clear_current:

"Clears the current GdkGLContext. Any OpenGL calls after this function returns will be ignored until gdk_gl_context_make_current() is called."
eglMakeCurrent:

"If the calling thread already has a current rendering context of the same client API type as ctx, then that context is flushed and marked as no longer current."
"To release the current context without assigning a new one, set ctx to EGL_NO_CONTEXT and set draw and read to EGL_NO_SURFACE."
glXMakeContextCurrent:

"BadAccess is generated if ctx was current to another thread at the time glXMakeContextCurrent was called."

Solution

Call gdk_gl_context_clear_current() in fl_opengl_manager_make_platform_current() before calling eglMakeCurrent() so any active GdkGLContext is cleanly unbound on both GdkDisplay and the X server before
binding the EGL platform_context.
Update fl_compositor_opengl_new() to use fl_opengl_manager_make_platform_current() instead of fl_opengl_manager_make_current(), and call fl_opengl_manager_clear_current() after shader setup, compositor
initialization, and shader disposal.
Add xvfb: "1" to Linux windowing_test in .ci.yaml.

jtmcdole · 2026-09-22T15:54:08Z

Running in presubmit: https://ci.chromium.org/ui/p/flutter/builders/try/Linux%20windowing_test/1/overview

jtmcdole · 2026-09-22T16:30:42Z

This still failed - but the X11 tests are passing. Now the failure is in 2 tests that require a window manager on the headless bot.

@robert-ancell - can you help out here? This is our last bringup:true test on the dashboard!

gemini-code-assist

Code Review

This pull request addresses OpenGL context state desynchronization under libglvnd by clearing the GdkGLContext before making the platform context current. Key changes include updating fl_opengl_manager_make_platform_current to clear the current GdkGLContext, clearing contexts in the compositor and shader, and adding corresponding unit tests. Feedback highlights a temporary DO NOT SUBMIT flag in .ci.yaml, warns of potential side effects from clearing the OpenGL context inside the setup_shader helper, and identifies a potential memory leak in the test due to overwriting the fixture's compositor member.

gemini-code-assist · 2026-09-22T22:17:00Z

  glBufferData(GL_ARRAY_BUFFER, sizeof(vertex_data), vertex_data,
               GL_STATIC_DRAW);
+
+  fl_opengl_manager_clear_current(self->opengl_manager);


Clearing the OpenGL context inside a helper function like setup_shader can lead to unexpected side effects if the caller expects the context to remain current, or if the caller is responsible for managing the context lifecycle. It is generally cleaner and safer to have the caller (which made the context current) also clear it.

References

Optimize for readability and maintainability by keeping context management balanced and predictable. ^(link)

robert-ancell · 2026-09-23T01:49:44Z

This still failed - but the X11 tests are passing. Now the failure is in 2 tests that require a window manager on the headless bot.

@robert-ancell - can you help out here? This is our last bringup:true test on the dashboard!

I think this CI is all Google specific? So I'm not sure how it works. Looking at engine/src/flutter/testing/xvfb.py shows it runs openbox if it is installed, so this might be just a case of making sure devicelab has this available?

jtmcdole · 2026-09-23T03:00:12Z

This still failed - but the X11 tests are passing. Now the failure is in 2 tests that require a window manager on the headless bot.
@robert-ancell - can you help out here? This is our last bringup:true test on the dashboard!

I think this CI is all Google specific? So I'm not sure how it works. Looking at engine/src/flutter/testing/xvfb.py shows it runs openbox if it is installed, so this might be just a case of making sure devicelab has this available?

Thanks, I'll check in the morning if openbox is available or can be made available on the machines. One step closer to no bringup:true.

jtmcdole · 2026-09-23T22:12:05Z

Adding some code to bring up openbox got the resizing tests to work, but there are most tests that fail after. This is a spicy test group....

jtmcdole · 2026-09-23T23:06:01Z

Hit some built in flakes. (2 failures -> pass). still investigating but my longer term view is:

break this up. xvfb:"1" just to get the build building
write an issue about the other failures and pass it to someone that really knows GTK.

jtmcdole · 2026-09-24T01:12:21Z

OK - trying to deflake this test by running it 16 times. I will say that this is very AI-agent delivered.

#	Build ID	Status	First Attempt (`flaky`)	`CRITICAL` / `ERROR`	`WARNING` / Notes
1	`8669823180179404913`	`SUCCESS`	✅ `flaky: false` (1/1)	None	Clean
2	`8669823178675407009`	`SUCCESS`	✅ `flaky: false` (1/1)	None	`WARNING: Timed out waiting for OpenGL frame of size 800x600 (have 1x1)`
3	`8669823177034190529`	`SUCCESS`	✅ `flaky: false` (1/1)	None	`WARNING: Timed out waiting for OpenGL frame of size 800x600 (have 1x1)`
4	`8669821346653741361`	`SUCCESS`	⚠️ `flaky: true` (1 failure, passed on 2nd)	`CRITICAL: fl_engine_get_mouse_cursor_handler(nullptr)` (`FL_IS_ENGINE(self)`)	Attempt 1 hit `cursor_changed_cb()` after `fl_view_begin_destroy()` cleared `self->engine`; Attempt 2 passed cleanly
5	`8669821343177405409`	`SUCCESS`	✅ `flaky: false` (1/1)	None	Clean
6	`8669821340563019089`	`SUCCESS`	⚠️ `flaky: true` (1 failure, passed on 2nd)	`CRITICAL: fl_engine_get_mouse_cursor_handler(nullptr)` (`FL_IS_ENGINE(self)`)	Attempt 1 hit `cursor_changed_cb()` after `fl_view_begin_destroy()` cleared `self->engine`; Attempt 2 passed cleanly
7	`8669820751505996529`	`SUCCESS`	✅ `flaky: false` (1/1)	None	Clean
8	`8669820750073104129`	`SUCCESS`	✅ `flaky: false` (1/1)	None	`WARNING: Timed out waiting for OpenGL frame of size 800x600 (have 1x1)`
9	`8669820748717472801`	`SUCCESS`	✅ `flaky: false` (1/1)	None	Clean
10	`8669820747100312961`	`SUCCESS`	✅ `flaky: false` (1/1)	None	Clean
11	`8669820745155647729`	`SUCCESS`	✅ `flaky: false` (1/1)	None	Clean
12	`8669820743918593057`	`SUCCESS`	✅ `flaky: false` (1/1)	None	`WARNING: Timed out waiting for OpenGL frame of size 800x600 (have 1x1)`
13	`8669820742414882657`	`SUCCESS`	✅ `flaky: false` (1/1)	None	`WARNING: Timed out waiting for OpenGL frame of size 800x600 (have 1x1)`
14	`8669820740988831649`	`SUCCESS`	✅ `flaky: false` (1/1)	None	`WARNING: Timed out waiting for OpenGL frame of size 800x600 (have 1x1)`
15	`8669820739613289569`	`SUCCESS`	✅ `flaky: false` (1/1)	None	Clean
16	`8669820738260266593`	`SUCCESS`	✅ `flaky: false` (1/1)	None	Clean

Summary

Overall Build Status: 16 / 16 (100%) SUCCESS.
First-Attempt Pass Rate: 14 / 16 (87.5%) flaky: false (16 passes out of 18 total test executions).
Zero Gdk-CRITICAL / Gtk-CRITICAL assertions: The GDK_IS_WINDOW / GTK_IS_WIDGET use-after-free during GtkWidget::draw is completely eliminated across
all 18 executions.
Follow-up fix for the 2 retries (#4 and #6): Both retries hit cursor_changed_cb() after fl_view_begin_destroy() cleared self->engine before
destroy_toplevel_cb() disposed FlView; fixed in fl_view.cc by keeping g_clear_object(&self->engine) in fl_view_dispose() and guarding cursor_changed_cb().

jtmcdole · 2026-09-24T02:18:40Z

Every single one—50 out of 50 (100.0%)—passed on the first attempt (flaky: false) with zero retries and zero CRITICAL / ERROR messages.

Well, that's nice.

jtmcdole · 2026-09-24T23:26:39Z

+      wmProcess = await startProcess('openbox', const <String>[
+        '--sm-disable',
+        '--startup',
+        'echo $readyMarker',
+      ]);


openbox is the windowmanager used by xvfb.py testing in the engine; it is available on the luci bots.

jtmcdole · 2026-09-24T23:27:37Z


  // Determine once whether glBlitFramebuffer is available on this driver.
-  fl_opengl_manager_make_current(opengl_manager);
+  fl_opengl_manager_make_platform_current(opengl_manager);


This looks like a bug that was using the wrong context... switched to platform context instead

jtmcdole · 2026-09-25T00:54:03Z

@robert-ancell / @mattkae - I believe this is ready for review. I am not a GTK expert though and would appreciate feedback. I ran ~100 tests on our CI and they were all non-flakey with these changes.

jtmcdole · 2026-09-28T17:29:30Z

force push to merge conflicting changes at master.

robert-ancell

There's a lot going on in this PR. I think cherry picking out the individual fixes for the Linux embedder is the safer way to be sure on these and make them easier to understand and bisect on if we have future issues. If you don't want to do that I'm happy to do those and then this PR should slim right down to just enabling the tests on CI.

jtmcdole · 2026-09-29T22:31:00Z

There's a lot going on in this PR. I think cherry picking out the individual fixes for the Linux embedder is the safer way to be sure on these and make them easier to understand and bisect on if we have future issues. If you don't want to do that I'm happy to do those and then this PR should slim right down to just enabling the tests on CI.

I have the three commits that I could stack. I'll do that and sent them out individually.

…ing multi-window teardown When tearing down an active secondary window on Linux: 1. Calling gtk_widget_destroy() synchronously from a Dart platform task running inside fl_view_renderer_opengl_draw() -> wait_for_frame() -> fl_task_runner_wait() unrealized the GdkWindow and freed the GtkWindow while gtk_widget_draw_internal() was still active on the C call stack. Add fl_view_begin_destroy() and fl_view_renderer_opengl_cancel_wait() to abort wait_for_frame() immediately, and schedule gtk_widget_destroy() on the GLib main loop in fl_view_monitor_dispose() once the draw stack unwinds. 2. In BaseWindowControllerLinux.destroy(), unreference FlViewMonitor and FlWindowMonitor before closing their Dart NativeCallables so that GTK signals emitted during teardown do not invoke closed callbacks. 3. Suppress listener notifications in _ToplevelWindowControllerLinux once _destroyed is true to prevent window manager signals (such as notify::is-active) from triggering rebuilds on destroyed views.

mattkae

Small points, but looks good overall! Deferring to @robert-ancell for the final review

mattkae · 2026-10-09T14:43:18Z

+  if (FL_IS_VIEW_RENDERER_OPENGL(self->renderer)) {
+    fl_view_renderer_opengl_cancel_wait(
+        FL_VIEW_RENDERER_OPENGL(self->renderer));
+  }


Instead of this check, should we just define fl_view_renderer_cancel_wait on each renderer, and then have it be a cancel for the GL renderer? Just thinking it might be a better pattern instead of basically static_casting

mattkae · 2026-10-09T15:01:23Z

+    final VoidCallback? onDestroy = _onDestroy;
+    _onDestroy = null;
+    onDestroy?.call();


Can you leave a comment as to why you're capturing this first?

flutter-dashboard Bot added the CICD Run CI/CD label Sep 21, 2026

gemini-code-assist Bot reviewed Sep 21, 2026

View reviewed changes

jtmcdole requested a review from gaaclarke September 21, 2026 21:46

gaaclarke previously approved these changes Sep 21, 2026

View reviewed changes

jtmcdole marked this pull request as draft September 21, 2026 21:52

jtmcdole dismissed gaaclarke’s stale review via 3b559bb September 22, 2026 15:28

github-actions Bot added engine flutter/engine related. See also e: labels. platform-linux Building on or for Linux specifically a: desktop Running on desktop team-linux Owned by the Linux platform team labels Sep 22, 2026

jtmcdole added CICD Run CI/CD and removed CICD Run CI/CD labels Sep 22, 2026

jtmcdole requested a review from robert-ancell September 22, 2026 16:30

jtmcdole marked this pull request as ready for review September 22, 2026 22:15

jtmcdole requested a review from a team as a code owner September 22, 2026 22:15

gemini-code-assist Bot reviewed Sep 22, 2026

View reviewed changes

jtmcdole force-pushed the windowing_test_linux branch from 7c91ba9 to 0e0e5e6 Compare September 23, 2026 22:06

github-actions Bot added the framework flutter/packages/flutter repository. See also f: labels. label Sep 23, 2026

jtmcdole commented Sep 23, 2026

View reviewed changes

Comment thread dev/devicelab/bin/tasks/windowing_test_linux.dart Resolved

jtmcdole commented Sep 23, 2026

View reviewed changes

Comment thread engine/src/flutter/shell/platform/linux/fl_opengl_manager.cc Resolved

jtmcdole force-pushed the windowing_test_linux branch from 498dedd to 5353edd Compare September 24, 2026 22:45

jtmcdole changed the title ~~Add xvfb: "1" to Linux windowing_test~~ [Linux] Fix multi-window OpenGL/GTK lifecycle and enable Xvfb/openbox for windowing_test Sep 24, 2026

jtmcdole commented Sep 24, 2026

View reviewed changes

jtmcdole requested a review from mattkae September 25, 2026 00:52

jtmcdole force-pushed the windowing_test_linux branch from 5353edd to 91c7c66 Compare September 28, 2026 17:28

robert-ancell requested changes Sep 28, 2026

View reviewed changes

jtmcdole changed the base branch from master to windowing_test_egl_desync September 29, 2026 22:50

jtmcdole added this pull request to stack #193531 September 29, 2026 22:50

jtmcdole changed the title ~~[Linux] Fix multi-window OpenGL/GTK lifecycle and enable Xvfb/openbox for windowing_test~~ [Linux] Synchronize multi-window teardown across GTK draw and raster threads Sep 29, 2026

jtmcdole force-pushed the windowing_test_linux branch from 91c7c66 to d2df67d Compare September 29, 2026 23:02

jtmcdole removed this pull request from stack #193531 September 29, 2026 23:02

jtmcdole changed the base branch from windowing_test_egl_desync to windowing_renderer_sync September 29, 2026 23:03

jtmcdole added this pull request to stack #193534 September 29, 2026 23:03

jtmcdole changed the title ~~[Linux] Synchronize multi-window teardown across GTK draw and raster threads~~ [Linux] Cancel active frame wait and defer GTK window destruction during multi-window teardown Sep 29, 2026

jtmcdole force-pushed the windowing_test_linux branch from d2df67d to c27b022 Compare September 29, 2026 23:14

jtmcdole force-pushed the windowing_test_linux branch from c27b022 to 0b76221 Compare October 1, 2026 22:52

jtmcdole force-pushed the windowing_test_linux branch from 0b76221 to 34067b1 Compare October 2, 2026 03:07

Axiaobo7788 mentioned this pull request Oct 5, 2026

[Linux][Desktop] Tracking issue for frame pacing and display synchronization gaps affecting animation smoothness #191245

Open

mattkae reviewed Oct 9, 2026

View reviewed changes

Conversation

jtmcdole commented Sep 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

flutter-dashboard Bot commented Sep 21, 2026

Uh oh!

jtmcdole commented Sep 22, 2026

API References

Solution

Uh oh!

jtmcdole commented Sep 22, 2026

Uh oh!

jtmcdole commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

gemini-code-assist Bot Sep 22, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

robert-ancell commented Sep 23, 2026

Uh oh!

jtmcdole commented Sep 23, 2026

Uh oh!

jtmcdole commented Sep 23, 2026

Uh oh!

jtmcdole commented Sep 23, 2026

Uh oh!

Uh oh!

Uh oh!

jtmcdole commented Sep 24, 2026

Summary

Uh oh!

jtmcdole commented Sep 24, 2026

Uh oh!

jtmcdole Sep 24, 2026

Choose a reason for hiding this comment

Uh oh!

jtmcdole Sep 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jtmcdole commented Sep 25, 2026

Uh oh!

jtmcdole commented Sep 28, 2026

Uh oh!

robert-ancell left a comment

Choose a reason for hiding this comment

Uh oh!

jtmcdole commented Sep 29, 2026

Uh oh!

mattkae left a comment

Choose a reason for hiding this comment

Uh oh!

mattkae Oct 9, 2026

Choose a reason for hiding this comment

Uh oh!

mattkae Oct 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jtmcdole commented Sep 21, 2026 •

edited

Loading

jtmcdole commented Sep 22, 2026 •

edited

Loading

jtmcdole Sep 24, 2026 •

edited

Loading