diff options
| -rw-r--r-- | src/indicator-ng.c | 13 | ||||
| -rw-r--r-- | tests/CMakeLists.txt | 21 | ||||
| -rw-r--r-- | tests/test-css-provider-leak.c | 128 |
3 files changed, 157 insertions, 5 deletions
diff --git a/src/indicator-ng.c b/src/indicator-ng.c index a94f57e..0c76df6 100644 --- a/src/indicator-ng.c +++ b/src/indicator-ng.c @@ -53,6 +53,7 @@ struct _IndicatorNg gint64 last_service_restart; GMenuModel *lMenuSections[MENU_SECTIONS]; + GtkCssProvider *label_css_provider; }; static void indicator_ng_initable_iface_init (GInitableIface *initable); @@ -157,6 +158,7 @@ indicator_ng_dispose (GObject *object) indicator_ng_free_actions_and_menu (self); + g_clear_object (&self->label_css_provider); g_clear_object (&self->entry.label); g_clear_object (&self->entry.image); g_clear_object (&self->entry.menu); @@ -561,13 +563,14 @@ static void indicator_ng_set_label(IndicatorNg *self, const gchar *label) } GtkWidget *pParent = gtk_widget_get_parent(GTK_WIDGET(self->entry.label)); - GtkCssProvider *pCssProvider = gtk_css_provider_new(); - GtkStyleContext *pStyleContext = gtk_widget_get_style_context(GTK_WIDGET(self->entry.label)); - gtk_style_context_add_provider(pStyleContext, GTK_STYLE_PROVIDER(pCssProvider), GTK_STYLE_PROVIDER_PRIORITY_APPLICATION); + if (self->label_css_provider == NULL) { + self->label_css_provider = gtk_css_provider_new(); + GtkStyleContext *pStyleContext = gtk_widget_get_style_context(GTK_WIDGET(self->entry.label)); + gtk_style_context_add_provider(pStyleContext, GTK_STYLE_PROVIDER(self->label_css_provider), GTK_STYLE_PROVIDER_PRIORITY_APPLICATION); + } gchar *sCss = g_strdup_printf("label{padding-left: %ipx;}", nPadding); - gtk_css_provider_load_from_data(pCssProvider, sCss, -1, NULL); + gtk_css_provider_load_from_data(self->label_css_provider, sCss, -1, NULL); g_free(sCss); - g_object_unref(pCssProvider); if (GTK_IS_BOX(pParent)) { gtk_box_set_spacing(GTK_BOX(pParent), nSpacing); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index a461dab..3f0ef94 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -195,6 +195,26 @@ add_custom_command( ) add_test("test-desktop-shortcuts-tester" "test-desktop-shortcuts-tester") +# test-css-provider-leak +add_test_executable_by_name(test-css-provider-leak) + +# test-css-provider-leak-tester +add_custom_command( + OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/test-css-provider-leak-tester" + DEPENDS "${CMAKE_CURRENT_BINARY_DIR}/test-css-provider-leak" + WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} + VERBATIM + COMMAND + echo "#!/bin/sh" > "${CMAKE_CURRENT_BINARY_DIR}/test-css-provider-leak-tester" + COMMAND + echo ". ${CMAKE_CURRENT_SOURCE_DIR}/run-xvfb.sh" >> "${CMAKE_CURRENT_BINARY_DIR}/test-css-provider-leak-tester" + COMMAND + echo "gtester -k --verbose -o=${CMAKE_CURRENT_BINARY_DIR}/loader-check-results.xml ${CMAKE_CURRENT_BINARY_DIR}/test-css-provider-leak" >> "${CMAKE_CURRENT_BINARY_DIR}/test-css-provider-leak-tester" + COMMAND + chmod +x "${CMAKE_CURRENT_BINARY_DIR}/test-css-provider-leak-tester" +) +add_test("test-css-provider-leak-tester" "test-css-provider-leak-tester") + if (FLAVOUR_GTK3 AND ENABLE_IDO) # test-indicator-ng add_test_executable_by_name(test-indicator-ng) @@ -267,6 +287,7 @@ set (ALL_TESTERS "service-version-tester" "service-version-multiwatch-tester" "test-desktop-shortcuts-tester" + "test-css-provider-leak-tester" "loader-tester" ) diff --git a/tests/test-css-provider-leak.c b/tests/test-css-provider-leak.c new file mode 100644 index 0000000..aeb7d49 --- /dev/null +++ b/tests/test-css-provider-leak.c @@ -0,0 +1,128 @@ +/* + * Test that demonstrates the GtkCssProvider memory leak in + * indicator_ng_set_label (before fix) and validates the fix. + * + * Before the fix, every call to indicator_ng_set_label() created a new + * GtkCssProvider and added it to the label's GtkStyleContext without + * removing the old one. Since the datetime indicator updates the label + * once per second, this leaked ~2 KB/sec (~170 MB/day). + * + * This test calls set_label in a loop and measures heap growth via + * /proc/self/statm to show the leak is real and the fix works. + */ + +#include <gtk/gtk.h> +#include <stdio.h> + +static long +get_rss_kb (void) +{ + long rss = 0; + FILE *f = fopen ("/proc/self/statm", "r"); + if (f) { + long size, resident; + if (fscanf (f, "%ld %ld", &size, &resident) == 2) + rss = resident * (sysconf (_SC_PAGESIZE) / 1024); + fclose (f); + } + return rss; +} + +/* + * Simulate what indicator_ng_set_label does using the UNFIXED code: + * creates and adds a new GtkCssProvider every call. + */ +static void +set_label_leaky (GtkLabel *label, guint nPadding) +{ + GtkCssProvider *pCssProvider = gtk_css_provider_new (); + GtkStyleContext *pStyleContext = gtk_widget_get_style_context (GTK_WIDGET (label)); + gtk_style_context_add_provider (pStyleContext, GTK_STYLE_PROVIDER (pCssProvider), + GTK_STYLE_PROVIDER_PRIORITY_APPLICATION); + gchar *sCss = g_strdup_printf ("label{padding-left: %ipx;}", nPadding); + gtk_css_provider_load_from_data (pCssProvider, sCss, -1, NULL); + g_free (sCss); + g_object_unref (pCssProvider); +} + +/* + * Simulate what indicator_ng_set_label does AFTER the fix: + * reuse a single provider, only reload its CSS data. + */ +static void +set_label_fixed (GtkLabel *label, guint nPadding, GtkCssProvider **cached) +{ + if (*cached == NULL) { + *cached = gtk_css_provider_new (); + GtkStyleContext *pStyleContext = gtk_widget_get_style_context (GTK_WIDGET (label)); + gtk_style_context_add_provider (pStyleContext, GTK_STYLE_PROVIDER (*cached), + GTK_STYLE_PROVIDER_PRIORITY_APPLICATION); + } + gchar *sCss = g_strdup_printf ("label{padding-left: %ipx;}", nPadding); + gtk_css_provider_load_from_data (*cached, sCss, -1, NULL); + g_free (sCss); +} + +#define ITERATIONS 100000 + +static void +test_leaky_set_label (void) +{ + GtkWidget *label = gtk_label_new ("12:00:00"); + g_object_ref_sink (label); + + long rss_before = get_rss_kb (); + + for (int i = 0; i < ITERATIONS; i++) { + set_label_leaky (GTK_LABEL (label), 6); + } + + long rss_after = get_rss_kb (); + long growth_kb = rss_after - rss_before; + + g_test_message ("leaky: RSS before=%ld KB, after=%ld KB, growth=%ld KB over %d iterations", + rss_before, rss_after, growth_kb, ITERATIONS); + + /* 100k leaked providers should use at least 50 MB. + If we see significant growth, the leak is confirmed. */ + g_assert_cmpint (growth_kb, >, 50000); + + g_object_unref (label); +} + +static void +test_fixed_set_label (void) +{ + GtkWidget *label = gtk_label_new ("12:00:00"); + g_object_ref_sink (label); + + long rss_before = get_rss_kb (); + + GtkCssProvider *cached = NULL; + for (int i = 0; i < ITERATIONS; i++) { + set_label_fixed (GTK_LABEL (label), 6, &cached); + } + + long rss_after = get_rss_kb (); + long growth_kb = rss_after - rss_before; + + g_test_message ("fixed: RSS before=%ld KB, after=%ld KB, growth=%ld KB over %d iterations", + rss_before, rss_after, growth_kb, ITERATIONS); + + /* With the fix, memory growth should be negligible (< 5 MB). */ + g_assert_cmpint (growth_kb, <, 5000); + + g_object_unref (cached); + g_object_unref (label); +} + +int +main (int argc, char **argv) +{ + gtk_test_init (&argc, &argv); + + g_test_add_func ("/indicator-ng/css-provider-leak/leaky", test_leaky_set_label); + g_test_add_func ("/indicator-ng/css-provider-leak/fixed", test_fixed_set_label); + + return g_test_run (); +} |