PR#7: Fix crashes when locking the screen while certain extensions are active - rpms/gnome-shell

rpms / gnome-shell

#7 Fix crashes when locking the screen while certain extensions are active

Merged 4 years ago by fmuellner. Opened 4 years ago by sgallagh.

rpms/ sgallagh/gnome-shell bz1817082-f32 into f32

Fix crashes when locking the screen while certain extensions are active

Stephen Gallagher • 4 years ago

aa8da7b

0001-extensionSystem-Disable-extension-before-unloading-s.patch

file added

+51

		`@@ -0,0 +1,51 @@`
		`+ From 6d5e93b00b5b4ce5315276e071a98c8db5ff6463 Mon Sep 17 00:00:00 2001`
		`+ From: =?UTF-8?q?Florian=20M=C3=BCllner?= <fmuellner@gnome.org>`
		`+ Date: Wed, 13 May 2020 20:10:56 +0200`
		`+ Subject: [PATCH] extensionSystem: Disable extension before unloading`
		`+ stylesheet`
		`+`
		`+ Removing a stylesheet from the theme will trigger a style update. There's`
		`+ little point in updating the extension actors that are about to be destroyed`
		`+ (hopefully), so call the extension's disable() function first.`
		`+`
		`+ https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/2757`
		`+ ---`
		`+ js/ui/extensionSystem.js \| 12 ++++++------`
		`+ 1 file changed, 6 insertions(+), 6 deletions(-)`
		`+`
		`+ diff --git a/js/ui/extensionSystem.js b/js/ui/extensionSystem.js`
		`+ index 1a34f19e9e384b1d352207ea78eba9e7e1f80ba3..5a50b473ac2bdb25ec1a7f47cbd2d00ebf889800 100644`
		`+ --- a/js/ui/extensionSystem.js`
		`+ +++ b/js/ui/extensionSystem.js`
		`+ @@ -100,22 +100,22 @@ var ExtensionManager = class {`
		`+ } catch (e) {`
		`+ this.logExtensionError(otherUuid, e);`
		`+ }`
		`+ }`
		`+`
		`+ + try {`
		`+ + extension.stateObj.disable();`
		`+ + } catch (e) {`
		`+ + this.logExtensionError(uuid, e);`
		`+ + }`
		`+ +`
		`+ if (extension.stylesheet) {`
		`+ let theme = St.ThemeContext.get_for_stage(global.stage).get_theme();`
		`+ theme.unload_stylesheet(extension.stylesheet);`
		`+ delete extension.stylesheet;`
		`+ }`
		`+`
		`+ - try {`
		`+ - extension.stateObj.disable();`
		`+ - } catch (e) {`
		`+ - this.logExtensionError(uuid, e);`
		`+ - }`
		`+ -`
		`+ for (let i = 0; i < order.length; i++) {`
		`+ let otherUuid = order[i];`
		`+ try {`
		`+ this.lookup(otherUuid).stateObj.enable();`
		`+ } catch (e) {`
		`+ --`
		`+ 2.26.2`
		`+`

gnome-shell.spec

file modified

+8 -1

		`@@ -1,6 +1,6 @@`
		`Name: gnome-shell`
		`Version: 3.36.2`
		`- Release: 2%{?dist}`
		`+ Release: 3%{?dist}`
		`Summary: Window management and application launching for GNOME`

		`License: GPLv2+`
		`@@ -20,6 +20,9 @@`
		`Patch4: extension-prefs-compat.patch`
		`Patch5: 0001-st-theme-Unref-CRStylesheet-after-removing-old-theme.patch`

		`+ # Avoid crash in extensions`
		`+ Patch6: 0001-extensionSystem-Disable-extension-before-unloading-s.patch`
		`+`
		`%define eds_version 3.33.1`
		`%define gnome_desktop_version 3.35.91`
		`%define glib2_version 2.56.0`
		`@@ -216,6 +219,10 @@`
		`%{_mandir}/man1/gnome-shell.1*`

		`%changelog`
		`+ * Wed May 20 2020 Stephen Gallagher <sgallagh@redhat.com> - 3.36.2-3`
		`+ - Fix crashes when locking the screen while certain extensions are active`
		`+ - Resolves: rhbz#1817082`
		`+`
		`* Fri May 01 2020 Florian Müllner <fmuellner@redhat.com> - 3.36.2-2`
		`- Fix a crash`

sgallagh commented 4 years ago

Resolves: rhbz#1817082

Signed-off-by: Stephen Gallagher sgallagh@redhat.com

catanzaro commented 4 years ago

Please ping us here again when https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/1247 is merged upstream

sgallagh commented 4 years ago

Hmm, I thought this was superseding it, but I may have misread. Can we get some movement on 1247 then? I even offered an alternative patch. This issue is painful.

catanzaro commented 4 years ago

OK, I see, this is https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/1247 which is already merged upstream.

sgallagh commented 4 years ago

@catanzaro I'm confused; that link is not yet merged.

This patch in this pull is from https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/1256

sgallagh commented 4 years ago

Referenced from https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/1247#note_798959

Pull-Request has been merged by fmuellner

4 years ago

catanzaro commented 4 years ago

I just pasted wrong I guess :P

fmuellner commented 4 years ago

Hmm, I thought this was superseding it, but I may have misread.

It fixes the crash if the extension is well-behaving enough to not leave around actors after it has been disabled.

It's still a good idea to avoid a crash with buggy extensions, but the patch that landed is an improvement in its own right: It's more correct and cuts out a couple of pointless style changes.

I even offered an alternative patch.

Yes, but that doesn't necessarily speed things up. It means we know have to agree on which approach we want to land first, so that's an extra step :-)