Skip to content

[PATCH] increased CPU/memory usage over time

Submitted by dwk..@..il.com

Assigned to Xfce-Goodies Maintainers

Link to original bug (#16013)

Description

I am running xfce4-cpufreq-plugin 1.2.1-1 on Debian. I noticed that it ends up using increasingly more CPU and RAM over several days, to the point where it uses 50% CPU and has a 600MB heap (while it started with 4MB). I verified that it is not performing additional system calls, which was my first guess, and indeed /proc/X/stat shows that it is spending these CPU cycles in userspace. I dumped its heap from gdb and found half a million instances of the string "Sans". I traced the code and found that cpuFreq->layout_changed was being set to TRUE every update (once per second in my case). The root cause of this is the following bug:

xfce4-cpufreq-plugin.c:
237         if (label_size.width > cpuFreq->label_max_width)
238           cpuFreq->label_max_width = label_size.width;
239         cpuFreq->layout_changed = TRUE;

It always sets layout_changed to TRUE even when width==label_max_width (the normal case). This should be

237         if (label_size.width > cpuFreq->label_max_width)
238           cpuFreq->layout_changed = TRUE;
239         cpuFreq->label_max_width = label_size.width;

Luckily this snippet appears correctly in the other place it was copy pasted in the file. Now it doesn't update repeatedly, which fixes the problem for me. But there is clearly a memory leak of some sort. After looking very hard at the code in cpufreq_label_set_font(), I think I found it.

78     provider = gtk_css_provider_new ();
79 
80     gtk_css_provider_load_from_data (provider, css, -1, NULL);
81     gtk_style_context_add_provider (
82       GTK_STYLE_CONTEXT (gtk_widget_get_style_context (GTK_WIDGET (cpuFreq->label))),
83       GTK_STYLE_PROVIDER (provider), GTK_STYLE_PROVIDER_PRIORITY_APPLICATION);

This gets called every time a font is loaded. It adds a new GtkStyleProvider but doesn't remove the old one. Hence, over time, many many GtkStyleProviders exist all tied to the label widget. My suggested fix is to remember the old pointer so that it can be removed. My complete patch is as follows:

diff --git a/xfce4-cpufreq-plugin.c b/xfce4-cpufreq-plugin.c
index e886121..e2282f3 100644
--- a/xfce4-cpufreq-plugin.c
+++ b/xfce4-cpufreq-plugin.c
@@ -75,7 +75,15 @@ cpufreq_label_set_font (void)
 
   if (css)
   {
+    if(cpuFreq->label_css_provider)
+    {
+      gtk_style_context_remove_provider (
+        GTK_STYLE_CONTEXT (gtk_widget_get_style_context (GTK_WIDGET (cpuFreq->label))),
+        GTK_STYLE_PROVIDER (cpuFreq->label_css_provider));
+    }
+
     provider = gtk_css_provider_new ();
+    cpuFreq->label_css_provider = provider;
 
     gtk_css_provider_load_from_data (provider, css, -1, NULL);
     gtk_style_context_add_provider (
@@ -235,8 +243,8 @@ cpufreq_update_label (CpuInfo *cpu)
       else
       {
         if (label_size.width > cpuFreq->label_max_width)
-          cpuFreq->label_max_width = label_size.width;
-        cpuFreq->layout_changed = TRUE;
+          cpuFreq->layout_changed = TRUE;
+        cpuFreq->label_max_width = label_size.width;
       }
     }
   }
@@ -551,6 +559,7 @@ cpufreq_prepare_label (CpuFreqPlugin *cpufreq)
   {
     gtk_widget_destroy (cpufreq->label);
     cpufreq->label = NULL;
+    cpufreq->label_css_provider = NULL;
   }
 
   if (cpuFreq->options->show_label_freq || cpuFreq->options->show_label_governor)
@@ -576,6 +585,7 @@ cpufreq_widgets (void)
   css = g_strdup_printf("button { padding: 0px; }");
 
   provider = gtk_css_provider_new ();
+  cpuFreq->label_css_provider = provider;
   gtk_css_provider_load_from_data (provider, css, -1, NULL);
   gtk_style_context_add_provider (
     GTK_STYLE_CONTEXT (gtk_widget_get_style_context (GTK_WIDGET (cpuFreq->button))),
diff --git a/xfce4-cpufreq-plugin.h b/xfce4-cpufreq-plugin.h
index a6895e4..f163e3c 100644
--- a/xfce4-cpufreq-plugin.h
+++ b/xfce4-cpufreq-plugin.h
@@ -86,6 +86,7 @@ typedef struct
 
   /* Widgets */
   GtkWidget *button, *box, *icon, *label;
+  GtkCssProvider *label_css_provider;
   gboolean layout_changed;
   gint label_max_width;

I am successfully running this modified version on my system. I will report back if it continues to use excessive cpu or memory.

P.S. I initially thought there was a bug with mixing up width and height around line 222, but upon further reflection it is correct. The max_width variable simply stores a height value in the vertical case.

Version: 1.2.1

Edited by Ghost User