Fix Patch Manager search-handle leaks causing unbounded CPU growth - #275
Open
kartun83 wants to merge 2 commits into
Open
Fix Patch Manager search-handle leaks causing unbounded CPU growth#275kartun83 wants to merge 2 commits into
kartun83 wants to merge 2 commits into
Conversation
updateStateAsync() only cancelled its datasource-lookup search on the match-found path; when no match was found the Search stayed in DB::m_searches forever. Since DB::updateSearches() rescans every entry in m_searches against every newly loaded patch on each bank load/patch change, leaked searches make every subsequent load do strictly more work, causing CPU usage to climb with the number of banks loaded/patches switched and never recover.
cancelSearch() always inserted the handle into m_cancelledSearches, but that set is only ever pruned inside executeSearch's per-datasource cancellation check, which never runs for a search that has already completed. Since practically every Patch Manager UI interaction (tree/list rebuilds, filters, patch selection) calls cancelSearch() on an already-finished search, the handle was leaked permanently. Only track the handle as cancelled when the search hasn't completed yet, so it can still be picked up by the pending-cancellation check.
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
Two related leaks in the Patch Manager's search-handle bookkeeping (
pluginLib::patchDB::DB) that cause plugin host CPU usage to climb after loading multiple banks / switching many patches, and never recover:PatchManager::updateStateAsync()only calledcancelSearch()on the "found a datasource match" path; the "no match found" early return skipped it, so theSearchstayed inDB::m_searchesforever. SinceDB::updateSearches()rescans every entry inm_searches(including leaked ones) against every newly loaded patch on each subsequent bank load/patch change, leaked searches make every future load do strictly more work, with cost proportional to cumulative history rather than current state.DB::cancelSearch()always inserted the handle intom_cancelledSearches, but that set is only ever pruned insideexecuteSearch()'s per-datasource cancellation check, which never runs for a search that has already completed — the common case for essentially every Patch Manager UI interaction (tree/list rebuilds, filtering, patch selection). This is a genuine unbounded memory leak, distinct in character from the first (insert/find/erase only, never iterated, so not itself a CPU driver).Both were found while investigating Bitwig host CPU climbing to 150-170% and never dropping back after loading several banks and switching patches in Osirus/OsTIrus.
Changes
updateStateAsync(): dispatchcancelSearch(handle)on the empty-results path too, mirroring the already-correct success path.cancelSearch(): only track a handle inm_cancelledSearchesif the search hasn't completed yet, since a completed search will never be looked at again.Test plan
jucePluginLib,jucePluginEditorLibtargets, Xcode/macOS)