Skip to content

Fix: resolve memory leaks and remove debug info leakage#346

Closed
rm335 wants to merge 1 commit into
dwarvesf:developfrom
rm335:fix/memory-leaks-and-security-cleanup
Closed

Fix: resolve memory leaks and remove debug info leakage#346
rm335 wants to merge 1 commit into
dwarvesf:developfrom
rm335:fix/memory-leaks-and-security-cleanup

Conversation

@rm335

@rm335 rm335 commented Mar 13, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Remove debug print(keyCode) in GlobalKeybindingPreferences.description that leaked user hotkey data to the
    system console
  • Add removeObserver(self) in LauncherApplication/AppDelegate.applicationWillTerminate
    DistributedNotificationCenter does not use zeroing-weak references, so unremoved observers risk a crash on dealloc
  • Add deinit with removeObserver(self) to PreferencesViewController — observer registered in viewDidLoad had
    no cleanup path
  • Use [weak self] in two DispatchQueue.main.asyncAfter closures in StatusBarController to prevent retaining
    self beyond intended lifetime

Test plan

  • Verify global hotkey still works after setting it in Preferences
  • Open and close Preferences window multiple times, confirm no observer accumulation (use Instruments Allocations)
  • Confirm LauncherApplication terminates cleanly without console crash logs
  • Confirm auto-collapse timer still fires correctly after expanding the menu bar
  • Verify no keyCode output appears in Console.app during normal usage

sdenike added a commit to sdenike/hidden-revived that referenced this pull request Apr 23, 2026
Adds the non-overlapping pieces of huynguyenh's PR dwarvesf#335 on top of
rm335's PR dwarvesf#346. Handles the biggest leak source (NSLayoutConstraint
retention when NSImageViews were recreated 9-10 times per toggle).

- Deactivate NSLayoutConstraints before removing views in
  NSStackView.removeAllSubViews (prevents constraint retention of
  removed NSImageViews — primary leak source on macOS Sequoia/Tahoe)
- Extend StatusBarController.deinit to invalidate timer and remove
  the always-hidden NSStatusItem
- Change toggleStatusBarIfNeeded to only create the always-hidden
  status item when it doesn't already exist, instead of tearing down
  and recreating on every .alwayHideToggle notification

Combined with the widest-display collapse-length calc in dwarvesf#354 and the
bounded-length work in dwarvesf#344, this should resolve the runaway memory
growth reported in upstream dwarvesf#326, dwarvesf#336, dwarvesf#351, dwarvesf#352.

Co-Authored-By: huynguyenh <hoanghuy2908@gmail.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sdenike added a commit to sdenike/hidden-revived that referenced this pull request Apr 23, 2026
- Fix: resolve memory leaks and remove debug info leakage (PR dwarvesf#346, Rob Mulder)
- Fix comprehensive memory leaks (adapted from PR dwarvesf#335, huynguyenh)

Addresses the cumulative leak causing 2.89GB memory usage on macOS
Sequoia/Tahoe. Closes upstream dwarvesf#326, dwarvesf#336, dwarvesf#351, dwarvesf#352.
sdenike added a commit to sdenike/hidden-revived that referenced this pull request Apr 29, 2026
Adds the non-overlapping pieces of huynguyenh's PR dwarvesf#335 on top of
rm335's PR dwarvesf#346. Handles the biggest leak source (NSLayoutConstraint
retention when NSImageViews were recreated 9-10 times per toggle).

- Deactivate NSLayoutConstraints before removing views in
  NSStackView.removeAllSubViews (prevents constraint retention of
  removed NSImageViews — primary leak source on macOS Sequoia/Tahoe)
- Extend StatusBarController.deinit to invalidate timer and remove
  the always-hidden NSStatusItem
- Change toggleStatusBarIfNeeded to only create the always-hidden
  status item when it doesn't already exist, instead of tearing down
  and recreating on every .alwayHideToggle notification

Combined with the widest-display collapse-length calc in dwarvesf#354 and the
bounded-length work in dwarvesf#344, this should resolve the runaway memory
growth reported in upstream dwarvesf#326, dwarvesf#336, dwarvesf#351, dwarvesf#352.

Co-Authored-By: huynguyenh <hoanghuy2908@gmail.com>
sdenike added a commit to sdenike/hidden-revived that referenced this pull request Apr 29, 2026
- Fix: resolve memory leaks and remove debug info leakage (PR dwarvesf#346, Rob Mulder)
- Fix comprehensive memory leaks (adapted from PR dwarvesf#335, huynguyenh)

Addresses the cumulative leak causing 2.89GB memory usage on macOS
Sequoia/Tahoe. Closes upstream dwarvesf#326, dwarvesf#336, dwarvesf#351, dwarvesf#352.
tieubao added a commit that referenced this pull request Jun 11, 2026
Absorbs the still-relevant pieces of three community PRs that overlapped this branch, with authorship credit:

- collapse length re-applies to the live separator when displays change, and isCollapsed compares > btnHiddenLength so the state survives the recompute; collapse sizing uses frame.width x2 capped at the documented 10,000pt NSStatusItem maximum (from #354)
- NSLayoutConstraint.deactivate before removeFromSuperview in removeAllSubViews, plugging the tutorial-view rebuild leak (from #335)
- PreferencesViewController balances its prefsChanged observer in deinit (from #335/#346)

Co-authored-by: Laveez <niko.muukkonen1@gmail.com>
Co-authored-by: huynguyenh <hoanghuy2908@gmail.com>
Co-authored-by: Rob Mulder <hallo@robmulder.com>
@tieubao

tieubao commented Jun 11, 2026

Copy link
Copy Markdown
Member

The weak-self closure fixes and debug-print removal from this PR landed independently in #362/#363; your PreferencesViewController observer cleanup is absorbed into commit 8582e4b on #363 with a Co-authored-by credit. Thanks; we'll reconcile/close once the stack merges.

tieubao added a commit that referenced this pull request Jun 11, 2026
Absorbs the still-relevant pieces of three community PRs that overlapped this branch, with authorship credit:

- collapse length re-applies to the live separator when displays change, and isCollapsed compares > btnHiddenLength so the state survives the recompute; collapse sizing uses frame.width x2 capped at the documented 10,000pt NSStatusItem maximum (from #354)
- NSLayoutConstraint.deactivate before removeFromSuperview in removeAllSubViews, plugging the tutorial-view rebuild leak (from #335)
- PreferencesViewController balances its prefsChanged observer in deinit (from #335/#346)

Co-authored-by: Laveez <niko.muukkonen1@gmail.com>
Co-authored-by: huynguyenh <hoanghuy2908@gmail.com>
Co-authored-by: Rob Mulder <hallo@robmulder.com>
tieubao added a commit that referenced this pull request Jun 11, 2026
Absorbs the still-relevant pieces of three community PRs that overlapped this branch, with authorship credit:

- collapse length re-applies to the live separator when displays change, and isCollapsed compares > btnHiddenLength so the state survives the recompute; collapse sizing uses frame.width x2 capped at the documented 10,000pt NSStatusItem maximum (from #354)
- NSLayoutConstraint.deactivate before removeFromSuperview in removeAllSubViews, plugging the tutorial-view rebuild leak (from #335)
- PreferencesViewController balances its prefsChanged observer in deinit (from #335/#346)

Co-authored-by: Laveez <niko.muukkonen1@gmail.com>
Co-authored-by: huynguyenh <hoanghuy2908@gmail.com>
Co-authored-by: Rob Mulder <hallo@robmulder.com>
@tieubao

tieubao commented Jun 11, 2026

Copy link
Copy Markdown
Member

All pieces of this are now in develop: the weak-self fixes and debug-print removal landed via #362/#363 independently, and your PreferencesViewController observer cleanup is in commit cf66bfe with your Co-authored-by credit. Thanks; closing as absorbed.

@tieubao tieubao closed this Jun 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants