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