improve webserver shutdown and free some stuff to make the valgrind output cleaner

Sat, 24 Aug 2024 22:37:12 +0200

author
Olaf Wintermann <olaf.wintermann@gmail.com>
date
Sat, 24 Aug 2024 22:37:12 +0200
changeset 556
b036ccad4b49
parent 555
66b0accda0a8
child 557
e35829a3a6d8

improve webserver shutdown and free some stuff to make the valgrind output cleaner

src/server/config/acl.c file | annotate | diff | comparison | revisions
src/server/config/mimeconf.c file | annotate | diff | comparison | revisions
src/server/daemon/config.c file | annotate | diff | comparison | revisions
src/server/daemon/config.h file | annotate | diff | comparison | revisions
src/server/daemon/event.c file | annotate | diff | comparison | revisions
src/server/daemon/main.c file | annotate | diff | comparison | revisions
src/server/daemon/threadpools.c file | annotate | diff | comparison | revisions
src/server/daemon/threadpools.h file | annotate | diff | comparison | revisions
src/server/daemon/webserver.c file | annotate | diff | comparison | revisions
src/server/daemon/webserver.h file | annotate | diff | comparison | revisions
src/server/safs/init.c file | annotate | diff | comparison | revisions
src/server/util/pblock.c file | annotate | diff | comparison | revisions
src/server/util/pblock.h file | annotate | diff | comparison | revisions
src/server/util/thrpool.c file | annotate | diff | comparison | revisions
src/server/util/thrpool.h file | annotate | diff | comparison | revisions
--- a/src/server/config/acl.c	Sat Aug 24 18:34:13 2024 +0200
+++ b/src/server/config/acl.c	Sat Aug 24 22:37:12 2024 +0200
@@ -57,7 +57,10 @@
 }
 
 void free_acl_file(ACLFile *conf) {
-    //ucx_mempool_destroy(conf->parser.mp->pool);
+    cxListDestroy(conf->namedACLs);
+    cxListDestroy(conf->uriACLs);
+    cxListDestroy(conf->pathACLs);
+    cxMempoolDestroy(conf->parser.mp->data); // TODO: is there a better way to access the mempool?
     free(conf);
 }
 
--- a/src/server/config/mimeconf.c	Sat Aug 24 18:34:13 2024 +0200
+++ b/src/server/config/mimeconf.c	Sat Aug 24 22:37:12 2024 +0200
@@ -63,7 +63,7 @@
 }
 
 void free_mime_config(MimeConfig *conf) {
-    //ucx_mempool_destroy(conf->parser.mp->pool);
+    cxMempoolDestroy(conf->parser.mp->data); // TODO: is there a better way to access the mempool?
     free(conf);
 }
 
--- a/src/server/daemon/config.c	Sat Aug 24 18:34:13 2024 +0200
+++ b/src/server/daemon/config.c	Sat Aug 24 22:37:12 2024 +0200
@@ -59,7 +59,11 @@
 #include "../util/atomic.h"
 #include "cx/buffer.h"
 
-pool_handle_t *init_pool;
+static pool_handle_t *init_pool;
+
+pool_handle_t* cfg_get_init_pool(void) {
+    return init_pool;
+}
 
 char* cfg_config_file_path(const char *file) {
     cxstring base = CX_STR("config/");
@@ -694,13 +698,18 @@
     
     // TODO: check if all important configs are set
     
+    int ret = 0;
     HttpListener *listener = http_listener_create(&lc);
-    if(!listener) {
-        return 1;
+    if(listener) {
+        listener->default_vs.vs_name = cx_strdup_a(cfg->a, (cxstring){lc.vs.ptr, lc.vs.length}).ptr;
+        cxListAdd(cfg->listeners, listener);
+    } else {
+        ret = 1;
     }
     
-    listener->default_vs.vs_name = cx_strdup_a(cfg->a, (cxstring){lc.vs.ptr, lc.vs.length}).ptr;
-    cxListAdd(cfg->listeners, listener);
+    free(lc.name.ptr);
+    free(lc.vs.ptr);
+    free(lc.threadpool.ptr);
     
     return 0;
 }
--- a/src/server/daemon/config.h	Sat Aug 24 18:34:13 2024 +0200
+++ b/src/server/daemon/config.h	Sat Aug 24 22:37:12 2024 +0200
@@ -104,6 +104,8 @@
     CxMap   *map;
 };
 
+pool_handle_t* cfg_get_init_pool(void);
+
 char* cfg_config_file_path(const char *file);
 
 InitConfig* load_init_conf(const char *file);
--- a/src/server/daemon/event.c	Sat Aug 24 18:34:13 2024 +0200
+++ b/src/server/daemon/event.c	Sat Aug 24 22:37:12 2024 +0200
@@ -79,6 +79,8 @@
 }
 
 void shutdown_eventhandlers_wait(void) {
+    log_ereport(LOG_INFORM, "shutdown eventhandlers");
+    
     CxIterator i = cxMapIteratorValues(event_handler_map);
     cx_foreach(EVHandler *, e, i) {
         evhandler_shutdown(e);
@@ -90,6 +92,8 @@
     }
     
     cxMapDestroy(event_handler_map);
+    
+    log_ereport(LOG_INFORM, "all eventhandlers closed");
 }
 
 void evhandler_shutdown(EVHandler *h) {
--- a/src/server/daemon/main.c	Sat Aug 24 18:34:13 2024 +0200
+++ b/src/server/daemon/main.c	Sat Aug 24 22:37:12 2024 +0200
@@ -358,6 +358,8 @@
     if(srvctrl_wait()) {
         return EXIT_FAILURE;
     }
+    webserver_end();
+    log_ereport(LOG_INFORM, "webserver: exit");
 
     return EXIT_SUCCESS;
 }
--- a/src/server/daemon/threadpools.c	Sat Aug 24 18:34:13 2024 +0200
+++ b/src/server/daemon/threadpools.c	Sat Aug 24 22:37:12 2024 +0200
@@ -146,3 +146,16 @@
 threadpool_t* get_iopool(cxstring name) {
     return cxMapGet(io_pool_map, cx_hash_key_bytes((const unsigned char*)name.ptr, name.length));
 }
+
+
+void shutdown_threadpools(void) {
+    log_ereport(LOG_INFORM, "shutdown threadpools");
+    CxIterator i = cxMapIteratorValues(thread_pool_map);
+    cx_foreach(threadpool_t*, tp, i) {
+        threadpool_shutdown(tp);
+    }
+    i = cxMapIteratorValues(io_pool_map);
+    cx_foreach(threadpool_t*, tp, i) {
+        threadpool_shutdown(tp);
+    }
+}
--- a/src/server/daemon/threadpools.h	Sat Aug 24 18:34:13 2024 +0200
+++ b/src/server/daemon/threadpools.h	Sat Aug 24 22:37:12 2024 +0200
@@ -54,6 +54,8 @@
 threadpool_t* get_default_iopool();
 threadpool_t* get_iopool(cxstring name);
 
+void shutdown_threadpools(void);
+
 #ifdef	__cplusplus
 }
 #endif
--- a/src/server/daemon/webserver.c	Sat Aug 24 18:34:13 2024 +0200
+++ b/src/server/daemon/webserver.c	Sat Aug 24 22:37:12 2024 +0200
@@ -248,6 +248,7 @@
             return -1;
         }
     }
+    free(tmp_priv.ptr);
     
     
     // create srvctrl unix domain socket
@@ -279,13 +280,21 @@
     log_ereport(LOG_INFORM, "webserver shutdown");
     
     srvctrl_shutdown();
-    
+}
+
+void webserver_end() {
     // execute restart callbacks
     RestartCallback *re = atrestart;
     while(re) {
         re->func(re->data);
         re = re->next;
     }
+    
+    shutdown_threadpools();
+    
+    shutdown_eventhandlers_wait();
+    
+    webserver_destroy();
 }
 
 int webserver_reconfig() {
@@ -326,6 +335,14 @@
     }
 }
 
+void webserver_destroy() {
+    // free some stuff
+    // this is not necessary, because the whole process will exit
+    // however it will result in a nicer valgrind output with less
+    // memory leaks
+    pool_destroy(cfg_get_init_pool());
+}
+
 int nsapi_runtime_version() {
     return 303;
 }
--- a/src/server/daemon/webserver.h	Sat Aug 24 18:34:13 2024 +0200
+++ b/src/server/daemon/webserver.h	Sat Aug 24 22:37:12 2024 +0200
@@ -45,10 +45,13 @@
 int webserver_init();
 int webserver_run();
 void webserver_shutdown();
+void webserver_end();
 int webserver_reconfig();
 
 void webserver_atrestart(void (*fn)(void *), void *data);
 
+void webserver_destroy();
+
 int ws_init_ssl();
 
 #ifdef	__cplusplus
--- a/src/server/safs/init.c	Sat Aug 24 18:34:13 2024 +0200
+++ b/src/server/safs/init.c	Sat Aug 24 22:37:12 2024 +0200
@@ -31,6 +31,7 @@
 
 #include "../daemon/func.h"
 #include "../daemon/log.h"
+#include "../daemon/config.h"
 
 #include "init.h"
 
@@ -72,6 +73,7 @@
     }
 
     // load function symbols
+    pool_handle_t *init_pool = cfg_get_init_pool();
     int b = 0;
     for(int i=0;;i++) {
         if(funcs[i] == '-') {
@@ -82,6 +84,9 @@
                 b = 1;
             }
 
+            // TODO: although this works fine, is not really clean to just
+            //       destroy the string with random 0-bytes
+            //       maybe use cxstr here
             funcs[i] = 0;
 
             // we have a function name         
@@ -94,7 +99,7 @@
             struct FuncStruct fc;
             ZERO(&fc, sizeof(struct FuncStruct));
             fc.func = (FuncPtr)sym;
-            fc.name = cx_strdup(cx_str(funcs)).ptr;
+            fc.name = pool_strdup(init_pool, funcs);
             add_function(&fc);
 
             if(b) {
--- a/src/server/util/pblock.c	Sat Aug 24 18:34:13 2024 +0200
+++ b/src/server/util/pblock.c	Sat Aug 24 22:37:12 2024 +0200
@@ -527,7 +527,11 @@
         if (count > 0) {
             pb_key **keys = _pbKeys.buckets[i].elements;
             for (unsigned j = 0 ; j < count ; j++) {
-                free(keys[j]);
+                pb_key *key = keys[j];
+                if(key) {
+                    free((char*)key->name);
+                    free(key);
+                }
             }
             free(keys);
         }
--- a/src/server/util/pblock.h	Sat Aug 24 18:34:13 2024 +0200
+++ b/src/server/util/pblock.h	Sat Aug 24 22:37:12 2024 +0200
@@ -74,7 +74,7 @@
 #ifdef	__cplusplus
 extern "C" {
 #endif
-
+   
 NSAPI_PUBLIC pb_param *INTparam_create(const char *name, const char *value);
 
 NSAPI_PUBLIC int INTparam_free(pb_param *pp);
--- a/src/server/util/thrpool.c	Sat Aug 24 18:34:13 2024 +0200
+++ b/src/server/util/thrpool.c	Sat Aug 24 22:37:12 2024 +0200
@@ -78,7 +78,10 @@
 
         free(job);
     }
-    ws_atomic_dec32(&pool->num_threads);
+    uint32_t nthreads = ws_atomic_dec32(&pool->num_threads);
+    if(nthreads == 0) {
+        log_ereport(LOG_INFORM, "threadpool closed"); // TODO: log threadpool name
+    }
     return NULL;
 }
 
@@ -162,3 +165,13 @@
     }
     pool->queue_len++;
 }
+
+void threadpool_shutdown(threadpool_t *pool) {
+    int nthreads = pool->max_threads;
+    for(int i=0;i<nthreads;i++) {
+        pthread_mutex_lock(&pool->queue_lock);
+        threadpool_enqueue_job(pool, &kill_job);
+        pthread_cond_signal(&pool->available);
+        pthread_mutex_unlock(&pool->queue_lock);
+    }
+}
--- a/src/server/util/thrpool.h	Sat Aug 24 18:34:13 2024 +0200
+++ b/src/server/util/thrpool.h	Sat Aug 24 22:37:12 2024 +0200
@@ -61,6 +61,8 @@
 
 void threadpool_enqueue_job(threadpool_t *pool, threadpool_job *job);
 
+void threadpool_shutdown(threadpool_t *pool);
+
 #ifdef	__cplusplus
 }
 #endif

mercurial