Skip to content

Commit 731bd65

Browse files
committed
session server REFACTOR move ch thread data to ch_client
Move Call Home thread data from global server_opts.ch_threads array to individual ch_client->thread pointer. This simplifies thread management by eliminating the separate ch_threads_lock and array manipulation. Each ch_client now directly owns its thread data, making the lifecycle management more straightforward.
1 parent c5ac09e commit 731bd65

3 files changed

Lines changed: 95 additions & 172 deletions

File tree

src/server_config.c

Lines changed: 22 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -5350,10 +5350,9 @@ nc_server_config_reconcile_chclients_dispatch(struct nc_server_config *old_cfg,
53505350
{
53515351
int rc = 0;
53525352
struct nc_ch_client *old_ch_client, *new_ch_client;
5353-
struct nc_server_ch_thread_arg **ch_thread_arg;
53545353
int found;
5355-
LY_ARRAY_COUNT_TYPE i = 0;
5356-
char **started_clients = NULL, **client_name = NULL;
5354+
LY_ARRAY_COUNT_TYPE i;
5355+
struct nc_ch_client **started_clients = NULL, **started_client_ptr;
53575356
int dispatch_new_clients = 1;
53585357

53595358
if (!server_opts.ch_dispatch_data.acquire_ctx_cb || !server_opts.ch_dispatch_data.release_ctx_cb ||
@@ -5369,48 +5368,28 @@ nc_server_config_reconcile_chclients_dispatch(struct nc_server_config *old_cfg,
53695368
/*
53705369
* == PHASE 1: START NEW CLIENTS ==
53715370
* Start clients present in new_cfg that are not already running.
5372-
* Track successfully started clients for potential rollback.
5371+
* Track successfully started threads for potential rollback.
53735372
*/
53745373
if (dispatch_new_clients) {
53755374
/* only dispatch if all required CBs are set */
53765375
LY_ARRAY_FOR(new_cfg->ch_clients, struct nc_ch_client, new_ch_client) {
5377-
found = 0;
5378-
5379-
/* CH THREADS LOCK (reading server_opts.ch_threads) */
5380-
if (nc_rwlock_lock(&server_opts.ch_threads_lock, NC_RWLOCK_READ, NC_CH_THREADS_LOCK_TIMEOUT, __func__) != 1) {
5381-
rc = 1;
5382-
goto rollback;
5376+
if (new_ch_client->thread) {
5377+
/* already running */
5378+
continue;
53835379
}
53845380

5385-
LY_ARRAY_FOR(server_opts.ch_threads, struct nc_server_ch_thread_arg *, ch_thread_arg) {
5386-
if (!strcmp(new_ch_client->name, (*ch_thread_arg)->client_name)) {
5387-
/* already running, do not start again */
5388-
found = 1;
5389-
break;
5390-
}
5381+
/* this is a new Call Home client, dispatch it */
5382+
rc = _nc_connect_ch_client_dispatch(new_ch_client, server_opts.ch_dispatch_data.acquire_ctx_cb,
5383+
server_opts.ch_dispatch_data.release_ctx_cb, server_opts.ch_dispatch_data.ctx_cb_data,
5384+
server_opts.ch_dispatch_data.new_session_cb, server_opts.ch_dispatch_data.new_session_cb_data);
5385+
if (rc) {
5386+
/* FAILURE! trigger rollback */
5387+
goto rollback;
53915388
}
53925389

5393-
/* CH THREADS UNLOCK */
5394-
nc_rwlock_unlock(&server_opts.ch_threads_lock, __func__);
5395-
5396-
if (!found) {
5397-
/* this is a new Call Home client, dispatch it */
5398-
rc = _nc_connect_ch_client_dispatch(new_ch_client, server_opts.ch_dispatch_data.acquire_ctx_cb,
5399-
server_opts.ch_dispatch_data.release_ctx_cb, server_opts.ch_dispatch_data.ctx_cb_data,
5400-
server_opts.ch_dispatch_data.new_session_cb, server_opts.ch_dispatch_data.new_session_cb_data);
5401-
if (rc) {
5402-
/* FAILURE! trigger rollback */
5403-
goto rollback;
5404-
}
5405-
5406-
/* successfully started, track it for potential rollback */
5407-
LY_ARRAY_NEW_GOTO(NULL, started_clients, client_name, rc, rollback);
5408-
*client_name = strdup(new_ch_client->name);
5409-
NC_CHECK_ERRMEM_GOTO(!*client_name, rc = 1, rollback);
5410-
5411-
/* ownership transferred to array */
5412-
client_name = NULL;
5413-
}
5390+
/* successfully started, track client for potential rollback */
5391+
LY_ARRAY_NEW_GOTO(NULL, started_clients, started_client_ptr, rc, rollback);
5392+
*started_client_ptr = new_ch_client;
54145393
}
54155394
}
54165395

@@ -5428,9 +5407,9 @@ nc_server_config_reconcile_chclients_dispatch(struct nc_server_config *old_cfg,
54285407
}
54295408
}
54305409

5431-
if (!found) {
5410+
if (!found && old_ch_client->thread) {
54325411
/* this Call Home client was deleted, notify it to stop */
5433-
if ((rc = nc_session_server_ch_client_dispatch_stop_if_dispatched(old_ch_client->name, NC_RWLOCK_WRITE))) {
5412+
if ((rc = nc_session_server_ch_client_dispatch_stop(old_ch_client))) {
54345413
ERR(NULL, "Failed to dispatch stop for Call Home client \"%s\".", old_ch_client->name);
54355414
goto rollback;
54365415
}
@@ -5448,15 +5427,12 @@ nc_server_config_reconcile_chclients_dispatch(struct nc_server_config *old_cfg,
54485427
* to return to the pre-call state.
54495428
*/
54505429
LY_ARRAY_FOR(started_clients, i) {
5451-
nc_session_server_ch_client_dispatch_stop_if_dispatched(started_clients[i], NC_RWLOCK_WRITE);
5430+
nc_session_server_ch_client_dispatch_stop(started_clients[i]);
54525431
}
54535432
/* rc is already set to non-zero from the failure point */
54545433

54555434
cleanup:
5456-
/* free the tracking list and its contents */
5457-
LY_ARRAY_FOR(started_clients, i) {
5458-
free(started_clients[i]);
5459-
}
5435+
/* free the tracking list */
54605436
LY_ARRAY_FREE(started_clients);
54615437
return rc;
54625438
}
@@ -6073,6 +6049,8 @@ nc_server_config_dup(const struct nc_server_config *src, struct nc_server_config
60736049
dst_ch_client->max_attempts = src_ch_client->max_attempts;
60746050
dst_ch_client->max_wait = src_ch_client->max_wait;
60756051

6052+
dst_ch_client->thread = src_ch_client->thread;
6053+
60766054
LY_ARRAY_INCREMENT(dst->ch_clients);
60776055
}
60786056

src/session_p.h

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -753,6 +753,8 @@ struct nc_server_config {
753753
NC_CH_START_WITH start_with; /**< How to select the Call Home endpoint to connect to. */
754754
uint8_t max_attempts; /**< Maximum number of attempts to connect to the given Call Home endpoint. */
755755
uint16_t max_wait; /**< Maximum time to wait for a Call Home connection in seconds. */
756+
757+
struct nc_server_ch_thread_arg *thread; /**< Call Home client thread data, if dispatched. */
756758
} *ch_clients; /**< Call Home clients (sized-array, see libyang docs). */
757759

758760
#ifdef NC_ENABLED_SSH_TLS
@@ -784,11 +786,6 @@ struct nc_server_opts {
784786
void *content_id_data; /**< Data passed to the content_id_clb callback. */
785787
void (*content_id_data_free)(void *data); /**< Callback to free the content_id_data. */
786788

787-
/* ACCESS locked - call home thread creation/deletion - WRITE lock
788-
* - call home threads data access (e.g. to signal thread to end) - READ lock */
789-
pthread_rwlock_t ch_threads_lock; /**< Lock for data of Call Home threads. */
790-
struct nc_server_ch_thread_arg **ch_threads; /**< Call Home threads' data, one for each CH client (sized-array, see libyang docs). */
791-
792789
/* ACCESS locked - options modified by YANG data/API - WRITE lock
793790
* - options read when accepting sessions - READ lock */
794791
pthread_rwlock_t config_lock; /**< Lock for the server configuration. */
@@ -1389,20 +1386,19 @@ NC_MSG_TYPE nc_connect_callhome(const char *host, uint16_t port, NC_TRANSPORT_IM
13891386
#ifdef NC_ENABLED_SSH_TLS
13901387

13911388
/**
1392-
* @brief Stop a dispatched Call Home client thread, if such thread was dispatched for the given client name.
1389+
* @brief Stop a dispatched Call Home client thread, if such thread was dispatched for the given client.
1390+
*
1391+
* @warning The caller MUST hold both WRITE config lock and CONFIG APPLY mutex when calling this function.
13931392
*
1394-
* @param[in] client_name Name of the Call Home client to stop the thread for.
1395-
* @param[in] config_lock_mode Lock mode of the configuration lock, if it is held by the caller.
1396-
* If NC_RWLOCK_WRITE is passed, config lock will be briefly unlocked and then locked again.
1397-
* The caller MUST ensure that it holds the CONFIG APPLY mutex, so that nobody steals the wrlock from him.
1398-
* @return 0 if the thread was successfully stopped or not found, 1 on error.
1393+
* @param[in] ch_client Call Home client to stop the thread for, can be NULL.
1394+
* @return 0 if the thread was successfully stopped, 1 on error.
13991395
*/
1400-
int nc_session_server_ch_client_dispatch_stop_if_dispatched(const char *client_name, enum nc_rwlock_mode config_lock_mode);
1396+
int nc_session_server_ch_client_dispatch_stop(struct nc_ch_client *ch_client);
14011397

14021398
/**
14031399
* @brief Dispatch a thread connecting to a listening NETCONF client and creating Call Home sessions.
14041400
*
1405-
* @note The config lock MUST be held.
1401+
* @note The config WRITE lock MUST be held.
14061402
*
14071403
* @param[in] ch_client Call Home client to dispatch the thread for.
14081404
* @param[in] acquire_ctx_cb Callback for acquiring new session context.
@@ -1412,7 +1408,7 @@ int nc_session_server_ch_client_dispatch_stop_if_dispatched(const char *client_n
14121408
* @param[in] new_session_cb_data Arbitrary user data passed to @p new_session_cb.
14131409
* @return 0 if the thread was successfully created, -1 on error.
14141410
*/
1415-
int _nc_connect_ch_client_dispatch(const struct nc_ch_client *ch_client, nc_server_ch_session_acquire_ctx_cb acquire_ctx_cb,
1411+
int _nc_connect_ch_client_dispatch(struct nc_ch_client *ch_client, nc_server_ch_session_acquire_ctx_cb acquire_ctx_cb,
14161412
nc_server_ch_session_release_ctx_cb release_ctx_cb, void *ctx_cb_data, nc_server_ch_new_session_cb new_session_cb,
14171413
void *new_session_cb_data);
14181414

0 commit comments

Comments
 (0)