Repository navigation
[patch] Run window teardown when the run loop throws - #426
Merged
Merged
Conversation
An exception from a consumer's OnRender or OnUpdate callback propagates straight out of window.Run(). RunWindowLoop called TeardownWindow on the line after Run(), so on that path it never ran: the window was left undisposed and the static field still pointed at it, which also made a later Start() throw "Application is already running." TeardownWindow was also missing the two font cleanups that the Closing handler did. Closing does not fire on a faulted loop, so the pinned font GCHandles and the AllocHGlobal buffers leaked on both the blocking and the embedded paths - the embedded session already had its finally, but it converged on a teardown that was short two steps. Factor the five cleanups into CleanupWindowResources and have all three paths - the Closing handler, and TeardownWindow for the blocking and embedded loops - go through it, then put RunWindowLoop's teardown in a finally. Every call in the sequence is idempotent, which a normal close relies on: it runs the handler and then the teardown. Fixes #406 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011rsegt5ct1Cg7M8ffSGgUi
|
This was referenced Sep 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Fixes #406
What was wrong
An exception from a consumer's
OnRender/OnUpdatecallback propagates straight out ofwindow.Run(). Two gaps meant that path skipped teardown:RunWindowLoophad notry/finally.TeardownWindow()sat on the line afterwindow!.Run(), so a throw skipped it entirely — the window was left undisposed andImGuiApp.windowstill pointed at the orphaned one, which also made a laterStart()throw "Application is already running."TeardownWindowwas two steps short of theClosinghandler. It ranCleanupController/CleanupInputContext/CleanupOpenGLbut notCleanupPinnedFontData()orFontHelper.CleanupCustomFonts().Closingdoesn't fire on a faulted loop, so the pinned fontGCHandles and theMarshal.AllocHGlobalbuffers leaked — on the embedded path too, which already had itsfinallybut converged on the shorter sequence.This matters for hosts that catch the exception and keep running (embedding hosts, plugin hosts, harnesses that restart sessions): each faulted session leaked a GL context, controller, input context and pinned native memory.
What changed
ImGui.App/ImGuiApp.cs:CleanupWindowResources(), per the issue's own suggestion, and pointed all three paths at it — theClosinghandler, andTeardownWindow()for the blocking and embedded loops. Three call sites converging on one implementation is what stops a path drifting short again.TeardownWindow()now calls it, then disposes and clearswindowas before.RunWindowLoop()does its teardown in afinally.Every call in the sequence is idempotent, which a normal close relies on: it runs the
Closinghandler and then the teardown.No change was needed in
ImGuiApp.Embedded.cs—EmbeddedSession.RunLoopalready had thefinally; it picks up the font cleanups through the shared method.Tests
Two tests added to
tests/ImGui.App.Tests/WindowTeardownTests.cs:RunWindowLoopTearsDownWhenTheLoopThrows— a mock window whoseRun(Action)throws. Asserts the window and input context are disposed, the static fields are cleared (soStart()is legal again), and that the consumer's exception still reaches the caller unchanged rather than being swallowed by the teardown.TeardownWindowReleasesPinnedFontMemory— seeds a pinnedGCHandleand anAllocHGlobalbuffer and asserts teardown drains both lists. This covers the embedded path as well, since it ends in the same method.Verified both fail on the pre-fix source and pass with it (source change stashed, rebuilt, re-run): 2 failed / 436 passed before, 438 passed after.
Regression slice, all green on Linux:
ImGui.App.TestsImGui.App.Testing.TestsImGui.Widgets.TestsImGui.Popups.TestsImGui.Color.Testsdotnet build ImGui.slnsucceeds with 0 warnings.🤖 Generated with Claude Code
https://claude.ai/code/session_011rsegt5ct1Cg7M8ffSGgUi
Generated by Claude Code