From a972cd9239e19a1cf985dfb6413f88883e845869 Mon Sep 17 00:00:00 2001 From: Roman Janota Date: Mon, 27 Apr 2026 15:30:15 +0200 Subject: [PATCH 1/4] test_ch BUGFIX atomic thread sync flag Fixes #602 --- tests/test_ch.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/test_ch.c b/tests/test_ch.c index b7996f59..6be19740 100644 --- a/tests/test_ch.c +++ b/tests/test_ch.c @@ -25,6 +25,7 @@ #include +#include "compat.h" #include "ln2_test.h" struct test_ch_data { @@ -392,7 +393,7 @@ setup_tls(void **state) * Verifies that deleting a CH client from config properly signals the thread to stop, * even when the session is in RUNNING state. */ -static int session_established_flag = 0; +static ATOMIC_T session_established_flag = 0; static int ch_new_session_set_flag_cb(const char *client_name, struct nc_session *new_session, void *user_data) @@ -405,7 +406,7 @@ ch_new_session_set_flag_cb(const char *client_name, struct nc_session *new_sessi ret = nc_ps_add_session(ps, new_session); assert_int_equal(ret, 0); - session_established_flag = 1; + ATOMIC_STORE_RELAXED(session_established_flag, 1); return 0; } @@ -430,7 +431,7 @@ server_thread_delete_while_session(void *arg) ch_session_release_ctx_cb, test_ctx, ch_new_session_set_flag_cb, ps); assert_int_equal(ret, 0); - while (!session_established_flag) { + while (!ATOMIC_LOAD_RELAXED(session_established_flag)) { usleep(10000); } @@ -512,7 +513,7 @@ setup_delete_while_session(void **state) test_ctx->free_test_data = test_nc_ch_free_test_data; *state = test_ctx; - session_established_flag = 0; + ATOMIC_STORE_RELAXED(session_established_flag, 0); ret = nc_server_config_add_ch_address_port(test_ctx->ctx, "ch_delete_test", "endpt", NC_TI_SSH, "127.0.0.1", TEST_PORT_3_STR, &tree); From fcaa1c750ac804fd20f48a185c3f0dbdde12fc25 Mon Sep 17 00:00:00 2001 From: Roman Janota Date: Mon, 27 Apr 2026 15:31:54 +0200 Subject: [PATCH 2/4] session server UPDATE move CH thread data free Free the CH thread data in the main thread instead of the thread itself, because the data is allocated by the main thread, so it should also be destroyed by it. --- src/session_server.c | 43 +++++++++++++++++++++++++++---------------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/src/session_server.c b/src/session_server.c index 31f83a0b..a353b7e8 100644 --- a/src/session_server.c +++ b/src/session_server.c @@ -3725,19 +3725,6 @@ nc_ch_client_thread(void *arg) VRB(session, "Call Home client \"%s\" thread exit.", data->client_name); free(cur_endpt_name); - /* find the client and clear thread pointer */ - if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_WRITE, NC_CONFIG_LOCK_TIMEOUT, __func__) == 1) { - client = nc_server_ch_client_get(data->client_name); - if (client && (client->thread == data)) { - client->thread = NULL; - } - nc_rwlock_unlock(&server_opts.config_lock, __func__); - } - - free(data->client_name); - pthread_mutex_destroy(&data->cond_lock); - pthread_cond_destroy(&data->cond); - free(data); return NULL; } @@ -3748,12 +3735,15 @@ nc_session_server_ch_client_dispatch_stop(struct nc_ch_client *ch_client) struct nc_server_ch_thread_arg *thread_arg; pthread_t tid; enum nc_rwlock_mode config_lock_mode = NC_RWLOCK_WRITE; + char *ch_client_name = NULL; if (!ch_client || !ch_client->thread) { return 0; } thread_arg = ch_client->thread; + ch_client_name = strdup(thread_arg->client_name); + NC_CHECK_ERRMEM_GOTO(!ch_client_name, rc = 1, cleanup); /* CH COND LOCK */ if (nc_mutex_lock(&thread_arg->cond_lock, NC_CH_COND_LOCK_TIMEOUT, __func__) != 1) { @@ -3779,19 +3769,40 @@ nc_session_server_ch_client_dispatch_stop(struct nc_ch_client *ch_client) /* wait for the thread to end */ r = pthread_join(tid, NULL); if (r) { - ERR(NULL, "Joining Call Home client \"%s\" thread failed (%s).", ch_client->name, strerror(r)); + ERR(NULL, "Joining Call Home client \"%s\" thread failed (%s).", ch_client_name, strerror(r)); + rc = 1; + goto cleanup; + } + + /* CONFIG WRITE LOCK - re-acquire to clear the thread pointer and free the thread data */ + if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_WRITE, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + /* if we fail, attempt to lock again in cleanup. + * ch thread data will cause a memory leak, but we should avoid a possible crash this way */ + ERRINT; rc = 1; goto cleanup; } + config_lock_mode = NC_RWLOCK_WRITE; + + /* clear the thread pointer, + * ch_client MUST remain valid even though we unlocked config lock, + * because the caller MUST hold config apply mutex, so no one can change the config and free the client */ + ch_client->thread = NULL; + + /* free the thread data */ + free(thread_arg->client_name); + pthread_mutex_destroy(&thread_arg->cond_lock); + pthread_cond_destroy(&thread_arg->cond); + free(thread_arg); cleanup: if (config_lock_mode == NC_RWLOCK_NONE) { /* CONFIG LOCK - lock it back if we unlocked it. It MUST succeed, if the caller holds the config apply mutex */ - if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_WRITE, -1, __func__) != 1) { + if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_WRITE, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { ERRINT; } } - + free(ch_client_name); return rc; } From 1edb7f8b7a9c2cfe723daa266330ef7fba524a99 Mon Sep 17 00:00:00 2001 From: Roman Janota Date: Mon, 27 Apr 2026 15:34:09 +0200 Subject: [PATCH 3/4] SOVERSION bump to version 5.4.1 --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 25994290..bd90d0e4 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -66,7 +66,7 @@ set(LIBNETCONF2_VERSION ${LIBNETCONF2_MAJOR_VERSION}.${LIBNETCONF2_MINOR_VERSION # with backward compatible change and micro version is connected with any internal change of the library. set(LIBNETCONF2_MAJOR_SOVERSION 5) set(LIBNETCONF2_MINOR_SOVERSION 4) -set(LIBNETCONF2_MICRO_SOVERSION 0) +set(LIBNETCONF2_MICRO_SOVERSION 1) set(LIBNETCONF2_SOVERSION_FULL ${LIBNETCONF2_MAJOR_SOVERSION}.${LIBNETCONF2_MINOR_SOVERSION}.${LIBNETCONF2_MICRO_SOVERSION}) set(LIBNETCONF2_SOVERSION ${LIBNETCONF2_MAJOR_SOVERSION}) From 9294b6181e5d6c257f5cc0f0fdc8c214b63e1053 Mon Sep 17 00:00:00 2001 From: Roman Janota Date: Mon, 27 Apr 2026 15:34:27 +0200 Subject: [PATCH 4/4] VERSION bump to version 4.4.1 --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index bd90d0e4..943ff12b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -58,7 +58,7 @@ set(CMAKE_MACOSX_RPATH TRUE) # micro version is changed with a set of small changes or bugfixes anywhere in the project. set(LIBNETCONF2_MAJOR_VERSION 4) set(LIBNETCONF2_MINOR_VERSION 4) -set(LIBNETCONF2_MICRO_VERSION 0) +set(LIBNETCONF2_MICRO_VERSION 1) set(LIBNETCONF2_VERSION ${LIBNETCONF2_MAJOR_VERSION}.${LIBNETCONF2_MINOR_VERSION}.${LIBNETCONF2_MICRO_VERSION}) # Version of the library