UshakovVasilii / gnome-shell-extension-freon

Shows CPU temperature, disk temperature, video card temperature (NVIDIA/Catalyst/Bumblebee&NVIDIA), voltage and fan RPM
https://extensions.gnome.org/extension/841/freon
GNU General Public License v2.0
431 stars 77 forks source link

gnome-shell crash on 3.26 #71

Closed hedgepigdaniel closed 6 years ago

hedgepigdaniel commented 6 years ago

This has started happening regularly for me after upgrading to 3.26. It happens when logging in, before the desktop becomes visible. Often I have to try logging in up to 5 times before it works.

Stack trace:

0 0x1ffeffc980 b /usr/share/gnome-shell/extensions/freon@UshakovVasilii_Github.yahoo.com/extension.js:360 (0x42dbdc48 @ 1995)

1 0x1ffeffd680 I resource:///org/gnome/gjs/modules/_legacy.js:82 (0x3a4c2c48 @ 71)

2 0x3a981920 i /usr/share/gnome-shell/extensions/freon@UshakovVasilii_Github.yahoo.com/extension.js:231 (0x42dbdb38 @ 17)

3 0x1ffeffe360 b self-hosted:913 (0x3a4ee4d8 @ 346)

4 0x1ffeffeb60 b /usr/share/gnome-shell/extensions/freon@UshakovVasilii_Github.yahoo.com/commandLineUtil.js:38 (0x42dc66f8 @ 454)

5 0x1ffefff970 b self-hosted:917 (0x3a4ee4d8 @ 394)

GNOME bugzilla: https://bugzilla.gnome.org/show_bug.cgi?id=788931 Ubuntu bug: https://bugs.launchpad.net/ubuntu/+source/gnome-shell/+bug/1714989 Similar issue in another extension: https://github.com/RaphaelRochet/arch-update/issues/81#issuecomment-345083876

The upshot seems to be that extensions must not call set_text on a label who's actor has been destroyed (Which I'm guessing Freon is doing because the callback for CommandLineUtil is called even after _onDestroy runs).

hedgepigdaniel commented 6 years ago

I think this fixes it: https://github.com/UshakovVasilii/gnome-shell-extension-freon/pull/72

UshakovVasilii commented 6 years ago

Thanks!!! But I'm not sure that is will help.

As I understand problem related to set_text method on St.Label from _hotLabels array.

let l = this._hotLabels[s.label];
if(l)
    l.set_text(s.value);

But you connected update-display to PanelMenu.Button, destroy will be called for this element only if extension will be disabled.

As I understand to fix this problem we can just switch order from:

let l = this._hotLabels[self.label];
...
l.destroy();
delete this._hotLabels[self.label];

to

let l = this._hotLabels[self.label];
...
delete this._hotLabels[self.label];
l.destroy();

Does it make sense?

hedgepigdaniel commented 6 years ago

Hmm, I'm not sure how that would fix it. You mean here right: https://github.com/UshakovVasilii/gnome-shell-extension-freon/blob/master/freon%40UshakovVasilii_Github.yahoo.com/extension.js#L443?

I think this bug was happening because in CommandLineUtil it was asynchronously doing things (after a command line action completed) and some time while it was running the label actor was destroyed (I don't know why, tbh I don't know what a label actor is either, I just copied this: https://github.com/jderose9/dash-to-panel/pull/263/files).

If l.destroy somehow causes set_text to happen and doing delete this._hotLabels[self.label]; first prevents that, then sure, I think this would fix it.

UshakovVasilii commented 6 years ago

I sent extension to review please check fix if it is possible https://extensions.gnome.org/review/7701

hedgepigdaniel commented 6 years ago

I'm running with it, so I'll let you know if the crash happens again. So far I've logged in about 10 times and it hasn't crashed, so looking good!

Thanks

On Thu, Nov 23, 2017 at 12:47 AM, Vasilii notifications@github.com wrote:

I sent extension to review please check fix if it is possible https://extensions.gnome.org/review/7701

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/UshakovVasilii/gnome-shell-extension-freon/issues/71#issuecomment-346354369, or mute the thread https://github.com/notifications/unsubscribe-auth/ABKUcaiD2lIWvf_0ar-RfbF95woKzFV8ks5s5CXsgaJpZM4Qjk-H .

hedgepigdaniel commented 6 years ago

I got one crash in st_label_set_text today, but I was running without the fix to gnome-shell here: https://github.com/GNOME/gnome-shell/commit/3b4be770a0590bcee9c739f3d9264320e23d555b, and it didn't happen immediately after login like the ones triggered by freon did. So I think the patch is working.

Daniel

On Thu, Nov 23, 2017 at 10:06 AM, Daniel Playfair Cal < daniel.playfair.cal@gmail.com> wrote:

I'm running with it, so I'll let you know if the crash happens again. So far I've logged in about 10 times and it hasn't crashed, so looking good!

Thanks

On Thu, Nov 23, 2017 at 12:47 AM, Vasilii notifications@github.com wrote:

I sent extension to review please check fix if it is possible https://extensions.gnome.org/review/7701

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/UshakovVasilii/gnome-shell-extension-freon/issues/71#issuecomment-346354369, or mute the thread https://github.com/notifications/unsubscribe-auth/ABKUcaiD2lIWvf_0ar-RfbF95woKzFV8ks5s5CXsgaJpZM4Qjk-H .