From 8880ce63d6c1c02c08f1ae4e8023ac99a7533810 Mon Sep 17 00:00:00 2001 From: Kenny Daniel Date: Mon, 13 Apr 2026 00:17:32 -0700 Subject: src/indicator-ng.c: Fix GtkCssProvider memory leak in indicator_ng_set_label indicator_ng_set_label() created a new GtkCssProvider and added it to the label's GtkStyleContext on every call, without removing old ones. gtk_style_context_add_provider() accumulates providers, so each call leaked a provider (~1 KB). Since ayatana-indicator-datetime sends action-state-changed once per second to update the clock label, this leaked ~86,400 providers/day, causing mate-indicator-applet-complete to accumulate ~1.4 GB RSS over 7 days. Fix by storing a single GtkCssProvider in the IndicatorNg struct and reusing it. The provider is created on first use and cleaned up in dispose. Test results (100,000 iterations of set_label): Before fix: +99,692 KB RSS growth After fix: +0 KB RSS growth --- tests/CMakeLists.txt | 21 +++++++ tests/test-css-provider-leak.c | 128 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 149 insertions(+) create mode 100644 tests/test-css-provider-leak.c (limited to 'tests') 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 +#include + +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 (); +} -- cgit v1.2.3