Skip to content

Commit 53393d3

Browse files
Roytakmichalvasko
authored andcommitted
server config BUGFIX sync concurrent config updates
1 parent 987c5d6 commit 53393d3

3 files changed

Lines changed: 24 additions & 54 deletions

File tree

src/server_config.c

Lines changed: 18 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -6146,52 +6146,6 @@ nc_server_config_cert_exp_notif_thread_wakeup(void)
61466146

61476147
#endif /* NC_ENABLED_SSH_TLS */
61486148

6149-
/**
6150-
* @brief Wait for any pending updates to complete, then mark the server as "applying configuration".
6151-
*
6152-
* @return 0 on success, 1 on timeout.
6153-
*/
6154-
static int
6155-
nc_server_config_update_start(void)
6156-
{
6157-
if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_WRITE, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) {
6158-
return 1;
6159-
}
6160-
6161-
if (!server_opts.applying_config) {
6162-
/* set the flag */
6163-
server_opts.applying_config = 1;
6164-
}
6165-
6166-
/* UNLOCK */
6167-
nc_rwlock_unlock(&server_opts.config_lock, __func__);
6168-
return 0;
6169-
}
6170-
6171-
/**
6172-
* @brief Clear the "applying configuration" flag once the configuration update is done.
6173-
*/
6174-
static void
6175-
nc_server_config_update_end(void)
6176-
{
6177-
int r;
6178-
struct timespec ts_timeout;
6179-
6180-
/* get the time point of timeout */
6181-
nc_timeouttime_get(&ts_timeout, NC_SERVER_CONFIG_UPDATE_WAIT_TIMEOUT_SEC * 1000);
6182-
6183-
/* WR LOCK */
6184-
r = nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_WRITE, NC_CONFIG_LOCK_TIMEOUT, __func__);
6185-
6186-
/* clear the flag */
6187-
server_opts.applying_config = 0;
6188-
6189-
if (r == 1) {
6190-
/* UNLOCK */
6191-
nc_rwlock_unlock(&server_opts.config_lock, __func__);
6192-
}
6193-
}
6194-
61956149
API int
61966150
nc_server_config_setup_diff(const struct lyd_node *data)
61976151
{
@@ -6200,8 +6154,13 @@ nc_server_config_setup_diff(const struct lyd_node *data)
62006154

62016155
NC_CHECK_ARG_RET(NULL, data, 1);
62026156

6203-
/* wait until previous is done, then mark us as applying */
6204-
NC_CHECK_RET(nc_server_config_update_start());
6157+
/* CONFIG UPDATE LOCK
6158+
* - avoids concurrent updates
6159+
* - readers are still allowed to read the old config while we are applying the new one
6160+
*/
6161+
if (nc_mutex_lock(&server_opts.config_update_lock, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) {
6162+
return 1;
6163+
}
62056164

62066165
/* CONFIG RD LOCK */
62076166
if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_READ, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) {
@@ -6269,8 +6228,9 @@ nc_server_config_setup_diff(const struct lyd_node *data)
62696228
/* free the new config in case of error */
62706229
nc_server_config_free(&config_copy);
62716230
}
6272-
nc_server_config_update_end();
62736231

6232+
/* CONFIG UPDATE UNLOCK */
6233+
nc_mutex_unlock(&server_opts.config_update_lock, __func__);
62746234
return ret;
62756235
}
62766236

@@ -6283,8 +6243,13 @@ nc_server_config_setup_data(const struct lyd_node *data)
62836243

62846244
NC_CHECK_ARG_RET(NULL, data, 1);
62856245

6286-
/* wait until previous is done, then mark us as applying */
6287-
NC_CHECK_RET(nc_server_config_update_start());
6246+
/* CONFIG UPDATE LOCK
6247+
* - avoids concurrent updates
6248+
* - readers are still allowed to read the old config while we are applying the new one
6249+
*/
6250+
if (nc_mutex_lock(&server_opts.config_update_lock, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) {
6251+
return 1;
6252+
}
62886253

62896254
/* check that the config data are not diff (no op attr) */
62906255
LY_LIST_FOR(data, tree) {
@@ -6356,8 +6321,9 @@ nc_server_config_setup_data(const struct lyd_node *data)
63566321
/* free the new config in case of error */
63576322
nc_server_config_free(&config);
63586323
}
6359-
nc_server_config_update_end();
63606324

6325+
/* CONFIG UPDATE UNLOCK */
6326+
nc_mutex_unlock(&server_opts.config_update_lock, __func__);
63616327
return ret;
63626328
}
63636329

src/session_p.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -781,7 +781,6 @@ struct nc_server_opts {
781781
* - options read when accepting sessions - READ lock */
782782
pthread_rwlock_t config_lock; /**< Lock for the server configuration. */
783783
struct nc_server_config config; /**< YANG Server configuration. */
784-
int applying_config; /**< Flag indicating that the server configuration is currently being applied. */
785784

786785
#ifdef NC_ENABLED_SSH_TLS
787786
char *authkey_path_fmt; /**< Path to users' public keys that may contain tokens with special meaning. */
@@ -831,6 +830,10 @@ struct nc_server_opts {
831830
/* ACCESS unlocked - set from env */
832831
FILE *tls_keylog_file; /**< File to log TLS secrets to. */
833832
#endif
833+
834+
/* ACCESS locked - separate lock to allow concurrent reads of the config while an update is being applied */
835+
pthread_mutex_t config_update_lock; /**< Lock for synchronizing configuration updates, used to wait for pending
836+
configuration update to complete before accepting a new one. */
834837
};
835838

836839
/**

src/session_server.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,8 @@
5454

5555
struct nc_server_opts server_opts = {
5656
.hello_lock = PTHREAD_RWLOCK_INITIALIZER,
57-
.config_lock = PTHREAD_RWLOCK_INITIALIZER
57+
.config_lock = PTHREAD_RWLOCK_INITIALIZER,
58+
.config_update_lock = PTHREAD_MUTEX_INITIALIZER,
5859
};
5960

6061
static nc_rpc_clb global_rpc_clb = NULL;

0 commit comments

Comments
 (0)