[Freeciv-Dev] Re: (PR#6742) civclient memory leak

["Raimar Falke" <i-freeciv-lists@xxxxxxxxxxxxx>]

On Mon, Nov 03, 2003 at 07:44:59AM -0800, Christian Knoke wrote:
> 
> CVS 03 NOV 2003 HEAD GTK 1.2
> 
> Memory consumption of civclient grows rapidly, right now 17900/14612
> VSS/RSS with 4 players on standard map.

I created 3 valgrind dumps:
 - starting client&server and stopping
 - same as above but with 1 turn between
 - same as above but with 5 turns between
with the GTK2 client.

Available here: http://www.freeciv.org/~rfalke/leaks.tar.bz2.

Findings:

We loose 300k in 5 turns.

There is a constant (does not increase with the number of turns)
memory leak in cm.c. Because a cache was not destroyed. The attached
patch fixes this.

The remaining leaks are gtk2 related (either in the gtk lib or in the
gtk client code).

30k are caused by update_menus:
+ 15400 bytes in 110 blocks are still reachable in loss record 7697 of 7709
+    at 0x4003BE90: calloc (vg_clientfuncs.c:239)
+    by 0x405E4A90: g_malloc0 (in /usr/lib/libglib-2.0.so.0.200.0)
+    by 0x405A46C0: g_type_create_instance (in 
/usr/lib/libgobject-2.0.so.0.200.0)
+    by 0x4058F29B: (within /usr/lib/libgobject-2.0.so.0.200.0)
+    by 0x4058E939: g_object_newv (in /usr/lib/libgobject-2.0.so.0.200.0)
+    by 0x4058EFB1: g_object_new_valist (in /usr/lib/libgobject-2.0.so.0.200.0)
+    by 0x4058E5D2: g_object_new (in /usr/lib/libgobject-2.0.so.0.200.0)
+    by 0x40285ABB: gtk_accel_label_new (in /usr/lib/libgtk-x11-2.0.so.0.200.0)
+    by 0x402F094E: gtk_image_menu_item_new_with_label (in 
/usr/lib/libgtk-x11-2.0.so.0.200.0)
+    by 0x809930B: update_menus (menu.c:1057)
maybe the old entries should be unrefed/deleted/freed.

Another 30k are caused by
+ 10240 bytes in 5 blocks are still reachable in loss record 7692 of 7709
+    at 0x4003B9B4: malloc (vg_clientfuncs.c:100)
+    by 0x405E4A36: g_malloc (in /usr/lib/libglib-2.0.so.0.200.0)
+    by 0x405E5963: g_mem_chunk_alloc (in /usr/lib/libglib-2.0.so.0.200.0)
+    by 0x405D19B6: g_datalist_id_set_data_full (in 
/usr/lib/libglib-2.0.so.0.200.0)
+    by 0x40591608: g_object_set_data (in /usr/lib/libgobject-2.0.so.0.200.0)
+    by 0x404B919A: gdk_pango_context_get_for_screen (in 
/usr/lib/libgdk-x11-2.0.so.0.200.0)
+    by 0x4048E7E9: gdk_pango_context_get (in 
/usr/lib/libgdk-x11-2.0.so.0.200.0)
+    by 0x809549B: show_city_desc (mapview.c:784)
Because the result of gdk_pango_context_get must be freed. Docu says
so.

For the remaing leaks see "diff" in the tar.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "There are three ways to get something done. Do it yourself, hire someone
  to do it for you or forbid your kids to do it."

Index: client/civclient.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/civclient.c,v
retrieving revision 1.178
diff -u -u -r1.178 civclient.c
--- client/civclient.c  2003/10/09 11:59:20     1.178
+++ client/civclient.c  2003/11/05 18:21:28
@@ -636,7 +636,6 @@
 **************************************************************************/
 void client_game_free()
 {
-  cm_free();
   free_client_goto();
   free_help_texts();
   attribute_free();
Index: common/aicore/cm.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/aicore/cm.c,v
retrieving revision 1.13
diff -u -u -r1.13 cm.c
--- common/aicore/cm.c  2003/10/22 07:12:14     1.13
+++ common/aicore/cm.c  2003/11/05 18:21:30
@@ -572,6 +572,26 @@
 }
 
 /****************************************************************************
+...
+*****************************************************************************/
+static void clear_cache(void)
+{
+  int i, j;
+  for (i = 0; i < MAX_FIELDS_USED + 1; i++) {
+    for (j = 0; j < MAX_COMBINATIONS; j++) {
+      if (!cache3.results[i].combinations[j].is_valid) {
+       continue;
+      }
+      if (cache3.results[i].combinations[j].cache1) {
+       free(cache3.results[i].combinations[j].cache1);
+       cache3.results[i].combinations[j].cache1 = NULL;
+      }
+    }
+  }
+  cache3.pcity = NULL;
+}
+
+/****************************************************************************
  Uses worker_positions_used, entertainers, scientists and taxmen to
  get the remaining stats.
 *****************************************************************************/
@@ -1439,6 +1459,7 @@
   cache2.allocated_size = 0;
   cache2.allocated_trade = 0;
   cache2.allocated_luxury = 0;
+  clear_cache();
 }
 
 /****************************************************************************
@@ -1474,19 +1495,7 @@
          pcity->id);
 
   if (cache3.pcity == pcity) {
-    int i, j;
-    for (i = 0; i < MAX_FIELDS_USED + 1; i++) {
-      for (j = 0; j < MAX_COMBINATIONS; j++) {
-       if (!cache3.results[i].combinations[j].is_valid) {
-         continue;
-       }
-       if (cache3.results[i].combinations[j].cache1) {
-         free(cache3.results[i].combinations[j].cache1);
-         cache3.results[i].combinations[j].cache1 = NULL;
-       }
-      }
-    }
-    cache3.pcity = NULL;
+    clear_cache();
   }
 }