Errors on startup and after prefs change

Sorry for the long post but I have a couple issues with my extension that are beyond my experience level and need some help with. I’ll try to be brief.

I have an extension.js that adds a series of StatusPanels, extending St.BoxLayout, to this.main.box. Each panel has an icon and label describing the status of a server, up or down. Each panel polls a server periodically and updates its icon. Everything lays out ok but I’m getting startup errors:

Can’t update stage views actor overviewGroup is on because it needs an allocation.
Can’t update stage views actor overview is on because it needs an allocation.
Can’t update stage views actor Gjs_ui_overviewControls_ControlsManager is on because it needs an allocation.
Can’t update stage views actor Gjs_ui_workspaceThumbnail_ThumbnailsBox is on because it needs an allocation.
Can’t update stage views actor Gjs_ui_workspaceThumbnail_WorkspaceThumbnail is on because it needs an allocation.
Can’t update stage views actor ClutterActor is on because it needs an allocation.
Can’t update stage views actor ClutterActor is on because it needs an allocation.
Can’t update stage views actor Gjs_ui_workspaceThumbnail_WindowClone is on because it needs an allocation.
Can’t update stage views actor ClutterClone is on because it needs an allocation.

I assume they’re from my extension; I have all other extensions turned off.

Further, the extension listens for preference changes then removes and recreates all the panels. Then I get other errors:

Object St.Icon (0x5650d76fd230), has been already deallocated — impossible to set any property on it. This might be caused by the object having been destroyed from C code using something such as destroy(), dispose(), or remove() vfuncs.
== Stack trace for context 0x5650d5733120 ==
#0 7ffe80bf4790 b /home/craig/.local/share/gnome-shell/extensions/serverstatus@footeware.ca/statusPanel.js:111 (3ded9b7c5a10 @ 152)

This seems to related to the updating of the panels’ icons. I find that strange because in my prefs listener I call:

this.menu.box.destroy_all_children();

…then I recreate the panels and their icons so I don’t see how old ones are left around. I tried wrapping each icon access in an if-block:

const gicon = (message.status_code == 200) ? this.serverUpIcon : this.serverDownIcon;
log('before null check, icon is ’ + icon);
if (icon != null) {
log('after null check, icon is ’ + icon);
icon.gicon = gicon;
this.updateTaskbarCallback();
}

This produces the output:

before null check, icon is [null]
== Stack trace for context 0x5650d5733120 ==
#0 5650d6e330a8 i /home/craig/.local/share/gnome-shell/extensions/serverstatus@footeware.ca/statusPanel.js:104 (3ded9b7c5420 @ 144)
Object St.Icon (0x5650d7704870), has been already deallocated — impossible to access it. This might be caused by the object having been destroyed from C code using something such as destroy(), dispose(), or remove() vfuncs.
after null check, icon is [null]

You can see the icon is reported as null, I guess indicating it’s a stale resource somehow, but then it ignores the if check and displays code in the if-block. Not sure how that’s possible.

Anyhow, if you can provide some insights here I’d really appreciate it. I didn’t want to post a lot of code here to keep a long post shorter but you can see it all on github. Thanks in advance.

This code here is definitely going to cause you problems:

statusPanel.js - L59-L71

run_dispose() {
    clearInterval(intervalID);
    this.serverIcon = null;
    this.serverUpIcon = null;
    this.serverDownIcon = null;
    this.serverBadIcon = null;
    super.run_dispose;
  }

  destroy(){
    this.run_dispose();
    super.destroy();
  }

You should never override Clutter.Actor.destroy() or GObject.Object.run_dispose(), and almost never touch GObject.Object.run_dispose() at all in GJS. What you want to do here is connect to Clutter.Actor::destroy which is guaranteed to be called before the object is finalized and not interfere with object disposal:

this.connect('destroy', (actor) => {
    clearInterval(intervalID);

    // no need to null out variables here
});

This will also cause you problems. The call to Soup.Session.queue_message() is asynchronous, meaning that the callback can easily be invoked after your object has been destroyed:

  get(httpMethod, url, icon) {
    try {
      // Here your object may be not be destroyed
      let request = Soup.Message.new(httpMethod, url);
      this.session.queue_message(request, (session, message) => {
        // Here your object may be destroyed, when this callback is invoked
      });
    } catch (e) {
      // Here you are checking if the icon is `null`, but the result you're
      // getting is `[null]`, not `null`.
      if (icon != null) {
        icon.gicon = this.serverBadIcon;
        this.updateTaskbarCallback();
      }
    } finally {
      // not sure what this does
      return GLib.SOURCE_REMOVE;
    }
  }

You also have another, probably larger problem. In extension.js you are clearly creating more than one StatusPanel, however the intervalId variable in statusPanel.js is a module-level variable being shared between all instances of StatusPanel.

In other words, as soon as the second StatusPanel is created, the intervalId is replaced with a different ID. Only the GSource for the last interval created will ever be removed from the main loop. The other will continue to callback to destroyed objects every time the this.settings.frequency timeout is reached.

I hope that gives you a few places to start :slight_smile:

Thanks a lot for replying. I really appreciate the review. I don’t want the extension going out doing bad stuff.

I’ve switched the run_dispose/destroy code over to use the connect code. Good advice there.

I also changed the if-checks toif(icon != [null]) but it still gets into the if-block, i.e. I’m getting the same behavior.

Thanks for identifying the issue with intervalId being global too. Do you think switching to a synchronous Soup call will cause a problem? I understand that the extension thread is the same as the gnome shell thread. The whole point is to have multiple servers identified and polled :frowning: I specify a time-out of 5 seconds before Soup times out on a request but 5 seconds on the gnome shell thread seems a real problem to me.

Sorry, it just dawned on me that I may have misunderstood the intervalId thing. Is it the way I’m using intervalId that makes it global and that there’s a way to make it instance level?

UPDATE: I moved the intervalId from outside the class (global, class-level?) to this.intervalId inside the class and now the null icons have disappeared! It might all be working properly now. I’ll keep testing for a while yet but I really appreciate the help.

That pretty much won’t work. When you do if (icon != [null]) you are creating a new Array object with [null] and then asking if it is equal to icon. Of course it can’t be, because you just created it at the time of comparison.

I’m not sure what the best approach here is based on your current architecture, but creating situations where you may call back on destroyed objects is something you should try to avoid.

Yes, you can set the ID as something like this._sourceId then remove it in your Clutter.Actor::destroy callback:

this.connect('destroy', (actor) => {
    if (this._sourceId) {
        GLib.Source.remove(this._sourceId);
        this._sourceId = null;
    }
});

Got it, now checking for this.intervalId before removing it in destroy handler.

Note that you actually have to set it on the object, it won’t just be there.

_init() {
    // ...

    this._sourceId = GLib.timeout_add_seconds(
        GLib.PRIORITY_DEFAULT,
        this.setting.frequency,
        () => this.update(this.setting.url)
    );

    this.connect('destroy', (actor) => {
        if (this._sourceId) {
            GLib.Source.remove(this._sourceId);
            this._sourceId = null;
        }
    });
}

Yes, got that already once I clued into the global issue :slight_smile:

1 Like

This topic was automatically closed 30 days after the last reply. New replies are no longer allowed.