[PATCH 1/2] iwhd: avoid segfault during garbage collection

Jim Meyering jim at meyering.net
Wed Jun 8 20:42:48 UTC 2011


Here are two patches.
One or both of them contribute to making segfaults -- triggered
during garbage collection -- a lot less common for some work-loads.
I hope to construct a test case that triggers this reliably
tomorrow, and then I'll know more.

>From f8e97f41d4c4129120530082bfeb0e24a0953003 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Wed, 8 Jun 2011 16:20:22 +0200
Subject: [PATCH 1/2] iwhd: avoid segfault during garbage collection

Any function that is called in its own thread
via MHD_create_response_from_callback must begin with a call to
gc_register_thread, and end with a call to GC_unregister_my_thread.
For each of the following functions, since all but one has multiple
return points, create a trivial wrapper function with the same name
as the original function and rename the original to ORIGINAL_0.
The trivial wrapper ensures that the register/unregister calls
are made and returns the result.
* rest.c (proxy_get_cons): As above.
(proxy_query_func): Likewise.
(root_blob_generator): Likewise.
(parts_callback): Likewise.
* NEWS (Bug fixes): Mention it.
---
 NEWS   |    3 +++
 rest.c |   50 ++++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 49 insertions(+), 4 deletions(-)

diff --git a/NEWS b/NEWS
index 110500b..b107f87 100644
--- a/NEWS
+++ b/NEWS
@@ -8,6 +8,9 @@ iwhd NEWS                                                   -*- outline -*-

 ** Bug fixes

+  iwhd will segfault less often during garbage-collection.
+  [bug introduced in 0.91, partially fixed in 0.94]
+
   the iwhd init script start function would mistakenly succeed even when
   mongod failed to start in time.  Now, when mongod fails to start, that
   "start" function also fails.
diff --git a/rest.c b/rest.c
index 9c39673..73cf350 100644
--- a/rest.c
+++ b/rest.c
@@ -210,13 +210,14 @@ child_closer (void * ctx)

 /* Invoked from MHD. */
 static ssize_t
-proxy_get_cons (void *ctx, uint64_t pos, char *buf, size_t max)
+proxy_get_cons_0 (void *ctx, uint64_t pos, char *buf, size_t max)
 {
 	pipe_private	*pp	= ctx;
 	pipe_shared	*ps	= pp->shared;
 	my_state	*ms	= ps->owner;
 	ssize_t		 done;

+	gc_register_thread();
 	(void)pos;

 	DPRINTF("consumer asked to read %zu\n",max);
@@ -260,6 +261,17 @@ proxy_get_cons (void *ctx, uint64_t pos, char *buf, size_t max)
 	}

 	return done;
+	GC_unregister_my_thread();
+}
+
+/* This is just a wrapper.  See the *_0 function, above.  */
+static ssize_t
+proxy_get_cons (void *ctx, uint64_t pos, char *buf, size_t max)
+{
+	gc_register_thread();
+	ssize_t len = proxy_get_cons_0 (ctx, pos, buf, max);
+	GC_unregister_my_thread();
+	return len;
 }

 static int
@@ -778,7 +790,7 @@ proxy_query_init (my_state *ms, const char *expr)

 /* MHD reader function during queries.  Return -1 for EOF. */
 static ssize_t
-proxy_query_func (void *ctx, uint64_t pos, char *buf, size_t max)
+proxy_query_func_0 (void *ctx, uint64_t pos, char *buf, size_t max)
 {
 	my_state	*ms	= ctx;
 	size_t		 len;
@@ -833,6 +845,16 @@ proxy_query_func (void *ctx, uint64_t pos, char *buf, size_t max)
 	return len;
 }

+/* This is just a wrapper.  See the *_0 function, above.  */
+static ssize_t
+proxy_query_func (void *ctx, uint64_t pos, char *buf, size_t max)
+{
+	gc_register_thread();
+	ssize_t len = proxy_query_func_0 (ctx, pos, buf, max);
+	GC_unregister_my_thread();
+	return len;
+}
+
 /* Helper used by gc_register_finalizer_ms.  */
 static void
 destroy_state_postprocessor (void *ms_v, void *client_data)
@@ -1024,7 +1046,7 @@ static const fake_bucket_t fake_bucket_list[] = {
 };

 static ssize_t
-root_blob_generator (void *ctx, uint64_t pos, char *buf, size_t max)
+root_blob_generator_0 (void *ctx, uint64_t pos, char *buf, size_t max)
 {
 	my_state	*ms	= ctx;
 	const fake_bucket_t *fb;
@@ -1098,6 +1120,16 @@ root_blob_generator (void *ctx, uint64_t pos, char *buf, size_t max)
 	return len;
 }

+/* This is just a wrapper.  See the *_0 function, above.  */
+static ssize_t
+root_blob_generator (void *ctx, uint64_t pos, char *buf, size_t max)
+{
+	gc_register_thread();
+	ssize_t len = root_blob_generator_0 (ctx, pos, buf, max);
+	GC_unregister_my_thread();
+	return len;
+}
+
 static int
 proxy_api_root (void *cctx, struct MHD_Connection *conn, const char *url,
 		const char *method, const char *version, const char *data,
@@ -1429,7 +1461,7 @@ register_image (my_state *ms)
 }

 static ssize_t
-parts_callback (void *ctx, uint64_t pos, char *buf, size_t max)
+parts_callback_0 (void *ctx, uint64_t pos, char *buf, size_t max)
 {
 	my_state	*ms	= ctx;
 	size_t		 len;
@@ -1497,6 +1529,16 @@ parts_callback (void *ctx, uint64_t pos, char *buf, size_t max)
 	return len;
 }

+/* This is just a wrapper.  See the *_0 function, above.  */
+static ssize_t
+parts_callback (void *ctx, uint64_t pos, char *buf, size_t max)
+{
+	gc_register_thread();
+	ssize_t len = parts_callback_0 (ctx, pos, buf, max);
+	GC_unregister_my_thread();
+	return len;
+}
+
 static int
 show_parts (struct MHD_Connection *conn, my_state *ms)
 {
--
1.7.6.rc0.293.g40857


>From 892757eb29c4cff28ddbdafc4027866ff2962db8 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Wed, 8 Jun 2011 19:58:41 +0200
Subject: [PATCH 2/2] iwhd: don't explicitly free C++-allocated ms->query or
 ms->aquery

* gc-wrap.h [__cplusplus]: Include "gc_cpp.h".  This enables the
GC machinery also for our sole C++ compilation unit: meta.cpp.
* rest.c (destroy_state_postprocessor): Now that we've GC-enabled
meta.cpp, there's no need to call meta_query_stop for ms->query
and ms->aquery.
---
 gc-wrap.h |    6 ++++--
 rest.c    |    4 ----
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/gc-wrap.h b/gc-wrap.h
index 70c4b9d..f337e62 100644
--- a/gc-wrap.h
+++ b/gc-wrap.h
@@ -1,8 +1,10 @@
 #include <string.h>
 #define GC_THREADS
-#include "gc.h"

-#ifndef __cplusplus
+#ifdef __cplusplus
+# include "gc_cpp.h"
+#else
+# include "gc.h"
 # define malloc(n) GC_MALLOC(n)
 # define calloc(m,n) GC_MALLOC((m)*(n))
 # define free(p) GC_FREE(p)
diff --git a/rest.c b/rest.c
index 73cf350..7aac7d3 100644
--- a/rest.c
+++ b/rest.c
@@ -864,10 +864,6 @@ destroy_state_postprocessor (void *ms_v, void *client_data)
 		MHD_destroy_post_processor (ms->post);
 	if (ms->dict)
 		hash_free (ms->dict);
-	if (ms->query)
-		meta_query_stop (ms->query);
-	if (ms->aquery)
-		meta_query_stop (ms->aquery);
 }

 /* Tell the garbage collector that when freeing MS, it must invoke
--
1.7.6.rc0.293.g40857


More information about the iwhd-devel mailing list