[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