From 8bb9eb0fc9d45380823896fd5ae8f9340cdc31ea Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Fri, 18 Jan 2019 13:23:36 +0000 Subject: [PATCH] keyboard: Disconnect from input source manager when destroying indicator When an InputSourceIndicator is destroyed, the InputSourceManager it was connected to could (and probably will) outlive it (since the manager is a singleton). If the InputSourceManager emits any subsequent signals, the callbacks from the finalised InputSourceIndicator could be invoked, and will reference finalised objects. This can be triggered by running `pkexec true` from a gnome-terminal window, then calling `pkill pkexec` from another terminal (on a different VT or via SSH). This causes the dialogue to be cancelled by polkitd. Signed-off-by: Philip Withnall https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/357 --- js/ui/status/keyboard.js | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/js/ui/status/keyboard.js b/js/ui/status/keyboard.js index 980019cd4..ef2a6049b 100644 --- a/js/ui/status/keyboard.js +++ b/js/ui/status/keyboard.js @@ -832,6 +832,8 @@ var InputSourceIndicator = new Lang.Class({ _init() { this.parent(0.0, _("Keyboard")); + this.connect('destroy', this._onDestroy.bind(this)); + this._menuItems = {}; this._indicatorLabels = {}; @@ -856,11 +858,21 @@ var InputSourceIndicator = new Lang.Class({ this._sessionUpdated(); this._inputSourceManager = getInputSourceManager(); - this._inputSourceManager.connect('sources-changed', this._sourcesChanged.bind(this)); - this._inputSourceManager.connect('current-source-changed', this._currentSourceChanged.bind(this)); + this._inputSourceManagerSourcesChangedId = + this._inputSourceManager.connect('sources-changed', this._sourcesChanged.bind(this)); + this._inputSourceManagerCurrentSourceChangedId = + this._inputSourceManager.connect('current-source-changed', this._currentSourceChanged.bind(this)); this._inputSourceManager.reload(); }, + _onDestroy() { + if (this._inputSourceManager) { + this._inputSourceManager.disconnect(this._inputSourceManagerSourcesChangedId); + this._inputSourceManager.disconnect(this._inputSourceManagerCurrentSourceChangedId); + this._inputSourceManager = null; + } + }, + _sessionUpdated() { // re-using "allowSettings" for the keyboard layout is a bit shady, // but at least for now it is used as "allow popping up windows