From c2e419910f79eded983cdb15ef67eba3fa595618 Mon Sep 17 00:00:00 2001 From: xtne6f Date: Tue, 14 Jun 2016 19:08:52 +0900 Subject: [PATCH 1/5] Remove listening_ports dynamic array "listening_sockets" has duplicate information, so we have only to update this. Related commit is 2a4245e379df8cc69368291c99837ea5a86cfeec . --- src/civetweb.c | 58 +++++++++++++++++++++++++++++--------------------- 1 file changed, 34 insertions(+), 24 deletions(-) diff --git a/src/civetweb.c b/src/civetweb.c index 98130f53..4decef6e 100644 --- a/src/civetweb.c +++ b/src/civetweb.c @@ -1244,7 +1244,6 @@ struct mg_context { int context_type; /* 1 = server context, 2 = client context */ struct socket *listening_sockets; - in_port_t *listening_ports; unsigned int num_listening_sockets; volatile int @@ -1782,7 +1781,13 @@ mg_get_ports(const struct mg_context *ctx, size_t size, int *ports, int *ssl) } for (i = 0; i < size && i < ctx->num_listening_sockets; i++) { ssl[i] = ctx->listening_sockets[i].is_ssl; - ports[i] = ctx->listening_ports[i]; + ports[i] = +#if defined(USE_IPV6) + (ctx->listening_sockets[i].lsa.sa.sa_family == AF_INET6) + ? ntohs(ctx->listening_sockets[i].lsa.sin6.sin6_port) + : +#endif + ntohs(ctx->listening_sockets[i].lsa.sin.sin_port); } return i; } @@ -1802,13 +1807,19 @@ mg_get_server_ports(const struct mg_context *ctx, if (!ctx) { return -1; } - if (!ctx->listening_sockets || !ctx->listening_ports) { + if (!ctx->listening_sockets) { return -1; } for (i = 0; (i < size) && (i < (int)ctx->num_listening_sockets); i++) { - ports[cnt].port = ctx->listening_ports[i]; + ports[cnt].port = +#if defined(USE_IPV6) + (ctx->listening_sockets[i].lsa.sa.sa_family == AF_INET6) + ? ntohs(ctx->listening_sockets[i].lsa.sin6.sin6_port) + : +#endif + ntohs(ctx->listening_sockets[i].lsa.sin.sin_port); ports[cnt].is_ssl = ctx->listening_sockets[i].is_ssl; ports[cnt].is_redirect = ctx->listening_sockets[i].ssl_redir; @@ -9399,6 +9410,13 @@ redirect_to_https_port(struct mg_connection *conn, int ssl_index) mg_printf(conn, "HTTP/1.1 302 Found\r\nLocation: https://%s:%d%s%s%s\r\n\r\n", host, +#if defined(USE_IPV6) + (conn->ctx->listening_sockets[ssl_index].lsa.sa.sa_family + == AF_INET6) ? + (int)ntohs( + conn->ctx->listening_sockets[ssl_index].lsa.sin6.sin6_port + ) : +#endif (int)ntohs( conn->ctx->listening_sockets[ssl_index].lsa.sin.sin_port), conn->request_info.local_uri, @@ -10240,8 +10258,6 @@ close_all_listening_sockets(struct mg_context *ctx) } mg_free(ctx->listening_sockets); ctx->listening_sockets = NULL; - mg_free(ctx->listening_ports); - ctx->listening_ports = NULL; } @@ -10315,7 +10331,6 @@ set_ports_option(struct mg_context *ctx) struct vec vec; struct socket so, *ptr; - in_port_t *portPtr; union usa usa; socklen_t len; @@ -10459,7 +10474,8 @@ set_ports_option(struct mg_context *ctx) continue; } - if (getsockname(so.sock, &(usa.sa), &len) != 0) { + if (getsockname(so.sock, &(usa.sa), &len) != 0 + || usa.sa.sa_family != so.lsa.sa.sa_family) { int err = (int)ERRNO; mg_cry(fc(ctx), @@ -10473,6 +10489,16 @@ set_ports_option(struct mg_context *ctx) continue; } + /* Update lsa port in case of random free ports */ +#if defined(USE_IPV6) + if (so.lsa.sa.sa_family == AF_INET6) { + so.lsa.sin6.sin6_port = usa.sin6.sin6_port; + } else +#endif + { + so.lsa.sin.sin_port = usa.sin.sin_port; + } + if ((ptr = (struct socket *) mg_realloc(ctx->listening_sockets, (ctx->num_listening_sockets + 1) @@ -10484,25 +10510,9 @@ set_ports_option(struct mg_context *ctx) continue; } - if ((portPtr = - (in_port_t *)mg_realloc(ctx->listening_ports, - (ctx->num_listening_sockets + 1) - * sizeof(ctx->listening_ports[0]))) - == NULL) { - - mg_cry(fc(ctx), "%s", "Out of memory"); - closesocket(so.sock); - so.sock = INVALID_SOCKET; - mg_free(ptr); - continue; - } - set_close_on_exec(so.sock, fc(ctx)); ctx->listening_sockets = ptr; ctx->listening_sockets[ctx->num_listening_sockets] = so; - ctx->listening_ports = portPtr; - ctx->listening_ports[ctx->num_listening_sockets] = - ntohs(usa.sin.sin_port); ctx->num_listening_sockets++; portsOk++; } From 1c0212798cfd29609a76b94866ab04e76ecdc758 Mon Sep 17 00:00:00 2001 From: xtne6f Date: Tue, 14 Jun 2016 19:39:05 +0900 Subject: [PATCH 2/5] Preallocate memory for pollfds Because master_thread has no proper way to exit when allocation failed. --- src/civetweb.c | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/src/civetweb.c b/src/civetweb.c index 4decef6e..0aae38ff 100644 --- a/src/civetweb.c +++ b/src/civetweb.c @@ -1244,6 +1244,7 @@ struct mg_context { int context_type; /* 1 = server context, 2 = client context */ struct socket *listening_sockets; + struct pollfd *listening_socket_fds; unsigned int num_listening_sockets; volatile int @@ -10258,6 +10259,8 @@ close_all_listening_sockets(struct mg_context *ctx) } mg_free(ctx->listening_sockets); ctx->listening_sockets = NULL; + mg_free(ctx->listening_socket_fds); + ctx->listening_socket_fds = NULL; } @@ -10331,6 +10334,7 @@ set_ports_option(struct mg_context *ctx) struct vec vec; struct socket so, *ptr; + struct pollfd *pfd; union usa usa; socklen_t len; @@ -10510,9 +10514,23 @@ set_ports_option(struct mg_context *ctx) continue; } + if ((pfd = (struct pollfd *) + mg_realloc(ctx->listening_socket_fds, + (ctx->num_listening_sockets + 1) + * sizeof(ctx->listening_socket_fds[0]))) + == NULL) { + + mg_cry(fc(ctx), "%s", "Out of memory"); + closesocket(so.sock); + so.sock = INVALID_SOCKET; + mg_free(ptr); + continue; + } + set_close_on_exec(so.sock, fc(ctx)); ctx->listening_sockets = ptr; ctx->listening_sockets[ctx->num_listening_sockets] = so; + ctx->listening_socket_fds = pfd; ctx->num_listening_sockets++; portsOk++; } @@ -12641,10 +12659,9 @@ master_thread_run(void *thread_func_param) /* Server starts *now* */ ctx->start_time = time(NULL); - /* Allocate memory for the listening sockets, and start the server */ - pfd = - (struct pollfd *)mg_calloc(ctx->num_listening_sockets, sizeof(pfd[0])); - while (pfd != NULL && ctx->stop_flag == 0) { + /* Start the server */ + pfd = ctx->listening_socket_fds; + while (ctx->stop_flag == 0) { for (i = 0; i < ctx->num_listening_sockets; i++) { pfd[i].fd = ctx->listening_sockets[i].sock; pfd[i].events = POLLIN; @@ -12663,7 +12680,6 @@ master_thread_run(void *thread_func_param) } } } - mg_free(pfd); DEBUG_TRACE("%s", "stopping workers"); /* Stop signal received: somebody called mg_stop. Quit. */ From e071d4691e6b49cee431e16b3d7941cac23c9559 Mon Sep 17 00:00:00 2001 From: xtne6f Date: Tue, 14 Jun 2016 19:52:49 +0900 Subject: [PATCH 3/5] Fix unlocked broadcast Since the purpose of this broadcast is to notify stop_flag, locking is once needed. --- src/civetweb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/civetweb.c b/src/civetweb.c index 0aae38ff..c2c4197f 100644 --- a/src/civetweb.c +++ b/src/civetweb.c @@ -12686,10 +12686,10 @@ master_thread_run(void *thread_func_param) close_all_listening_sockets(ctx); /* Wakeup workers that are waiting for connections to handle. */ + (void)pthread_mutex_lock(&ctx->thread_mutex); pthread_cond_broadcast(&ctx->sq_full); /* Wait until all threads finish */ - (void)pthread_mutex_lock(&ctx->thread_mutex); while (ctx->running_worker_threads > 0) { (void)pthread_cond_wait(&ctx->thread_cond, &ctx->thread_mutex); } From faf6f1f3de4d23ef8d84b643256ee8c318d700f7 Mon Sep 17 00:00:00 2001 From: xtne6f Date: Tue, 14 Jun 2016 19:57:11 +0900 Subject: [PATCH 4/5] Remove an useless condition This must be an old relic to join threads. --- src/civetweb.c | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/src/civetweb.c b/src/civetweb.c index c2c4197f..7b80635b 100644 --- a/src/civetweb.c +++ b/src/civetweb.c @@ -1247,10 +1247,7 @@ struct mg_context { struct pollfd *listening_socket_fds; unsigned int num_listening_sockets; - volatile int - running_worker_threads; /* Number of currently running worker threads */ pthread_mutex_t thread_mutex; /* Protects (max|num)_threads */ - pthread_cond_t thread_cond; /* Condvar for tracking workers terminations */ struct socket queue[MGSQLEN]; /* Accepted sockets */ volatile int sq_head; /* Head of the socket queue */ @@ -12458,13 +12455,6 @@ worker_thread_run(void *thread_func_param) } } - /* Signal master that we're done with connection and exiting */ - (void)pthread_mutex_lock(&ctx->thread_mutex); - ctx->running_worker_threads--; - (void)pthread_cond_signal(&ctx->thread_cond); - /* assert(ctx->running_worker_threads >= 0); */ - (void)pthread_mutex_unlock(&ctx->thread_mutex); - pthread_setspecific(sTlsKey, NULL); #if defined(_WIN32) && !defined(__SYMBIAN32__) CloseHandle(tls.pthread_cond_helper_mutex); @@ -12688,11 +12678,6 @@ master_thread_run(void *thread_func_param) /* Wakeup workers that are waiting for connections to handle. */ (void)pthread_mutex_lock(&ctx->thread_mutex); pthread_cond_broadcast(&ctx->sq_full); - - /* Wait until all threads finish */ - while (ctx->running_worker_threads > 0) { - (void)pthread_cond_wait(&ctx->thread_cond, &ctx->thread_mutex); - } (void)pthread_mutex_unlock(&ctx->thread_mutex); /* Join all worker threads to avoid leaking threads. */ @@ -12757,7 +12742,6 @@ free_context(struct mg_context *ctx) * condvars */ (void)pthread_mutex_destroy(&ctx->thread_mutex); - (void)pthread_cond_destroy(&ctx->thread_cond); (void)pthread_cond_destroy(&ctx->sq_empty); (void)pthread_cond_destroy(&ctx->sq_full); @@ -12957,7 +12941,6 @@ mg_start(const struct mg_callbacks *callbacks, #endif ok = 0 == pthread_mutex_init(&ctx->thread_mutex, &pthread_mutex_attr); - ok &= 0 == pthread_cond_init(&ctx->thread_cond, NULL); ok &= 0 == pthread_cond_init(&ctx->sq_empty, NULL); ok &= 0 == pthread_cond_init(&ctx->sq_full, NULL); ok &= 0 == pthread_mutex_init(&ctx->nonce_mutex, &pthread_mutex_attr); @@ -13085,15 +13068,9 @@ mg_start(const struct mg_callbacks *callbacks, /* Start worker threads */ for (i = 0; i < ctx->cfg_worker_threads; i++) { - (void)pthread_mutex_lock(&ctx->thread_mutex); - ctx->running_worker_threads++; - (void)pthread_mutex_unlock(&ctx->thread_mutex); if (mg_start_thread_with_id(worker_thread, ctx, &ctx->workerthreadids[i]) != 0) { - (void)pthread_mutex_lock(&ctx->thread_mutex); - ctx->running_worker_threads--; - (void)pthread_mutex_unlock(&ctx->thread_mutex); if (i > 0) { mg_cry(fc(ctx), "Cannot start worker thread %i: error %ld", From 51c2afe8f1c582ccb73c0572f1cdebd6b3e44b3a Mon Sep 17 00:00:00 2001 From: xtne6f Date: Tue, 14 Jun 2016 22:03:21 +0900 Subject: [PATCH 5/5] Fix and improve pthread_cond in Windows Fix possible overrun of waitingthreadhdls in case of pthread_cond_timedwait() with abstime (fortunately abstime is not used). Related commit is 087cb745e509ac272abeea80aeb37278b0e96e5e . Related discussion is https://github.com/sunsetbrew/civetweb/pull/30 . --- src/civetweb.c | 57 +++++++++++++++++++++++++++++--------------------- 1 file changed, 33 insertions(+), 24 deletions(-) diff --git a/src/civetweb.c b/src/civetweb.c index 7b80635b..52869fef 100644 --- a/src/civetweb.c +++ b/src/civetweb.c @@ -327,8 +327,7 @@ typedef DWORD pthread_key_t; typedef HANDLE pthread_t; typedef struct { CRITICAL_SECTION threadIdSec; - int waitingthreadcount; /* The number of threads queued. */ - pthread_t *waitingthreadhdls; /* The thread handles. */ + struct mg_workerTLS *waiting_thread; /* The chain of threads */ } pthread_cond_t; #ifndef __clockid_t_defined @@ -1331,6 +1330,7 @@ struct mg_workerTLS { unsigned long thread_idx; #if defined(_WIN32) && !defined(__SYMBIAN32__) HANDLE pthread_cond_helper_mutex; + struct mg_workerTLS *next_waiting_thread; #endif }; @@ -2707,10 +2707,8 @@ pthread_cond_init(pthread_cond_t *cv, const void *unused) { (void)unused; InitializeCriticalSection(&cv->threadIdSec); - cv->waitingthreadcount = 0; - cv->waitingthreadhdls = - (pthread_t *)mg_calloc(MAX_WORKER_THREADS, sizeof(pthread_t)); - return (cv->waitingthreadhdls != NULL) ? 0 : -1; + cv->waiting_thread = NULL; + return 0; } @@ -2719,7 +2717,7 @@ pthread_cond_timedwait(pthread_cond_t *cv, pthread_mutex_t *mutex, const struct timespec *abstime) { - struct mg_workerTLS *tls = + struct mg_workerTLS **ptls, *tls = (struct mg_workerTLS *)pthread_getspecific(sTlsKey); int ok; struct timespec tsnow; @@ -2727,10 +2725,11 @@ pthread_cond_timedwait(pthread_cond_t *cv, DWORD mswaitrel; EnterCriticalSection(&cv->threadIdSec); - assert(cv->waitingthreadcount < MAX_WORKER_THREADS); - cv->waitingthreadhdls[cv->waitingthreadcount] = - tls->pthread_cond_helper_mutex; - cv->waitingthreadcount++; + /* Add this thread to cv's waiting list */ + ptls = &cv->waiting_thread; + for (; *ptls != NULL; ptls = &(*ptls)->next_waiting_thread); + tls->next_waiting_thread = NULL; + *ptls = tls; LeaveCriticalSection(&cv->threadIdSec); if (abstime) { @@ -2750,6 +2749,23 @@ pthread_cond_timedwait(pthread_cond_t *cv, pthread_mutex_unlock(mutex); ok = (WAIT_OBJECT_0 == WaitForSingleObject(tls->pthread_cond_helper_mutex, mswaitrel)); + if (!ok) { + ok = 1; + EnterCriticalSection(&cv->threadIdSec); + ptls = &cv->waiting_thread; + for (; *ptls != NULL; ptls = &(*ptls)->next_waiting_thread) { + if (*ptls == tls) { + *ptls = tls->next_waiting_thread; + ok = 0; + break; + } + } + LeaveCriticalSection(&cv->threadIdSec); + if (ok) { + WaitForSingleObject(tls->pthread_cond_helper_mutex, INFINITE); + } + } + /* This thread has been removed from cv's waiting list */ pthread_mutex_lock(mutex); return ok ? 0 : -1; @@ -2766,20 +2782,15 @@ pthread_cond_wait(pthread_cond_t *cv, pthread_mutex_t *mutex) static int pthread_cond_signal(pthread_cond_t *cv) { - int i; HANDLE wkup = NULL; BOOL ok = FALSE; EnterCriticalSection(&cv->threadIdSec); - if (cv->waitingthreadcount) { - wkup = cv->waitingthreadhdls[0]; + if (cv->waiting_thread) { + wkup = cv->waiting_thread->pthread_cond_helper_mutex; + cv->waiting_thread = cv->waiting_thread->next_waiting_thread; + ok = SetEvent(wkup); - - for (i = 1; i < cv->waitingthreadcount; i++) { - cv->waitingthreadhdls[i - 1] = cv->waitingthreadhdls[i]; - } - cv->waitingthreadcount--; - assert(ok); } LeaveCriticalSection(&cv->threadIdSec); @@ -2792,7 +2803,7 @@ static int pthread_cond_broadcast(pthread_cond_t *cv) { EnterCriticalSection(&cv->threadIdSec); - while (cv->waitingthreadcount) { + while (cv->waiting_thread) { pthread_cond_signal(cv); } LeaveCriticalSection(&cv->threadIdSec); @@ -2805,9 +2816,7 @@ static int pthread_cond_destroy(pthread_cond_t *cv) { EnterCriticalSection(&cv->threadIdSec); - assert(cv->waitingthreadcount == 0); - mg_free(cv->waitingthreadhdls); - cv->waitingthreadhdls = 0; + assert(cv->waiting_thread == NULL); LeaveCriticalSection(&cv->threadIdSec); DeleteCriticalSection(&cv->threadIdSec);