Skip to content

fix: disconnect FeatureIndicator settings handler + correct theme cleanup#521

Open
mayconrcmello wants to merge 1 commit intoforge-ext:mainfrom
mayconrcmello:fix/cleanup-signal-leaks-and-deadcode
Open

fix: disconnect FeatureIndicator settings handler + correct theme cleanup#521
mayconrcmello wants to merge 1 commit intoforge-ext:mainfrom
mayconrcmello:fix/cleanup-signal-leaks-and-deadcode

Conversation

@mayconrcmello
Copy link
Copy Markdown

Summary

Two small lifecycle fixes that surface during repeated enable/disable cycles (a common path during development, plus user-triggered prefs reloads or extension scans).

1. FeatureIndicator leaks a settings signal handler

indicator.js connects to extension.settings.connect("changed", …) in the constructor without storing the handler id. Gio.Settings returned by getSettings() is cached for the lifetime of the gnome-shell process, so the handler outlives the indicator. Every enable/disable cycle adds another listener; toggling tiling-mode-enabled or quick-settings-enabled then fires N callbacks, each operating on a previous (destroyed) indicator object.

Fix: store the id, override destroy() to disconnect.

2. extension.js disable() typo: themeWm instead of theme

enable() assigns this.theme = new ExtensionThemeManager(this), but disable() was clearing this.themeWm = null — a no-op that left this.theme referencing the manager after disable. Replaced with this.theme = null and added a comment explaining the previous typo.

Test plan

  • node --check on both touched files.
  • Manual reload cycles via gnome-extensions disable forge@jmmaranan.com && gnome-extensions enable forge@jmmaranan.com repeated several times — no journal warnings about tiling-mode-enabled callbacks firing on destroyed objects.
  • Toggle Super+T after multiple reloads — only one settings listener active.

Notes

  • Both changes are zero-behaviour for the happy path; they only affect state cleanup at disable time.
  • The settings leak compounds over time, so it can hide other extension bugs by spraying spurious GC sweep warnings into the journal.

Two small lifecycle fixes that surface during repeated enable/disable
cycles (e.g. while iterating on the extension during development):

1. FeatureIndicator.constructor connects to `extension.settings`
   without storing the handler id. `Gio.Settings` returned by
   `getSettings()` is cached for the lifetime of the gnome-shell
   process, so the handler outlives the indicator: each cycle
   accumulates another callback, all firing on previous (destroyed)
   indicators when the user toggles tiling-mode-enabled or
   quick-settings-enabled. Track the id and disconnect in destroy().

2. extension.js disable() set `this.themeWm = null`, but the field
   assigned in enable() is `this.theme` (an ExtensionThemeManager).
   The original statement was a typo / no-op leaving the real
   reference dangling on the extension object after disable. Replace
   with `this.theme = null`.

Both are zero-behaviour changes for the happy path; they only affect
state cleanup at disable time.
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.

1 participant