From 5eae09540b14bdd0e56e4aac8353eef5bd30c373 Mon Sep 17 00:00:00 2001 From: Andy Green Date: Thu, 1 Aug 2019 18:31:11 +0100 Subject: [PATCH] lws_lookup: fix wsi table when unrelated_to_ulimit The logic in the loops for insertion and deletion from the mini, forced to non ulimit max fds in the pt mode was not quite right. It showed up in hard to reproduce problem with the ws client spam test that uses the mini mode, on travis. This should fix the root cause. --- lib/core-net/pollfd.c | 53 ++++++++++++------- lib/plat/unix/unix-fds.c | 39 +++++++++++++- .../minimal-ws-client-spam.c | 2 +- 3 files changed, 71 insertions(+), 23 deletions(-) diff --git a/lib/core-net/pollfd.c b/lib/core-net/pollfd.c index 3543f98e6..6ba677bc1 100644 --- a/lib/core-net/pollfd.c +++ b/lib/core-net/pollfd.c @@ -304,14 +304,14 @@ __remove_wsi_socket_from_fds(struct lws *wsi) struct lws_pollargs pa = { wsi->desc.sockfd, 0, 0 }; struct lws_context_per_thread *pt = &context->pt[(int)wsi->tsi]; struct lws *end_wsi; - int v; - int m, ret = 0; + int v, m, ret = 0; #if !defined(_WIN32) if (!wsi->context->max_fds_unrelated_to_ulimit && wsi->desc.sockfd - lws_plat_socket_offset() > context->max_fds) { lwsl_err("fd %d too high (%d)\n", wsi->desc.sockfd, context->max_fds); + return 1; } #endif @@ -335,31 +335,44 @@ __remove_wsi_socket_from_fds(struct lws *wsi) LWS_EV_STOP | LWS_EV_READ | LWS_EV_WRITE | LWS_EV_PREPARE_DELETION); -// lwsl_debug("%s: wsi=%p, skt=%d, fds pos=%d, end guy pos=%d, endfd=%d\n", -// __func__, wsi, wsi->desc.sockfd, wsi->position_in_fds_table, -// pt->fds_count, pt->fds[pt->fds_count].fd); + /* + lwsl_debug("%s: wsi=%p, skt=%d, fds pos=%d, end guy pos=%d, endfd=%d\n", + __func__, wsi, wsi->desc.sockfd, wsi->position_in_fds_table, + pt->fds_count, pt->fds[pt->fds_count - 1].fd); */ if (m != LWS_NO_FDS_POS) { + char fixup = 0; - /* have the last guy take up the now vacant slot */ - pt->fds[m] = pt->fds[pt->fds_count - 1]; - /* this decrements pt->fds_count */ - lws_plat_delete_socket_from_fds(context, wsi, m); - pt->count_conns--; - v = (int) pt->fds[m].fd; - /* end guy's "position in fds table" is now the deletion - * guy's old one */ - end_wsi = wsi_from_fd(context, v); - if (!end_wsi) { - lwsl_err("no wsi for fd %d pos %d, pt->fds_count=%d\n", - (int)pt->fds[m].fd, m, pt->fds_count); - assert(0); - } else - end_wsi->position_in_fds_table = m; + assert(pt->fds_count && (unsigned int)m != pt->fds_count); /* deletion guy's lws_lookup entry needs nuking */ delete_from_fd(context, wsi->desc.sockfd); + if ((unsigned int)m != pt->fds_count - 1) { + /* have the last guy take up the now vacant slot */ + pt->fds[m] = pt->fds[pt->fds_count - 1]; + fixup = 1; + } + + pt->fds[pt->fds_count - 1].fd = -1; + + /* this decrements pt->fds_count */ + lws_plat_delete_socket_from_fds(context, wsi, m); + pt->count_conns--; + if (fixup) { + v = (int) pt->fds[m].fd; + /* old end guy's "position in fds table" is now the + * deletion guy's old one */ + end_wsi = wsi_from_fd(context, v); + if (!end_wsi) { + lwsl_err("no wsi for fd %d pos %d, " + "pt->fds_count=%d\n", + (int)pt->fds[m].fd, m, pt->fds_count); + assert(0); + } else + end_wsi->position_in_fds_table = m; + } + /* removed wsi has no position any more */ wsi->position_in_fds_table = LWS_NO_FDS_POS; } diff --git a/lib/plat/unix/unix-fds.c b/lib/plat/unix/unix-fds.c index 23245f009..221d96705 100644 --- a/lib/plat/unix/unix-fds.c +++ b/lib/plat/unix/unix-fds.c @@ -64,6 +64,29 @@ insert_wsi(const struct lws_context *context, struct lws *wsi) p = context->lws_lookup; done = &p[context->max_fds]; +#if defined(_DEBUG) + + /* confirm it doesn't already exist */ + + while (p != done && *p != wsi) + p++; + + assert(p == done); + p = context->lws_lookup; + + /* confirm fd doesn't already exist */ + + while (p != done && (!*p || (*p && (*p)->desc.sockfd != wsi->desc.sockfd))) + p++; + + if (p != done) { + lwsl_err("%s: wsi %p already says it has fd %d\n", + __func__, *p, wsi->desc.sockfd); + assert(0); + } + p = context->lws_lookup; +#endif + /* find an empty slot */ while (p != done && *p) @@ -96,15 +119,27 @@ delete_from_fd(const struct lws_context *context, int fd) p = context->lws_lookup; done = &p[context->max_fds]; - /* find an empty slot */ + /* find the match */ - while (p != done && *p && (*p)->desc.sockfd != fd) + while (p != done && (!*p || (*p && (*p)->desc.sockfd != fd))) p++; if (p == done) lwsl_err("%s: fd %d not found\n", __func__, fd); else *p = NULL; + +#if defined(_DEBUG) + p = context->lws_lookup; + while (p != done && (!*p || (*p && (*p)->desc.sockfd != fd))) + p++; + + if (p != done) { + lwsl_err("%s: fd %d in lws_lookup again at %d\n", __func__, + fd, (int)(p - context->lws_lookup)); + assert(0); + } +#endif } void diff --git a/minimal-examples/ws-client/minimal-ws-client-spam/minimal-ws-client-spam.c b/minimal-examples/ws-client/minimal-ws-client-spam/minimal-ws-client-spam.c index 9c3065fe9..2f278e770 100644 --- a/minimal-examples/ws-client/minimal-ws-client-spam/minimal-ws-client-spam.c +++ b/minimal-examples/ws-client/minimal-ws-client-spam/minimal-ws-client-spam.c @@ -32,7 +32,7 @@ static struct client clients[200]; static int interrupted, port = 443, ssl_connection = LCCSCF_USE_SSL; static const char *server_address = "libwebsockets.org", *pro = "lws-mirror-protocol"; -static int concurrent = 10, conn, tries, est, errors, closed, sent, limit = 100; +static int concurrent = 10, conn, tries, est, errors, closed, sent, limit = 100; struct pss { int conn;