Re: [gnome-db] GdaDataModelRows leaking?

[Murray Cumming <murrayc murrayc com>]

I can't see this leak in valgrind with a C test case, but I think that
is a valgrind error, because printfs show that the GdaRow dispose or
finalize handlers are never run.

On Thu, 2008-04-03 at 16:57 +0200, Vivien Malerba wrote:
> >  > the get_row() always create a new GdaRow object, but (and I agree the
> >  > names are badly chosen) the get_row vfunc is implemented by the
> >  > gda_postgres_recordset_get_row() function which first checks that the
> >  > requested GdaRow object does not already exist before calling
> >  > get_row().

The get_row vfunc is indeed set to gda_postgres_recordset_get_row(),
and that does indeed return an existing row if it already exists.

But if it does not exist then it does this the first time:

row_list = get_row (GDA_DATA_MODEL (model), priv_data, row, error);
gda_data_model_hash_insert_row (GDA_DATA_MODEL_HASH (model),
					row, row_list);

Because get_row() (not the get_row vfunc) _always_ returns a new
instance then that is leaking a reference,
because gda_data_model_hash_insert_row() takes (and releases) a
reference of its own.

I will apply the attached patch to fix this. It fixes a massive memory
leak, particularly when dealing with large tables. I guess that other
providers might have the same problem, but I am personally only
concerned with PostgreSQL.

-- 
murrayc murrayc com
www.murrayc.com
www.openismus.com

Index: providers/postgres/gda-postgres-recordset.c
===================================================================
--- providers/postgres/gda-postgres-recordset.c	(revision 3113)
+++ providers/postgres/gda-postgres-recordset.c	(working copy)
@@ -205,6 +205,7 @@
 	row_list = get_row (GDA_DATA_MODEL (model), priv_data, row, error);
 	gda_data_model_hash_insert_row (GDA_DATA_MODEL_HASH (model),
 					row, row_list);
+	g_object_unref (row_list); /* gda_data_model_hash_insert_row() holds a reference. */
 
 	return row_list;
 }
@@ -576,6 +577,8 @@
 	row_list = get_row (GDA_DATA_MODEL (model), priv_data, row, NULL);
 	gda_data_model_hash_insert_row (GDA_DATA_MODEL_HASH (model),
 					 row, row_list);
+	g_object_unref (row_list); /* gda_data_model_hash_insert_row() holds a reference. */
+
 	return gda_row_get_value (row_list, col);
 }
 
Index: ChangeLog
===================================================================
--- ChangeLog	(revision 3118)
+++ ChangeLog	(working copy)
@@ -1,3 +1,11 @@
+2008-04-06  Murray Cumming  <murrayc murrayc com>
+
+	* providers/postgres/gda-postgres-recordset.c
+	  (gda_postgres_recordset_get_row,
+	  gda_postgres_recordset_get_value_at): unref the result of get_row() 
+	(not the get_row vfunc), because gda_data_model_hash_insert_row() 
+	holds its own reference. This fixes the leak of each GdaRow.
+
 2008-04-05  Murray Cumming  <murrayc murrayc com>
 
 	* libgda/gda-connection.c (gda_connection_dispose): Unref the client,