Fix zombie window leak by passing win pointer directly to finish_destroy_win - #47
Open
jEsuSdA wants to merge 1 commit into
Open
Fix zombie window leak by passing win pointer directly to finish_destroy_win#47jEsuSdA wants to merge 1 commit into
jEsuSdA wants to merge 1 commit into
Conversation
…roy_win finish_destroy_win() searched for the window by ID using a linear scan that required w->destroyed to be true. However, find_win() (used by destroy_win() and other callers) filters out destroyed windows. This meant that when a window was marked destroyed but not yet freed, finish_destroy_win() could never find it again via ID lookup, causing an infinite accumulation of zombie win structs in the window list. Change finish_destroy_win() to accept a win* pointer directly instead of a Window ID. This eliminates the problematic ID lookup entirely. The callers already have the win* pointer available (destroy_win() finds it via find_win() before marking destroyed, and destroy_callback() receives it as an argument). This is a minimal change: only the function signature and its two callers are touched. The list unlink logic remains a linear scan (to be replaced by O(1) unlink once the doubly-linked list PR is merged).
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.
Summary
Fixes a memory leak where destroyed windows accumulate indefinitely because
finish_destroy_win()cannot find them via ID lookup.Problem
finish_destroy_win()searched for the window by ID using a linear scan that requiredw->destroyed == True. However,find_win()(used bydestroy_win()and other callers) filters out destroyed windows:When
destroy_win()marks a window asdestroyedand then callsfinish_destroy_win(), the latter scans the list looking forw->destroyed == True. But iffind_win()was called in between (or if the window is accessed again), the destroyed window is skipped. More importantly,finish_destroy_win()itself searches by ID and requiresw->destroyed, but since the caller already has thewin*pointer, the search is redundant and fragile.The result: zombie
winstructs accumulate in the window list forever, causing memory growth and eventual slowdown over long sessions.Solution
Change
finish_destroy_win()to accept awin*pointer directly instead of aWindowID. The callers already have the pointer:destroy_win()finds it viafind_win()before marking destroyeddestroy_callback()receives it as an argumentThis eliminates the problematic ID lookup entirely.
Scope
fastcompmgr.cis touchedmake clean && makeproduces zero warnings, zero errorsNote
The list unlink logic remains a linear scan (to be replaced by O(1) unlink once the doubly-linked list PR is merged).