From edd7efd43d3bc99b3b93c4cbd96ce45e98064a4e Mon Sep 17 00:00:00 2001 From: Andy Green <andy@warmcat.com> Date: Sun, 2 Sep 2018 19:23:40 +0800 Subject: [PATCH] client: libuv: fix close handling during redirect During client redirect we "reset" the wsi to the redirect address, involving closing the current fd that was told to redirect (it will usually be a completely different server or port). With libuv and its two-stage close that's not trivial. This solves the problem we will "reset" (overwrite) where the handle lives in the wsi with new a new connection / handle by having it copied out into an allocated watcher struct, which is freed in the uv close callback. To confirm it the minimal ws client example gets some new options, the original problem was replicated with this $ lws-minimal-ws-client-echo -s invalid.url.com -p 80 https://github.com/warmcat/libwebsockets/issues/1390 --- lib/event-libs/libuv/libuv.c | 10 +++++--- lib/roles/http/client/client-handshake.c | 4 ++-- .../minimal-ws-client-echo.c | 23 ++++++++++++------- 3 files changed, 24 insertions(+), 13 deletions(-) diff --git a/lib/event-libs/libuv/libuv.c b/lib/event-libs/libuv/libuv.c index ddcbc4ad..e04f4514 100644 --- a/lib/event-libs/libuv/libuv.c +++ b/lib/event-libs/libuv/libuv.c @@ -317,6 +317,7 @@ lws_libuv_closewsi_m(uv_handle_t* handle) lws_sockfd_type sockfd = (lws_sockfd_type)(lws_intptr_t)handle->data; lwsl_debug("%s: sockfd %d\n", __func__, sockfd); compatible_close(sockfd); + lws_free(handle); } int @@ -595,12 +596,15 @@ elops_check_client_connect_ok_uv(struct lws *wsi) static void elops_close_handle_manually_uv(struct lws *wsi) { - uv_handle_t *h = (void *)&wsi->w_read.uv.watcher; + struct lws_io_watcher *h = (void *)&wsi->w_read.uv.watcher, *nh; + + nh = lws_malloc(sizeof(*h), __func__); + *nh = *h; lwsl_debug("%s: lws_libuv_closehandle: wsi %p\n", __func__, wsi); - h->data = (void *)(lws_intptr_t)wsi->desc.sockfd; + ((uv_handle_t *)nh)->data = (void *)(lws_intptr_t)wsi->desc.sockfd; /* required to defer actual deletion until libuv has processed it */ - uv_close((uv_handle_t*)&wsi->w_read.uv.watcher, lws_libuv_closewsi_m); + uv_close((uv_handle_t *)nh, lws_libuv_closewsi_m); } static void diff --git a/lib/roles/http/client/client-handshake.c b/lib/roles/http/client/client-handshake.c index f64188bb..737e56b4 100644 --- a/lib/roles/http/client/client-handshake.c +++ b/lib/roles/http/client/client-handshake.c @@ -667,13 +667,13 @@ lws_client_reset(struct lws **pwsi, int ssl, const char *address, int port, lws_ssl_close(wsi); #endif + __remove_wsi_socket_from_fds(wsi); + if (wsi->context->event_loop_ops->close_handle_manually) wsi->context->event_loop_ops->close_handle_manually(wsi); else compatible_close(wsi->desc.sockfd); - __remove_wsi_socket_from_fds(wsi); - #if defined(LWS_WITH_TLS) wsi->tls.use_ssl = ssl; #else diff --git a/minimal-examples/ws-client/minimal-ws-client-echo/minimal-ws-client-echo.c b/minimal-examples/ws-client/minimal-ws-client-echo/minimal-ws-client-echo.c index cb8ce052..c28686c3 100644 --- a/minimal-examples/ws-client/minimal-ws-client-echo/minimal-ws-client-echo.c +++ b/minimal-examples/ws-client/minimal-ws-client-echo/minimal-ws-client-echo.c @@ -22,6 +22,7 @@ static struct lws_protocols protocols[] = { { NULL, NULL, 0, 0 } /* terminator */ }; +static struct lws_context *context; static int interrupted, port = 7681, options = 0; static const char *url = "/", *ads = "localhost"; @@ -87,9 +88,8 @@ void sigint_handler(int sig) int main(int argc, const char **argv) { struct lws_context_creation_info info; - struct lws_context *context; const char *p; - int n = 0, logs = LLL_USER | LLL_ERR | LLL_WARN | LLL_NOTICE + int logs = LLL_USER | LLL_ERR | LLL_WARN | LLL_NOTICE /* for LLL_ verbosity above NOTICE to be built into lws, * lws must have been configured and built with * -DCMAKE_BUILD_TYPE=DEBUG instead of =RELEASE */ @@ -97,8 +97,6 @@ int main(int argc, const char **argv) /* | LLL_EXT */ /* | LLL_CLIENT */ /* | LLL_LATENCY */ /* | LLL_DEBUG */; - signal(SIGINT, sigint_handler); - if ((p = lws_cmdline_option(argc, argv, "-d"))) logs = atoi(p); @@ -115,7 +113,10 @@ int main(int argc, const char **argv) if (lws_cmdline_option(argc, argv, "-o")) options |= 1; - lwsl_user("options %d\n", options); + if ((p = lws_cmdline_option(argc, argv, "-s"))) + ads = p; + + lwsl_user("options %d, ads %s\n", options, ads); memset(&info, 0, sizeof info); /* otherwise uninitialized garbage */ info.port = CONTEXT_PORT_NO_LISTEN; @@ -124,7 +125,13 @@ int main(int argc, const char **argv) if (!lws_cmdline_option(argc, argv, "-n")) info.extensions = extensions; info.pt_serv_buf_size = 32 * 1024; - info.options = LWS_SERVER_OPTION_VALIDATE_UTF8; + info.options = LWS_SERVER_OPTION_DO_SSL_GLOBAL_INIT | + LWS_SERVER_OPTION_VALIDATE_UTF8; + + if (lws_cmdline_option(argc, argv, "--libuv")) + info.options |= LWS_SERVER_OPTION_LIBUV; + else + signal(SIGINT, sigint_handler); context = lws_create_context(&info); if (!context) { @@ -132,8 +139,8 @@ int main(int argc, const char **argv) return 1; } - while (n >= 0 && !interrupted) - n = lws_service(context, 1000); + while (!lws_service(context, 1000) && !interrupted) + ; lws_context_destroy(context); -- GitLab