From 903c07bf98ce0158d209c90158282f44cc1ba47c Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Thu, 2 Jun 2011 09:19:56 +0200 Subject: [PATCH] API Change: uv_close only called by user - never automatically Add test that failing on_connect callback does not trigger on_close. --- test/benchmark-ping-pongs.c | 13 +++++- test/echo-server.c | 2 +- test/test-connection-fail.c | 84 ++++++++++++++++++++++++++++++++++--- test/test-list.h | 2 + uv-unix.c | 19 +++------ 5 files changed, 99 insertions(+), 21 deletions(-) diff --git a/test/benchmark-ping-pongs.c b/test/benchmark-ping-pongs.c index 3b0c3ce9..9fdd5608 100644 --- a/test/benchmark-ping-pongs.c +++ b/test/benchmark-ping-pongs.c @@ -47,7 +47,7 @@ typedef struct buf_s { static char PING[] = "PING\n"; static buf_t* buf_freelist = NULL; - +static int pinger_shutdown_cb_called; static int completed_pingers = 0; static int64_t start_time; @@ -117,6 +117,13 @@ static void pinger_write_ping(pinger_t* pinger) { static void pinger_shutdown_cb(uv_handle_t* handle, int status) { ASSERT(status == 0); + pinger_shutdown_cb_called++; + + /* + * The close callback has not been triggered yet. We must wait for EOF + * until we close the connection. + */ + ASSERT(completed_pingers == 0); } @@ -133,6 +140,9 @@ static void pinger_read_cb(uv_tcp_t* tcp, int nread, uv_buf_t buf) { buf_free(buf); } + ASSERT(pinger_shutdown_cb_called == 1); + uv_close((uv_handle_t*)tcp); + return; } @@ -146,7 +156,6 @@ static void pinger_read_cb(uv_tcp_t* tcp, int nread, uv_buf_t buf) { uv_req_init(&pinger->shutdown_req, (uv_handle_t*)tcp, pinger_shutdown_cb); uv_shutdown(&pinger->shutdown_req); break; - return; } else { pinger_write_ping(pinger); } diff --git a/test/echo-server.c b/test/echo-server.c index 069618a7..9eab707c 100644 --- a/test/echo-server.c +++ b/test/echo-server.c @@ -59,7 +59,7 @@ static void after_write(uv_req_t* req, int status) { static void after_shutdown(uv_req_t* req, int status) { - uv_close(req->handle); /* XXX Is the uv_close necessary??? */ + uv_close(req->handle); free(req); } diff --git a/test/test-connection-fail.c b/test/test-connection-fail.c index 9f8d8324..df7cdf5c 100644 --- a/test/test-connection-fail.c +++ b/test/test-connection-fail.c @@ -31,6 +31,10 @@ static uv_req_t req; static int connect_cb_calls; static int close_cb_calls; +static uv_timer_t timer; +static int timer_close_cb_calls; +static int timer_cb_calls; + static void on_close(uv_handle_t* handle, int status) { ASSERT(status == 0); @@ -38,20 +42,57 @@ static void on_close(uv_handle_t* handle, int status) { } -static void on_connect(uv_req_t *req, int status) { +static void timer_close_cb(uv_handle_t* handle, int status) { + ASSERT(status == 0); + timer_close_cb_calls++; +} + + +static void timer_cb(uv_handle_t* handle, int status) { + ASSERT(status == 0); + timer_cb_calls++; + + /* + * These are the important asserts. The connection callback has been made, + * but libuv hasn't automatically closed the socket. The user must + * uv_close the handle manually. + */ + ASSERT(close_cb_calls == 0); + ASSERT(connect_cb_calls == 1); + + /* Close the tcp handle. */ + uv_close((uv_handle_t*)&tcp); + + /* Close the timer. */ + uv_close(handle); +} + + +static void on_connect_with_close(uv_req_t *req, int status) { ASSERT(status == -1); ASSERT(uv_last_error().code == UV_ECONNREFUSED); connect_cb_calls++; + + ASSERT(close_cb_calls == 0); uv_close(req->handle); } -TEST_IMPL(connection_fail) { +static void on_connect_without_close(uv_req_t *req, int status) { + ASSERT(status == -1); + ASSERT(uv_last_error().code == UV_ECONNREFUSED); + connect_cb_calls++; + + uv_timer_start(&timer, timer_cb, 100, 0); + + ASSERT(close_cb_calls == 0); +} + + +void connection_fail(uv_connect_cb connect_cb) { struct sockaddr_in client_addr, server_addr; int r; - uv_init(); - client_addr = uv_ip4_addr("0.0.0.0", 0); /* There should be no servers listening on this port. */ @@ -63,7 +104,7 @@ TEST_IMPL(connection_fail) { /* We are never doing multiple reads/connects at a time anyway. */ /* so these handles can be pre-initialized. */ - uv_req_init(&req, (uv_handle_t*)&tcp, on_connect); + uv_req_init(&req, (uv_handle_t*)&tcp, connect_cb); uv_bind(&tcp, client_addr); r = uv_connect(&req, server_addr); @@ -73,6 +114,39 @@ TEST_IMPL(connection_fail) { ASSERT(connect_cb_calls == 1); ASSERT(close_cb_calls == 1); +} + + +/* + * This test attempts to connect to a port where no server is running. We + * expect an error. + */ +TEST_IMPL(connection_fail) { + uv_init(); + + connection_fail(on_connect_with_close); + + ASSERT(timer_close_cb_calls == 0); + ASSERT(timer_cb_calls == 0); + + return 0; +} + + +/* + * This test is the same as the first except it check that the close + * callback of the tcp handle hasn't been made after the failed connection + * attempt. + */ +TEST_IMPL(connection_fail_doesnt_auto_close) { + uv_init(); + + uv_timer_init(&timer, timer_close_cb, NULL); + + connection_fail(on_connect_without_close); + + ASSERT(timer_close_cb_calls == 1); + ASSERT(timer_cb_calls == 1); return 0; } diff --git a/test/test-list.h b/test/test-list.h index f5f633cb..f0f08e2b 100644 --- a/test/test-list.h +++ b/test/test-list.h @@ -28,6 +28,7 @@ TEST_DECLARE (bind_error_addrnotavail_2) TEST_DECLARE (bind_error_fault) TEST_DECLARE (bind_error_inval) TEST_DECLARE (connection_fail) +TEST_DECLARE (connection_fail_doesnt_auto_close) TEST_DECLARE (callback_stack) TEST_DECLARE (timer) TEST_DECLARE (timer_again) @@ -63,6 +64,7 @@ TASK_LIST_START TEST_ENTRY (bind_error_inval) TEST_ENTRY (connection_fail) + TEST_ENTRY (connection_fail_doesnt_auto_close) TEST_ENTRY (callback_stack) TEST_HELPER (callback_stack, echo_server) diff --git a/uv-unix.c b/uv-unix.c index 14a862ae..97e18038 100644 --- a/uv-unix.c +++ b/uv-unix.c @@ -343,7 +343,10 @@ void uv__server_io(EV_P_ ev_io* watcher, int revents) { return; } else { uv_err_new((uv_handle_t*)tcp, errno); - uv_close((uv_handle_t*)tcp); + /* + * XXX TODO how do we report this error to the user? What errors + * are possible here? + */ } } else { @@ -516,9 +519,8 @@ static void uv__drain(uv_tcp_t* tcp) { uv_shutdown_cb cb = req->cb; if (shutdown(tcp->fd, SHUT_WR)) { - /* Error. Nothing we can do, close the tcp. */ + /* Error. Report it. User should call uv_close(). */ uv_err_new((uv_handle_t*)tcp, errno); - uv_close((uv_handle_t*)tcp); if (cb) cb(req, -1); } else { uv_err_new((uv_handle_t*)tcp, 0); @@ -563,9 +565,6 @@ void uv__write(uv_tcp_t* tcp) { if (errno != EAGAIN) { uv_err_t err = uv_err_new((uv_handle_t*)tcp, errno); - /* XXX How do we tcp the error? Need test coverage here. */ - uv_close((uv_handle_t*)tcp); - if (cb) { cb(req, -1); } @@ -662,8 +661,8 @@ void uv__read(uv_tcp_t* tcp) { tcp->read_cb(tcp, 0, buf); return; } else { + /* Error. User should call uv_close(). */ uv_err_new((uv_handle_t*)tcp, errno); - uv_close((uv_handle_t*)tcp); tcp->read_cb(tcp, -1, buf); assert(!ev_is_active(&tcp->read_watcher)); return; @@ -673,10 +672,6 @@ void uv__read(uv_tcp_t* tcp) { uv_err_new_artificial((uv_handle_t*)tcp, UV_EOF); ev_io_stop(EV_DEFAULT_UC_ &tcp->read_watcher); tcp->read_cb(tcp, -1, buf); - - if (uv_flag_is_set((uv_handle_t*)tcp, UV_SHUT)) { - uv_close((uv_handle_t*)tcp); - } return; } else { /* Successful read */ @@ -780,8 +775,6 @@ static void uv__tcp_connect(uv_tcp_t* tcp) { if (connect_cb) { connect_cb(req, -1); } - - uv_close((uv_handle_t*)tcp); } }