[Linux] Don't clear the OpenGL context when a frame has been drawn - #192137
Conversation
The draw handler made a context current to draw the frame with, then cleared it, leaving no context current at all while GTK finished the frame. GTK makes the context it needs current before using OpenGL, so this doesn't need to be cleared. But GTK doesn't check that making its context current succeeded, and if it fails it carries on and uses OpenGL with no context current, which crashes. Leaving the context current means GTK still has one to fall back on when this happens, so it doesn't crash. See flutter#191775
|
It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
There was a problem hiding this comment.
Code Review
This pull request removes the call to gdk_gl_context_clear_current from the drawing loop in fl_view_renderer_opengl.cc to prevent GTK from crashing due to an unbound context. Feedback highlights that leaving the context bound indefinitely could lead to resource leaks or dangling pointer crashes, and recommends clearing the context during the disposal or destruction phase of the renderer.
I am having trouble creating individual review comments. Click here to see my feedback.
engine/src/flutter/shell/platform/linux/fl_view_renderer_opengl.cc (184-186)
Removing gdk_gl_context_clear_current() from the draw loop means self->render_context will remain bound as the current GL context on the GTK main thread indefinitely.\n\nWhen the renderer or view is destroyed, self->render_context is unreferenced. If it is still the current context on the thread, this can lead to:\n1. Resource Leaks: The GL driver or GDK may keep a reference to the current context, preventing it and its associated resources from being freed.\n2. Dangling Pointer / Crashes: If the context is freed but remains set as current in the thread's thread-local storage, subsequent GL operations or context switches on the main thread may dereference a dangling pointer and crash.\n\nTo prevent this, please ensure that gdk_gl_context_clear_current() is called during the disposal/destruction phase of FlViewRendererOpengl (e.g., in fl_view_renderer_opengl_dispose or fl_view_renderer_opengl_finalize).
mattkae
left a comment
There was a problem hiding this comment.
One piece of feedback to consider!
| // Leaving this context current gives GTK one to fall back on. It is created | ||
| // on the same window and shares with the context GTK would have used itself, | ||
| // so GTK can safely draw with it. |
There was a problem hiding this comment.
This feels like a weird, unexpected side effect for a caller potentially. Can this be documented on the method instead, that it will be leftover?
There was a problem hiding this comment.
Do you mean the documentation on fl_view_renderer_opengl_draw? This is a callback from GTK, not called by anything else. So the behaviour is specific to GTK and confirmed by inspecting the GTK source.
flutter/flutter@63170e9...b444e78 2026-09-08 katelovett@google.com Clean up lint errors in workflows (flutter/flutter#191987) 2026-09-08 32538273+ValentinVignal@users.noreply.github.com Remove no-shuffle tag from dropdown_test.dart (flutter/flutter#186398) 2026-09-08 32538273+ValentinVignal@users.noreply.github.com Remove no-shuffle from scrollbar test (flutter/flutter#186656) 2026-09-08 robert.ancell@canonical.com [tool] Let --local-engine reach the build system again (flutter/flutter#192361) 2026-09-08 bkonyi@google.com [flutter_tools] Safely remove web loading indicator on dart-app-ready (flutter/flutter#192233) 2026-09-08 robert.ancell@canonical.com [Linux] Don't clear the OpenGL context when a frame has been drawn (flutter/flutter#192137) 2026-09-08 chris@bracken.jp ci: update dartdoc from 9.0.6 to 9.0.9 (flutter/flutter#192405) 2026-09-07 50643541+Mairramer@users.noreply.github.com Add itemExtentBuilder support to ListView.separated (flutter/flutter#191157) 2026-09-07 engine-flutter-autoroll@skia.org Roll Skia from e1437642d259 to b3fa80a0e168 (5 revisions) (flutter/flutter#192362) 2026-09-07 engine-flutter-autoroll@skia.org Roll Skia from 2df865d9d369 to e1437642d259 (1 revision) (flutter/flutter#192360) 2026-09-07 engine-flutter-autoroll@skia.org Roll Skia from c7aa5512d148 to 2df865d9d369 (2 revisions) (flutter/flutter#192357) 2026-09-06 engine-flutter-autoroll@skia.org Roll Skia from db200e27a634 to c7aa5512d148 (2 revisions) (flutter/flutter#192351) 2026-09-06 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from _IgixhH4vEgdz9Oqn... to p8hHkUi8uWT9R3N4z... (flutter/flutter#192349) If this roll has caused a breakage, revert this CL and set the roller to dry run mode using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC boetger@google.com,stuartmorgan@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
…r#12791) flutter/flutter@63170e9...b444e78 2026-09-08 katelovett@google.com Clean up lint errors in workflows (flutter/flutter#191987) 2026-09-08 32538273+ValentinVignal@users.noreply.github.com Remove no-shuffle tag from dropdown_test.dart (flutter/flutter#186398) 2026-09-08 32538273+ValentinVignal@users.noreply.github.com Remove no-shuffle from scrollbar test (flutter/flutter#186656) 2026-09-08 robert.ancell@canonical.com [tool] Let --local-engine reach the build system again (flutter/flutter#192361) 2026-09-08 bkonyi@google.com [flutter_tools] Safely remove web loading indicator on dart-app-ready (flutter/flutter#192233) 2026-09-08 robert.ancell@canonical.com [Linux] Don't clear the OpenGL context when a frame has been drawn (flutter/flutter#192137) 2026-09-08 chris@bracken.jp ci: update dartdoc from 9.0.6 to 9.0.9 (flutter/flutter#192405) 2026-09-07 50643541+Mairramer@users.noreply.github.com Add itemExtentBuilder support to ListView.separated (flutter/flutter#191157) 2026-09-07 engine-flutter-autoroll@skia.org Roll Skia from e1437642d259 to b3fa80a0e168 (5 revisions) (flutter/flutter#192362) 2026-09-07 engine-flutter-autoroll@skia.org Roll Skia from 2df865d9d369 to e1437642d259 (1 revision) (flutter/flutter#192360) 2026-09-07 engine-flutter-autoroll@skia.org Roll Skia from c7aa5512d148 to 2df865d9d369 (2 revisions) (flutter/flutter#192357) 2026-09-06 engine-flutter-autoroll@skia.org Roll Skia from db200e27a634 to c7aa5512d148 (2 revisions) (flutter/flutter#192351) 2026-09-06 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from _IgixhH4vEgdz9Oqn... to p8hHkUi8uWT9R3N4z... (flutter/flutter#192349) If this roll has caused a breakage, revert this CL and set the roller to dry run mode using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC boetger@google.com,stuartmorgan@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
The draw handler made a context current to draw the frame with, then cleared it, leaving no context current at all while GTK finished the frame.
GTK makes the context it needs current before using OpenGL, so this doesn't need to be cleared. But GTK doesn't check that making its context current succeeded, and if it fails it carries on and uses OpenGL with no context current, which crashes.
Leaving the context current means GTK still has one to fall back on when this happens, so it doesn't crash.
See #191775