metrics: avoid double-free crash on shutdown by not calling pthread_exit

fixes #1207; thanks @studersi
upon exit, do write cached metrics into shared memory before exiting

Signed-off-by: Hans Zandbelt <[email protected]>

Author: zandbelt

ChangeLog

@@ -1,7 +1,9 @@
 03/18/2025
-- use case insensitive hostname/domain comparison in oidc_check_cookie_domain
-- remove the Location header from HTML based step up authentication redirects
+- core: use case insensitive hostname/domain comparison in oidc_check_cookie_domain
+- authz: remove the Location header from HTML based step up authentication redirects
   as it may conflict with its HTTP 200 status code and confuse middle boxes
+- metrics: avoid double-free on shutdown by not calling pthread_exit; fixes #1207; thanks @studersi
+- metrics: upon exit, do write cached metrics into shared memory before exiting
 - bump to 2.4.16.9dev
 
 02/17/2025

src/metrics.c

@@ -725,6 +725,8 @@ static unsigned int oidc_metric_random_int(unsigned int mod) {
 	return v % mod;
 }
 
+#define OIDC_METRICS_POLL_INTERVAL 250
+
 /*
  * thread that periodically writes the local data into the shared memory
  */
@@ -734,14 +736,21 @@ static void *APR_THREAD_FUNC oidc_metrics_thread_run(apr_thread_t *thread, void
 	/* sleep for a short random time <1s so child processes write-lock on a different frequency */
 	apr_sleep(apr_time_from_msec(oidc_metric_random_int(1000)));
 
+	/* calculate the number of short sleep intervals */
+	int n = _oidc_metrics_interval(s) / apr_time_from_msec(OIDC_METRICS_POLL_INTERVAL);
+
 	/* see if we are asked to exit */
 	while (_oidc_metrics_thread_exit == FALSE) {
 
-		apr_sleep(_oidc_metrics_interval(s));
+		/* break up the sleep interval in short intervals so we can exit timely fashion without confusing Apache
+		 * at shutdown */
+		for (int i = 0; i < n; i++) {
+			apr_sleep(apr_time_from_msec(OIDC_METRICS_POLL_INTERVAL));
+			if (_oidc_metrics_thread_exit == TRUE)
+				break;
+		}
 
-		// NB: exit here because the parent thread may have cleaned up the shared memory segment
-		if (_oidc_metrics_thread_exit == TRUE)
-			break;
+		// NB: no exit here because we need to write our local metrics into the cache before exiting
 
 		/* lock the mutex that protects the locally cached metrics */
 		oidc_cache_mutex_lock(s->process->pool, s, _oidc_metrics_process_mutex);
@@ -757,7 +766,8 @@ static void *APR_THREAD_FUNC oidc_metrics_thread_run(apr_thread_t *thread, void
 		oidc_cache_mutex_unlock(s->process->pool, s, _oidc_metrics_process_mutex);
 	}
 
-	apr_thread_exit(thread, APR_SUCCESS);
+	/* NB: don't call apr_thread_exit here because it seems that Apache is cleaning up its own threads */
+	// apr_thread_exit(thread, APR_SUCCESS);
 
 	return NULL;
 }