Sorry for the delay. Here's a test case. I'm not sure if you'll like my use of the limited broadcast address for simulating an ENETUNREACH error with a TCP connection, but it's the best that I could think of. Basically, we want to trigger a non-EINPROGRESS error in evutil_socket_connect() immediately at the connect() in order to bring about the assertion in the evhttp_connection_fail() error handling code.
diff --git a/test/regress_http.c b/test/regress_http.c
index 95511ae..9c49cae 100644
--- a/test/regress_http.c
+++ b/test/regress_http.c
@@ -3008,6 +3008,65 @@ http_stream_in_cancel_test(void *arg)
}
static void
+http_connection_fail_done(struct evhttp_request *req, void *arg)
+{
+ /* An ENETUNREACH error results in an unrecoverable
+ * evhttp_connection error (see evhttp_connection_fail()). The
+ * connection will be reset, and the user will be notified with a NULL
+ * req parameter. */
+ tt_assert(!req);
+
+ test_ok = 1;
+
+ end:
+ event_base_loopexit(arg, NULL);
+}
+
+/* Test unrecoverable evhttp_connection errors by generating an ENETUNREACH
+ * error on connection. */
+static void
+http_connection_fail_test(void *arg)
+{
+ struct basic_test_data *data = "">
+ ev_uint16_t port = 0;
+ struct evhttp_connection *evcon = NULL;
+ struct evhttp_request *req = NULL;
+
+ exit_base = data->base;
+ test_ok = 0;
+
+ /* auto detect a port */
+ http = http_setup(&port, data->base);
+ evhttp_free(http);
+ http = NULL;
+
+ /* Pick an unroutable address. The limited broadcast address should do
+ * when working with TCP. */
+ evcon = evhttp_connection_base_new(data->base, NULL, "255.255.255.255",
+ tt_assert(evcon);
+
+ /*
+ * At this point, we want to schedule an HTTP GET request
+ * server using our make request method.
+ */
+
+ req = evhttp_request_new(http_connection_fail_done, data->base);
+ tt_assert(req);
+
+ if (evhttp_make_request(evcon, req, EVHTTP_REQ_GET, "/") == -1) {
+ tt_abort_msg("Couldn't make request");
+ }
+
+ event_base_dispatch(data->base);
+
+ tt_int_op(test_ok, ==, 1);
+
+ end:
+ if (evcon)
+ evhttp_connection_free(evcon);
+}
+
+static void
http_connection_retry_done(struct evhttp_request *req, void *arg)
{
tt_assert(req);
@@ -3546,6 +3605,7 @@ struct testcase_t http_testcases[] = {
HTTP(stream_in),
HTTP(stream_in_cancel),
+ HTTP(connection_fail),
HTTP(connection_retry),
HTTP(data_length_constraints),
***** end patch *****
On Tue, May 10, 2011 at 2:46 AM, Kevin Ko <
kevin.s.ko@xxxxxxxxx> wrote:
> Hi,
>
> Patch in question:
> - Fix the case when failed evhttp_make_request() leaved request in the queue.
> -
http://levent.git.sourceforge.net/git/gitweb.cgi?p=levent/libevent;a=commit;h=0d6622e
>
> The above patch introduces a failing assertion in
> evhttp_connection_fail(). This happens because the patch defers the
> assignment of the outstanding request to the evcon->requests list,
> while evhttp_connection_fail() assumes that the request lies in the
> list.
>
> One scenario in which this can happen is when the request list is
> empty and a connection is made to an unreachable host. The assertion
> will then fail after bufferevent_socket_connect() errors out (with
> ENETUNREACH in my case).
>
> At the end of this message, I've attached a gdb backtrace of an
> instance with the failed assertion and a patch that should revise the
> original solution to have an effect only when
> evhttp_connection_connect() fails.
echo Cliff's request for a test case to exercise the behavior here and
make sure that it's actually doing the right thing.
***********************************************************************
To unsubscribe, send an e-mail to
majordomo@xxxxxxxxxxxxx with
unsubscribe libevent-users in the body.