garbage collection status

Jim Meyering jim at meyering.net
Thu Jan 13 21:18:03 UTC 2011


Thought I was done converting iwhd to use garbage collection,
(modulo performance, which I have not measured), but ran everything
valgrind-enabled once again, just to be sure.  I found two leaks
triggered by the combination of a new test and the use of GC:

          2,170 (56 direct, 2,114 indirect) bytes in 1 blocks are definitely lost in loss record 324 of 332
             at 0x4A059DC: operator new(unsigned long) (vg_replace_malloc.c:220)
             by 0x410DF2: RepoMeta::NewQuery(char const*, char const*, char const *) (meta.cpp:492)
             by 0x4151C9: meta_query_new (meta.cpp:505)
             by 0x41F0C7: proxy_api_root (rest.c:973)
             by 0x41FFF4: access_handler (rest.c:1948)
             by 0x4E335F8: ??? (in /usr/lib64/libmicrohttpd.so.10.0.0)
             by 0x4E3405F: MHD_connection_handle_idle (in /usr/lib64/libmicrohttp d.so.10.0.0)
             by 0x4E36986: ??? (in /usr/lib64/libmicrohttpd.so.10.0.0)
             by 0x383BE06D5A: start_thread (pthread_create.c:301)
             by 0x383B2E4A7C: clone (clone.S:115)

          1,287,447 (1,274,700 direct, 12,747 indirect) bytes in 300 blocks are d
efinitely lost in loss record 332 of 332
             at 0x4A05E46: malloc (vg_replace_malloc.c:195)
             by 0x4E395E5: MHD_create_post_processor (in /usr/lib64/libmicrohttpd .so.10.0.0)
             by 0x422436: proxy_add_prov (rest.c:1716)
             by 0x41FFF4: access_handler (rest.c:1948)
             by 0x4E335F8: ??? (in /usr/lib64/libmicrohttpd.so.10.0.0)
             by 0x4E3405F: MHD_connection_handle_idle (in /usr/lib64/libmicrohttp d.so.10.0.0)
             by 0x4E36986: ??? (in /usr/lib64/libmicrohttpd.so.10.0.0)
             by 0x383BE06D5A: start_thread (pthread_create.c:301)
             by 0x383B2E4A7C: clone (clone.S:115)

The first is just because I've haven't bothered to make GC
work for our sole C++ compilation unit.

The big one is allocated here:

    static int
    proxy_add_prov (void *cctx, struct MHD_Connection *conn, const char *url,
                       const char *method, const char *version, const char *data,
                       size_t *data_size, void **rctx)
    {
            ...
            if (ms->state == MS_NEW) {
                    ms->state = MS_NORMAL;
                    ms->url = (char *)url;
                    ms->dict = hash_initialize(13, NULL,
                                               kv_hash, kv_compare, kv_free);
    >>>>>>>>	ms->post = MHD_create_post_processor(conn,4096,
                            prov_iterator,ms->dict);
            }

And since "ms" is now freed via garbage collection, and the allocations
performed by MHD_create_post_processor are not detected in our set-up,
GC does not know to perform the equivalent of this,

    MHD_destroy_post_processor(ms->post);

which we used to do manually in "free_ms".

Fortunately, libgc has just what I need, the GC_register_finalizer
function, with which we can tell GC to invoke that function right
before "ms" is collected.

Here's the patch I'm using now.
For the moment, this is only for reference, because it depends
on many private changes that I'll post shortly, probably tomorrow:

>From bde460a958631b37f9b96828c0b98e127d133d5c Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Thu, 13 Jan 2011 22:17:00 +0100
Subject: [PATCH] avoid a leak via ms->post = MHD_create_post_processor(...

* rest.c (gc_register_finalizer_ms_post): New function.
(proxy_query, control_api_root, proxy_bucket_post): Use it.
(proxy_object_post, proxy_add_prov): Likewise.
(destroy_state_postprocessor): New function.
---
 rest.c |   24 ++++++++++++++++++++++++
 1 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/rest.c b/rest.c
index bff9c95..fc79793 100644
--- a/rest.c
+++ b/rest.c
@@ -730,6 +730,25 @@ proxy_query_func (void *ctx, uint64_t pos, char *buf, size_t max)
 	return len;
 }

+/* Helper used by gc_register_finalizer_ms_post.  */
+static void
+destroy_state_postprocessor (void *ms_v, void *client_data)
+{
+	my_state *ms = ms_v;
+	MHD_destroy_post_processor (ms->post);
+}
+
+/* Tell the garbage collector that when freeing MS, it must invoke
+   destroy_state_postprocessor(MS).  This is required for each ms->post
+   since they're allocated via MHD_create_post_processor, which is
+   in a separate library into which the GC has no view.  */
+static void
+gc_register_finalizer_ms_post(void *ms)
+{
+	if (ms)
+		GC_register_finalizer(ms, destroy_state_postprocessor, 0, 0, 0);
+}
+
 static int
 proxy_query (void *cctx, struct MHD_Connection *conn, const char *url,
 	     const char *method, const char *version, const char *data,
@@ -748,6 +767,7 @@ proxy_query (void *cctx, struct MHD_Connection *conn, const char *url,
 		ms->state = MS_NORMAL;
 		ms->post = MHD_create_post_processor(conn,4096,
 			query_iterator,ms);
+		gc_register_finalizer_ms_post(ms);
 	}
 	else if (*data_size) {
 		MHD_post_process(ms->post,data,*data_size);
@@ -1122,6 +1142,7 @@ control_api_root (void *cctx, struct MHD_Connection *conn, const char *url,
 					   kv_hash, kv_compare, kv_free);
 		ms->post = MHD_create_post_processor(conn,4096,
 			post_iterator,ms->dict);
+		gc_register_finalizer_ms_post(ms);
 		return MHD_YES;
 	}

@@ -1187,6 +1208,7 @@ proxy_bucket_post (void *cctx, struct MHD_Connection *conn, const char *url,
 					   kv_hash, kv_compare, kv_free);
 		ms->post = MHD_create_post_processor(conn,4096,
 			post_iterator,ms->dict);
+		gc_register_finalizer_ms_post(ms);
 	}
 	else if (*data_size) {
 		MHD_post_process(ms->post,data,*data_size);
@@ -1388,6 +1410,7 @@ proxy_object_post (void *cctx, struct MHD_Connection *conn, const char *url,
 					   kv_hash, kv_compare, kv_free);
 		ms->post = MHD_create_post_processor(conn,4096,
 			post_iterator,ms->dict);
+		gc_register_finalizer_ms_post(ms);
 	}
 	else if (*data_size) {
 		MHD_post_process(ms->post,data,*data_size);
@@ -1715,6 +1738,7 @@ proxy_add_prov (void *cctx, struct MHD_Connection *conn, const char *url,
 					   kv_hash, kv_compare, kv_free);
 		ms->post = MHD_create_post_processor(conn,4096,
 			prov_iterator,ms->dict);
+		gc_register_finalizer_ms_post(ms);
 	}
 	else if (*data_size) {
 		MHD_post_process(ms->post,data,*data_size);
--
1.7.3.5


More information about the iwhd-devel mailing list