Skip to content

Refactor CH thread management #594

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Roytak wants to merge 4 commits into
base: devel
Choose a base branch
from
Open

Refactor CH thread management #594

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7ef32c0
session server UPDATE CH thread lifecycle management
Roytak Apr 13, 2026
dc8687b
session server BUGFIX compile without ssh_tls
Roytak Apr 13, 2026
3a647b8
SOVERSION bump to version 5.3.7
Roytak Apr 13, 2026
b28885f
VERSION bump to version 4.3.1
Roytak Apr 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,15 @@ 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 3)
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
# Major version is changed with every backward non-compatible API/ABI change in the library, minor version changes
# 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 3)
set(LIBNETCONF2_MICRO_SOVERSION 6)
set(LIBNETCONF2_MICRO_SOVERSION 7)
set(LIBNETCONF2_SOVERSION_FULL ${LIBNETCONF2_MAJOR_SOVERSION}.${LIBNETCONF2_MINOR_SOVERSION}.${LIBNETCONF2_MICRO_SOVERSION})
set(LIBNETCONF2_SOVERSION ${LIBNETCONF2_MAJOR_SOVERSION})

Expand Down
172 changes: 76 additions & 96 deletions src/server_config.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3941,7 +3941,7 @@ config_netconf_client(const struct lyd_node *node, enum nc_operation parent_op,
struct lyd_node *n;
enum nc_operation op;
const char *name;
LY_ARRAY_COUNT_TYPE i = 0, j = 0;
LY_ARRAY_COUNT_TYPE i = 0;
struct nc_ch_client *ch_client = NULL;

NC_NODE_GET_OP(node, parent_op, &op);
Expand Down Expand Up @@ -3988,30 +3988,6 @@ config_netconf_client(const struct lyd_node *node, enum nc_operation parent_op,

/* all children processed, we can now delete the client */
if (op == NC_OP_DELETE) {
/* CH THREADS DATA RD LOCK */
if (nc_rwlock_lock(&server_opts.ch_threads_lock, NC_RWLOCK_READ, NC_CH_THREADS_LOCK_TIMEOUT, __func__) != 1) {
return 1;
}

/* find the thread data for this CH client, not found <==> CH thread not running */
LY_ARRAY_FOR(server_opts.ch_threads, j) {
if (!strcmp(server_opts.ch_threads[j]->client_name, name)) {
break;
}
}

if (j < LY_ARRAY_COUNT(server_opts.ch_threads)) {
/* the CH thread is running, notify it to stop */
if (nc_mutex_lock(&server_opts.ch_threads[j]->cond_lock, NC_CH_COND_LOCK_TIMEOUT, __func__) != 1) {
server_opts.ch_threads[j]->thread_running = 0;
pthread_cond_signal(&server_opts.ch_threads[j]->cond);
nc_mutex_unlock(&server_opts.ch_threads[j]->cond_lock, __func__);
}
}

/* CH THREADS DATA UNLOCK */
nc_rwlock_unlock(&server_opts.ch_threads_lock, __func__);

/* we can use 'i' from above */
if (i < LY_ARRAY_COUNT(config->ch_clients) - 1) {
config->ch_clients[i] = config->ch_clients[LY_ARRAY_COUNT(config->ch_clients) - 1];
Expand Down Expand Up @@ -5330,6 +5306,37 @@ nc_server_config_reconcile_sockets_listen(struct nc_server_config *old_cfg,

#ifdef NC_ENABLED_SSH_TLS

/**
* @brief Check if there are any new Call Home clients created in the new configuration.
*
* @param[in] old_cfg Old, currently active server configuration.
* @param[in] new_cfg New server configuration currently being applied.
* @return 1 if there are new CH clients, 0 otherwise.
*/
static int
nc_server_config_new_ch_clients_created(struct nc_server_config *old_cfg, struct nc_server_config *new_cfg)
{
struct nc_ch_client *old_ch_client, *new_ch_client;
int found;

/* check if there are any new clients */
LY_ARRAY_FOR(new_cfg->ch_clients, struct nc_ch_client, new_ch_client) {
found = 0;
LY_ARRAY_FOR(old_cfg->ch_clients, struct nc_ch_client, old_ch_client) {
if (!strcmp(new_ch_client->name, old_ch_client->name)) {
found = 1;
break;
}
}
if (!found) {
return 1;
}
}

/* no differences found */
return 0;
}

/**
* @brief Atomically dispatch new Call Home clients and reuse existing ones.
*
Expand All @@ -5347,55 +5354,63 @@ nc_server_config_reconcile_chclients_dispatch(struct nc_server_config *old_cfg,
int found;
LY_ARRAY_COUNT_TYPE i = 0;
char **started_clients = NULL, **client_name = NULL;
int dispatch_new_clients = 1;

if (!server_opts.ch_dispatch_data.acquire_ctx_cb || !server_opts.ch_dispatch_data.release_ctx_cb ||
!server_opts.ch_dispatch_data.new_session_cb) {
/* Call Home dispatch callbacks not set, nothing to do */
return 0;
/* Call Home dispatch callbacks not set, we can't dispatch new clients, but we can still stop deleted ones */
if (nc_server_config_new_ch_clients_created(old_cfg, new_cfg)) {
WRN(NULL, "New Call Home clients were created but Call Home dispatch callbacks are not set - "
"new clients will not be dispatched automatically.");
}
dispatch_new_clients = 0;
}

/*
* == PHASE 1: START NEW CLIENTS ==
* Start clients present in new_cfg that are not already running.
* Track successfully started clients for potential rollback.
*/
LY_ARRAY_FOR(new_cfg->ch_clients, struct nc_ch_client, new_ch_client) {
found = 0;
if (dispatch_new_clients) {
/* only dispatch if all required CBs are set */
LY_ARRAY_FOR(new_cfg->ch_clients, struct nc_ch_client, new_ch_client) {
found = 0;

/* CH THREADS LOCK (reading server_opts.ch_threads) */
if (nc_rwlock_lock(&server_opts.ch_threads_lock, NC_RWLOCK_READ, NC_CH_THREADS_LOCK_TIMEOUT, __func__) != 1) {
rc = 1;
goto rollback;
}
/* CH THREADS LOCK (reading server_opts.ch_threads) */
if (nc_rwlock_lock(&server_opts.ch_threads_lock, NC_RWLOCK_READ, NC_CH_THREADS_LOCK_TIMEOUT, __func__) != 1) {
rc = 1;
goto rollback;
}

LY_ARRAY_FOR(server_opts.ch_threads, struct nc_server_ch_thread_arg *, ch_thread_arg) {
if (!strcmp(new_ch_client->name, (*ch_thread_arg)->client_name)) {
/* already running, do not start again */
found = 1;
break;
LY_ARRAY_FOR(server_opts.ch_threads, struct nc_server_ch_thread_arg *, ch_thread_arg) {
if (!strcmp(new_ch_client->name, (*ch_thread_arg)->client_name)) {
/* already running, do not start again */
found = 1;
break;
}
}
}

/* CH THREADS UNLOCK */
nc_rwlock_unlock(&server_opts.ch_threads_lock, __func__);
/* CH THREADS UNLOCK */
nc_rwlock_unlock(&server_opts.ch_threads_lock, __func__);

if (!found) {
/* this is a new Call Home client, dispatch it */
rc = _nc_connect_ch_client_dispatch(new_ch_client, server_opts.ch_dispatch_data.acquire_ctx_cb,
server_opts.ch_dispatch_data.release_ctx_cb, server_opts.ch_dispatch_data.ctx_cb_data,
server_opts.ch_dispatch_data.new_session_cb, server_opts.ch_dispatch_data.new_session_cb_data);
if (rc) {
/* FAILURE! trigger rollback */
goto rollback;
}
if (!found) {
/* this is a new Call Home client, dispatch it */
rc = _nc_connect_ch_client_dispatch(new_ch_client, server_opts.ch_dispatch_data.acquire_ctx_cb,
server_opts.ch_dispatch_data.release_ctx_cb, server_opts.ch_dispatch_data.ctx_cb_data,
server_opts.ch_dispatch_data.new_session_cb, server_opts.ch_dispatch_data.new_session_cb_data);
if (rc) {
/* FAILURE! trigger rollback */
goto rollback;
}

/* successfully started, track it for potential rollback */
LY_ARRAY_NEW_GOTO(NULL, started_clients, client_name, rc, rollback);
*client_name = strdup(new_ch_client->name);
NC_CHECK_ERRMEM_GOTO(!*client_name, rc = 1, rollback);
/* successfully started, track it for potential rollback */
LY_ARRAY_NEW_GOTO(NULL, started_clients, client_name, rc, rollback);
*client_name = strdup(new_ch_client->name);
NC_CHECK_ERRMEM_GOTO(!*client_name, rc = 1, rollback);

/* ownership transferred to array */
client_name = NULL;
/* ownership transferred to array */
client_name = NULL;
}
}
}

Expand All @@ -5415,27 +5430,10 @@ nc_server_config_reconcile_chclients_dispatch(struct nc_server_config *old_cfg,

if (!found) {
/* this Call Home client was deleted, notify it to stop */
ch_thread_arg = NULL;

/* CH THREADS LOCK (reading server_opts.ch_threads) */
if (nc_rwlock_lock(&server_opts.ch_threads_lock, NC_RWLOCK_READ, NC_CH_THREADS_LOCK_TIMEOUT, __func__) != 1) {
/* Continue even if lock fails - best effort cleanup */
continue;
}
LY_ARRAY_FOR(server_opts.ch_threads, struct nc_server_ch_thread_arg *, ch_thread_arg) {
if (!strcmp(old_ch_client->name, (*ch_thread_arg)->client_name)) {
/* notify the thread to stop */
if (nc_mutex_lock(&(*ch_thread_arg)->cond_lock, NC_CH_COND_LOCK_TIMEOUT, __func__) != 1) {
(*ch_thread_arg)->thread_running = 0;
pthread_cond_signal(&(*ch_thread_arg)->cond);
nc_mutex_unlock(&(*ch_thread_arg)->cond_lock, __func__);
}
break;
}
if ((rc = nc_session_server_ch_client_dispatch_stop_if_dispatched(old_ch_client->name, NC_RWLOCK_WRITE))) {
ERR(NULL, "Failed to dispatch stop for Call Home client \"%s\".", old_ch_client->name);
goto rollback;
}
/* CH THREADS UNLOCK */
nc_rwlock_unlock(&server_opts.ch_threads_lock, __func__);
/* Note: if ch_thread_arg is NULL here, the thread wasn't running. That's fine. */
}
}

Expand All @@ -5450,25 +5448,7 @@ nc_server_config_reconcile_chclients_dispatch(struct nc_server_config *old_cfg,
* to return to the pre-call state.
*/
LY_ARRAY_FOR(started_clients, i) {
ch_thread_arg = NULL;

/* CH THREADS LOCK (reading server_opts.ch_threads) */
if (nc_rwlock_lock(&server_opts.ch_threads_lock, NC_RWLOCK_READ, NC_CH_THREADS_LOCK_TIMEOUT, __func__) != 1) {
/* Continue even if lock fails - best effort rollback */
continue;
}
LY_ARRAY_FOR(server_opts.ch_threads, struct nc_server_ch_thread_arg *, ch_thread_arg) {
if (!strcmp(started_clients[i], (*ch_thread_arg)->client_name)) {
/* notify the newly started thread to stop */
nc_mutex_lock(&(*ch_thread_arg)->cond_lock, NC_CH_COND_LOCK_TIMEOUT, __func__);
(*ch_thread_arg)->thread_running = 0;
pthread_cond_signal(&(*ch_thread_arg)->cond);
nc_mutex_unlock(&(*ch_thread_arg)->cond_lock, __func__);
break;
}
}
/* CH THREADS UNLOCK */
nc_rwlock_unlock(&server_opts.ch_threads_lock, __func__);
nc_session_server_ch_client_dispatch_stop_if_dispatched(started_clients[i], NC_RWLOCK_WRITE);
}
/* rc is already set to non-zero from the failure point */

Expand Down
28 changes: 20 additions & 8 deletions src/session.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -910,7 +910,7 @@ nc_session_free_transport(struct nc_session *session, int *multisession)
* NC_CLIENT: we are waiting for the server to acknowledge our SSH channel EOF
* by sending us its own SSH channel EOF. */
if (ssh_channel_poll_timeout(session->ti.libssh.channel, NC_SESSION_FREE_SSH_POLL_EOF_TIMEOUT, 0) != SSH_EOF) {
WRN(session, "Timeout for receiving SSH channel EOF from the peer elapsed.");
WRN(session, "Timeout for receiving SSH channel EOF from the %s elapsed.", session->side == NC_CLIENT ? "server" : "client");
}
}
ssh_channel_free(session->ti.libssh.channel);
Expand Down Expand Up @@ -1001,7 +1001,7 @@ nc_session_free_transport(struct nc_session *session, int *multisession)
API void
nc_session_free(struct nc_session *session, void (*data_free)(void *))
{
int r, i, rpc_locked = 0;
int r, i, rpc_locked = 0, ch_locked = 0;
int multisession = 0; /* flag for more NETCONF sessions on a single SSH session */
struct timespec ts;
NC_STATUS status;
Expand All @@ -1012,15 +1012,17 @@ nc_session_free(struct nc_session *session, void (*data_free)(void *))

if ((session->side == NC_SERVER) && (session->flags & NC_SESSION_CALLHOME)) {
/* CH LOCK, continue on error */
r = nc_mutex_lock(&session->opts.server.ch_lock, NC_SESSION_CH_LOCK_TIMEOUT, __func__);
if (nc_mutex_lock(&session->opts.server.ch_lock, NC_SESSION_CH_LOCK_TIMEOUT, __func__) == 1) {
ch_locked = 1;
}
}

/* store status, so we can check if this session is already closing */
status = session->status;

if ((session->side == NC_SERVER) && (session->flags & NC_SESSION_CALLHOME)) {
/* CH UNLOCK */
if (r == 1) {
if (ch_locked) {
/* only if we locked it */
nc_mutex_unlock(&session->opts.server.ch_lock, __func__);
}
Expand Down Expand Up @@ -1060,8 +1062,12 @@ nc_session_free(struct nc_session *session, void (*data_free)(void *))
nc_client_msgs_free(session);
}

if (session->status == NC_STATUS_RUNNING) {
/* notify the peer that we're closing the session */
/* notify the peer that we're closing the session, either if:
* - session running - normal disconnect from client
* - session invalid - client disconnected from a Call Home session */
if ((session->status == NC_STATUS_RUNNING) ||
((session->side == NC_SERVER) && (session->flags & NC_SESSION_CALLHOME) &&
(session->status == NC_STATUS_INVALID) && (session->term_reason == NC_SESSION_TERM_CLOSED))) {
if (session->side == NC_CLIENT) {
/* graceful close: <close-session> + transport shutdown indication */
nc_session_free_client_close_graceful(session);
Expand All @@ -1073,7 +1079,10 @@ nc_session_free(struct nc_session *session, void (*data_free)(void *))

if ((session->side == NC_SERVER) && (session->flags & NC_SESSION_CALLHOME)) {
/* CH LOCK */
nc_mutex_lock(&session->opts.server.ch_lock, NC_SESSION_CH_LOCK_TIMEOUT, __func__);
ch_locked = 0;
if (nc_mutex_lock(&session->opts.server.ch_lock, NC_SESSION_CH_LOCK_TIMEOUT, __func__) == 1) {
ch_locked = 1;
}
}

/* mark session for closing */
Expand All @@ -1096,7 +1105,10 @@ nc_session_free(struct nc_session *session, void (*data_free)(void *))

if ((session->side == NC_SERVER) && (session->flags & NC_SESSION_CALLHOME)) {
/* CH UNLOCK */
nc_mutex_unlock(&session->opts.server.ch_lock, __func__);
if (ch_locked) {
/* only if we locked it */
nc_mutex_unlock(&session->opts.server.ch_lock, __func__);
}
}

/* transport implementation cleanup */
Expand Down
12 changes: 12 additions & 0 deletions src/session_p.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -692,6 +692,7 @@ struct nc_server_ch_thread_arg {
uint8_t cur_attempt, void *user_data); /**< failed to create a new session cb */
void *new_session_fail_cb_data; /**< new session fail cb data */

pthread_t tid; /**< Thread ID of the Call Home client thread. */
int thread_running; /**< A boolean value that is truthy while the underlying Call Home thread is running */
pthread_mutex_t cond_lock; /**< Condition's lock used for signalling the thread to terminate */
pthread_cond_t cond; /**< Condition used for signalling the thread to terminate */
Expand Down Expand Up @@ -1387,6 +1388,17 @@ NC_MSG_TYPE nc_connect_callhome(const char *host, uint16_t port, NC_TRANSPORT_IM

#ifdef NC_ENABLED_SSH_TLS

/**
* @brief Stop a dispatched Call Home client thread, if such thread was dispatched for the given client name.
*
* @param[in] client_name Name of the Call Home client to stop the thread for.
* @param[in] config_lock_mode Lock mode of the configuration lock, if it is held by the caller.
* If NC_RWLOCK_WRITE is passed, config lock will be briefly unlocked and then locked again.
* The caller MUST ensure that it holds the CONFIG APPLY mutex, so that nobody steals the wrlock from him.
* @return 0 if the thread was successfully stopped or not found, 1 on error.
*/
int nc_session_server_ch_client_dispatch_stop_if_dispatched(const char *client_name, enum nc_rwlock_mode config_lock_mode);

/**
* @brief Dispatch a thread connecting to a listening NETCONF client and creating Call Home sessions.
*
Expand Down
Loading
Loading