Top icons stop displaying.

[zelch]

Alright, if you have a single icon being displayed by TopIconPlus, and we get a tray-icon-removed signal for an icon that isn't on top, we do the following:

We destroy the icon.
We call icons.splice(-1, 1), which appears to remove the first element of the array.
If the array is then empty (length == 0, and it will be if we only had one), we proceed to set the iconsContainer.active.visible = false.

Except... We actually have icons left, but now we can't ever see them, and future operations are going to get confused because we have icons in the container that are not in the icons array.

If you look at the _onTrayIconRemoved function in gnome-shell's legacyTray.js, we see:

_onTrayIconRemoved: function(tm, icon) {
    if (!this.actor.contains(icon))
        return;

    icon.get_parent().destroy();
    this._sync();
},

The this.actor.contains check seems to be trying to catch this exact case.

I suggest adding the following check to the top of onTrayIconRemoved:

if (icons.indexOf(icon) == -1) {
    log("onTrayIconRemoved for invalid icon!");
    return;
}

(Or without the log statement.)

With that in place, so far my vanishing pidgin seems to no longer be vanishing, and I have seen log entries for the invalid icon case.

[phocean]

Thank you for your research and detailed explanation ! Very appreciated.
I will look at this in a few days, as soon as I have free time.

[zelch]

I'm glad to help. :)

Looking at the logs (I added a fair bit of debugging statements while tracking this down, and have not removed them yet), I see another case which is causing things to disappear.

Somehow, moveToTop is getting called repeatedly, as best as I can tell there is a rapid sequence of disable/enable several times in a row. This gets moveToTray called repeatedly for each disable, which might cause problems with the tray signals getting connected repeatedly, but otherwise looks safe.

However 4 seconds later the idle handlers run, causing movetoTop to get called several times in a row.

The first one does what it is supposed to, however the following ones are another story. The repeated calls to disconnect cause warnings, but the bigger problem is that we create brand new iconsBoxLayout and iconsContainer objects, and since we have already removed the items from the tray iconBox, we don't add the icons to the new objects. (Nor do we destroy the old ones.)

At this point, things are broken and will remain broken until gnome-shell is restarted.

I am going to test with the following added to the top of moveToTop:

if (iconsBoxLayout) {
    log("moveToTop called while already moved to the top.");
    return;
} 

[zelch]

I am still seeing some oddities that I am trying to track down, it is helping, but... Odd.

I am curious, why is moveToTop called via GLib.idle_add instead of being called directly?

[tihoho]

@zelch u a fixed it bug? i have it bag too. icons just invisible sometime. and i don't know similar apps for tray icons..

[Snowman3456]

My icons are stopped displaying as well. I see a chunk of space where they ought to be. I have around 4-5 icons. Tried changing all of the display settings/opacity/contrast etc. No luck.

[marsdeat]

I have the same bug as @Enigma0, screenshot attached:

I can click on the "icons" that are invisible in that area and that brings up the appropriate windows and context menus, but there's no icon visible still.

[Snowman3456]

Started working again for me somehow.

[phocean]

@Enigma0 did you update the extension or anything special?

[phocean]

@zelch sorry for the long silence, I am coming back little by little.
With all your testing, have you got some stable code that fix the issue?

About the Glib call, it is inherited from the original extension. I am not sure if the priority really makes sense or is respected in any way... Another thing to test.

[Snowman3456]

@phocean Nope. I installed the original TopIcons extension several times with a bunch of enabling/disabling of each one.

[zelch]

@phocean I have code that definitely fixes the issue at the top. From the description I'm pretty sure that @Enigma0 has a different bug.

(Though, it is possible that they are tightly related.)

Now, I can't swear about stable. I keep encountering a gnome-shell segfault, though not very frequently (definitely not frequently enough to say that just because I don't see it in a week that it's really gone). I don't have any reason to believe that it is related to this extension at all, let alone my changes, but I don't have any evidence pointing at anything else either.

My raw diff is largely debug lines, I'll attach the whole thing though, as it may be helpful. And I do suggest going forward with the actual code changes. (Which I'd be happy to isolate out into a separate diff.)

[zelch]

TopIcons.diff.txt

Here is the diff with all the debugging.

[zelch]

And here is the diff with just the code changes.

TopIcons.code_only.diff.txt

[Snowman3456]

I still occasionally see 1 or 2 icons disappear (they still exist and can be interacted with) but usually closing and re-opening that particular tray program fixes it.

[zelch]

@Enigma0 If you can easily reproduce this, can you try applying my diff and restarting gnome-shell?

Save TopIcons.diff.txt in /tmp/

cd ~/.local/share/gnome-shell/extensions/[email protected]
cat /tmp/TopIcons.diff.txt | patch -s

Then log out and log back in.

If that fixes the problem, that would be interesting. If it doesn't fix the problem then getting the debug output from around the time that the icons disappeared with the following command would be helpful.

journalctl | grep gnome-session

(Note, you don't need to send the whole thing, just the timestamps right around when things started going wonky.)

Thanks.

[matjam]

I get the same thing with an app called openFortiGUI. After some time it kills topicons-plus and I have to disable/enable a few times, sometimes kill the app and restart.

@zelch I'll add your patch and see if it resolves the issue for me.

[Snowman3456]

@zelch I cannot anymore - seems to be fixed for me.

[zelch]

@Enigma0 To confirm, applying the patch seems to have resolved the issue?

[zelch]

Here is an additional fix on top of my non-logging version, which seems to have cleared up a crash due to race condition.

TopIcons_More.diff.txt

And here is an updated version of my full patch with all of the debug statements, sorry, no incremental patch here.

TopIcons_Debug.diff.txt

I will update if I see any additional crashes, but I'm pretty sure that this is pretty close to final.

@phocean How would you like everything for a release? The current diffs? A new diff with all the changes but none of the logging? A pull request?

[phocean]

@zelch Thanks, good news! :-)
The best would be a pull request, without the logging parts.
But I can manage anyway.

[Snowman3456]

@zelch No I was saying that since it was already working for me again, I wouldn't be able to test the patch.

[matjam]

@zelch the first patch didn't resolve it for me. It seems to reliably break when the system sleeps and is woken up again.

Where are the logs written?

edit: nvm; on my system journalctl /usr/bin/gnome-shell retrieves them.

I've reinstalled and added the latest Debug patch.

[zelch]

@matjam Please let me know if the most recent patch does it for you. There was a race condition where if the icon changed between us adding it and a time based trigger running things would crash. This looks like it was causing the crashes I have been able to see.
(But I don't do a lot of suspend/resume on my system.)

[zelch]

@phocean It looks like you have recently merged in some other changes. I will get everything merged, tested and commited and try and get you a pull request in the not too distant future.

[zelch]

Alright, my first attempt at merging my stuff with the current git head is... Horrifically unstable.

Enough so that I may need to bite the bullet and make a VM for more development of this, because doing it on my work machine is becoming non-viable.

[phocean]

@zelch Sorry for my long period of inactivity. If by any chance you are still using it, is it still an issue or can I close it?

I have to say that I have no issue at all in my environment.