multicore-sys-monitor@ccadeptic23 using old Network Manager library causing crashes

[coderforlife]

Applet: multicore-sys-monitor@ccadeptic23
Applet Version: v1.8.2, 2018-01-30
Cinnamon Version: 3.6.7
Distribution: Fedora 27

Notify author of applet
@jaszhix

Issue
On recent Fedora distributions the NetworkManager has been changed from "glib" to "libnm". Since the libnm version is loaded for other purposes, loading the glib version causes Cinnamon to crash. This applet is currently setup to load the old, glib, version causing Cinnamon to crash as long as either of the two old typelibs are loaded (NMClient or NetworkManager).

The new library just uses the NM typelib. Updating the following 3 fixes in DataSources.js fix the entire applet:

• Change const NMClient = imports.gi.NMClient and const NetworkManager = imports.gi.NetworkManager to const NM = imports.gi.NM;
• Change this.nmClient = NMClient.Client.new(); to this.nmClient = NM.Client.new(null);
• Change NetworkManager.DeviceState.CONNECTED to NM.DeviceState.ACTIVATED

Steps to reproduce
Using Cinnamon spin of Fedora 27 add the multicore-sys-monitor applet.

Other information
I am not doing this as a pull request since there is still one major issue to fix: this change is likely to break systems still using the glib NetworkManager and I am unsure how to address this. One important thing to note is that NetworkManager (and maybe NMClient) are ever imported (even if not used) the entire Cinnamon will be brought down. I am unsure if trying to import NM on older system will cause any problems. If not, then trying to use it may be the way to check (like how GTop is checked for in applet.js).

[coderforlife]

I guess the following would work (although I am not a JS programmer and have no way to test this on non-Fedora systems without lots of effort). I haven't tested this code at all, but I think the concept is solid.

Replace the two Network Manager imports with the following:

let CONNECTED_STATE;
let NMClient_new;
tryFn(function() {
  const NM = imports.gi.NM;
  CONNECTED_STATE = NM.DeviceState.ACTIVATED;
  NMClient_new = () => { return NM.Client.new(null); }
}, function(e) {
  const NMClient = imports.gi.NMClient;
  const NetworkManager = imports.gi.NetworkManager;
  CONNECTED_STATE = NetworkManager.DeviceState.CONNECTED;
  NMClient_new = () => { return NMClient.Client.new(); }
});

Then this.nmClient = NMClient.Client.new(); is replaced by this.nmClient = NMClient_new(); and NetworkManager.DeviceState.CONNECTED is replaced by CONNECTED_STATE.

[jaszhix]

Thanks for pointing this out to me. I was assuming the new NM typelib was a new library since it has different namespaces - I'm guessing there isn't a way to load the old typelib on the same system. In your example you have the right idea. const is block scoped so the declarations would need to be moved out of the try-catch.

[coderforlife]

But the variables that are const-ed are not used outside those blocks since they are just used to create some other variables (CONNECTED_STATE and NMClient_new). Also, if you do try to load both, depending on how, a warning will sometimes be shown in one of the error logs (I forget if it is glass.log or xsessionerrors.log, only showed up like once or twice out of several restart attempts) and Cinnamon will die. I strongly doubt there is anyway to get them to coexist in the same application.

[jaszhix]

I won't be able to PR this for at least a week, but if you're able to test on Mint 18.3 in a VM, feel free to open a PR.

[coderforlife]

So I tried these changes in my fork: coderforlife@0dec5ee

It works in Fedora but causes Cinnamon in Mint 18.3 to freeze. Looking at the .xsession-errors file I see the following relevant messages:

Cinnamon warning: Log level 16: cannot register existing type 'NMSetting'
Cinnamon warning: Log level 8: g_once_init_leave: assertion 'result != 0' failed
Cinnamon warning: Log level 8: g_type_register_static: assertion 'parent_type > 0' failed

(cinnamon: 2584): libnm-CRITICAL **: _nm_register_setting: assertion 'type != G_TYPE_INVALID' failed
Cinnamon warning: Log level 8: g_once_init_leave: assertion 'result != 0' failed

In .cinnamon/glass.log there are no interesting messages, in fact there is no line for Loaded applet multicore-sys-monitor@ccadeptic23 in ## ms like the rest of the applets.

When I look in the /usr/lib/x86_64-linux-gnu/girepository-1.0/ folder it has all three NM typelibs: NM, NMClient, and NetworkManager. My guess is that the NM one is being loaded even though it shouldn't be.

Do you have any other ideas on guessing which one should be used?

[coderforlife]

Well, one quick way I found was to run lsof -p <cinnamon pid> | grep NetworkManager-1.0.typelib. If there is a 0 exit code than we need to use the new system, otherwise we need to use the old system. This still means I need to be able to get the current Cinnamon's PID and be able to run that command and get the exit code. Additionally, this check may not work unless the built-in network applet loads before this one does (I believe it is able to detect which to use and loads the proper one).

[coderforlife]

Okay, my latest attempt actually works for both Fedora and Mint! coderforlife@602e8a1

It uses a small bash script to detect if NetworkManager-1.0.typelib is already being used or not using the above command line, resulting in the exit code of either 0 (yes, old-style) or 256 (no, new-style). The DataProviders.js uses GLib.spawn_sync to get this value and then acts accordingly. This works mostly great.

On the Mint system (in a VirtualBox) it takes 30-60 ms for the plugin to start which seems fast enough (seeing that the entire AppletManager takes ~500ms). However on my Fedora system (run on bare-metal) it takes a whopping 700 ms to start (out of 1144 ms which means that all the other plugins are starting faster while this one is significantly slower). My guess is that this is because it is running as a bash script which is then sourcing my .bashrc file which takes some time to run.

If you have ideas how to suppress that I think this would be ready for a pull-request.

[coderforlife]

My guess was wrong, even after clearing out my .bashrc file it still takes 500-1000ms to load the applet on my Fedora machine. Any thoughts on why this may be would be appreciated.

[coderforlife]

The newest version has times down to 200ms on my Fedora system and 60ms on Linux Mint in a VirtualBox. The high time required for mine is likely due to the much larger number of disks and CPUs needed to examine during first startup, however at least it is way down from the 0.5-1 seconds.

[jaszhix]

Ah yeah, its about 200ms for me as well on my 14 core workstation. It loads fine in an 18.3 VM. I was testing before you opened the PR, and noticed the new NM GI binding works fine on Ubuntu 18.04 bionic as well, but it doesn't crash on the older one. I wonder why that occurs on Fedora, maybe they removed the old version altogether.

I can merge your PR, thanks for looking into this!