two query-related bugs fixed

Jim Meyering jim at meyering.net
Fri Jun 10 16:26:19 UTC 2011


I've pushed the two bug fixes I'd already posted here
along with a new test for the former one.

Also, there were spurious gnulib test failures,
so I updated to the latest gnulib to fix them.

>From ceedf12a30b89a2d0652bde874f4ed73ed55c2e7 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Fri, 10 Jun 2011 18:17:09 +0200
Subject: [PATCH 1/4] build: update gnulib submodule to latest

---
 gnulib |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/gnulib b/gnulib
index 79aac72..f462d86 160000
--- a/gnulib
+++ b/gnulib
@@ -1 +1 @@
-Subproject commit 79aac72351f56c4998c4cd176a7fd30a1598972b
+Subproject commit f462d86108d41b1ca0f48b51caad6c696d67d8fd
--
1.7.6.rc0.293.g40857


>From 4881da1c5dac3a8b3c1a831bc6c98a809ad9d5b1 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 2/4] 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, 48 insertions(+), 5 deletions(-)

diff --git a/NEWS b/NEWS
index 32ec51f..d88d604 100644
--- a/NEWS
+++ b/NEWS
@@ -8,7 +8,8 @@ iwhd NEWS                                                   -*- outline -*-

 ** Bug fixes

-  a query_ with a result longer than FIXME would evoke a segfault
+  iwhd will segfault or hang 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
diff --git a/rest.c b/rest.c
index 315a941..06230ec 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
@@ -788,7 +800,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;
@@ -843,6 +855,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)
@@ -1034,7 +1056,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;
@@ -1108,6 +1130,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,
@@ -1439,7 +1471,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;
@@ -1507,6 +1539,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 3814a120275cc566caba9155edf3883f31405f26 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 3/4] 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 06230ec..af022af 100644
--- a/rest.c
+++ b/rest.c
@@ -874,10 +874,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


>From d0be22036399345441d4cdd154ad89e70ffd8dc3 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Fri, 10 Jun 2011 17:03:50 +0200
Subject: [PATCH 4/4] tests: improve some comments

* t/basic: Better comments for just-added test.
---
 t/basic |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/t/basic b/t/basic
index ccfc54d..e4aec0a 100644
--- a/t/basic
+++ b/t/basic
@@ -164,13 +164,16 @@ tr -s '\t \n' ' ' < q.xml > k && mv k q.xml
 printf '[ { "bucket": "b99", "key": "my_file" } ] ' > exp.xml
 compare q.xml exp.xml || fail=1

-# Run a query that produces nontrivial output by creating many
-# objects that match:
+# Run a query that produces nontrivial output;
+# First, create many objects that will match it.
+# Before iwhd-0.96, this would hang consistently around i==390
+# with "contention in meta_query_stop".
 for i in $(seq 500); do
   obj=http://localhost:$port/b99/99999999999999999999-$i
   echo x | curl -T - $obj
   printf mock | curl -T - $obj/target
 done
+# Run the query:
 curl -H 'Accept: */json' -d '$target=="mock"' \
     http://localhost:$port/b99/_query > q
 cat q
--
1.7.6.rc0.293.g40857


More information about the iwhd-devel mailing list