From 814eb8985cd538eefde712e30d3d427368670288 Mon Sep 17 00:00:00 2001 From: Roytak Date: Tue, 9 Dec 2025 16:34:28 +0900 Subject: [PATCH 1/2] session UPDATE add (rw/mut)lock timeout wrapper --- src/session.c | 121 ++++++++++++++++++++++++++++++++++++++++++++++++ src/session_p.h | 59 +++++++++++++++++++++++ 2 files changed, 180 insertions(+) diff --git a/src/session.c b/src/session.c index e803c0db..55401b4c 100644 --- a/src/session.c +++ b/src/session.c @@ -530,6 +530,127 @@ nc_session_client_msgs_unlock(struct nc_session *session, const char *func) return 1; } +int +nc_rwlock_lock(pthread_rwlock_t *rwlock, enum nc_rwlock_mode mode, int timeout, const char *func_name) +{ + int ret; + struct timespec ts_timeout; + + if (!rwlock || (mode == NC_RWLOCK_NONE)) { + ERRINT; + return -1; + } + + if (timeout > 0) { + /* get absolute time for timeout */ + nc_timeouttime_get(&ts_timeout, timeout); + + /* acquire lock with timeout based on mode */ + if (mode == NC_RWLOCK_READ) { + ret = pthread_rwlock_clockrdlock(rwlock, COMPAT_CLOCK_ID, &ts_timeout); + } else { + ret = pthread_rwlock_clockwrlock(rwlock, COMPAT_CLOCK_ID, &ts_timeout); + } + } else if (!timeout) { + /* try to acquire lock without waiting */ + if (mode == NC_RWLOCK_READ) { + ret = pthread_rwlock_tryrdlock(rwlock); + } else { + ret = pthread_rwlock_trywrlock(rwlock); + } + } else { + /* acquire lock without timeout */ + if (mode == NC_RWLOCK_READ) { + ret = pthread_rwlock_rdlock(rwlock); + } else { + ret = pthread_rwlock_wrlock(rwlock); + } + } + + if (ret) { + if ((ret == EBUSY) || (ret == ETIMEDOUT)) { + /* timeout */ + return 0; + } + + ERR(NULL, "%s: failed to lock rwlock in %s mode (%s).", func_name, + mode == NC_RWLOCK_READ ? "read" : "write", strerror(ret)); + return -1; + } + + return 1; +} + +void +nc_rwlock_unlock(pthread_rwlock_t *rwlock, const char *func_name) +{ + int r; + + if (!rwlock) { + ERRINT; + return; + } + + r = pthread_rwlock_unlock(rwlock); + if (r) { + ERR(NULL, "%s: failed to unlock rwlock (%s).", func_name, strerror(r)); + } +} + +int +nc_mutex_lock(pthread_mutex_t *mutex, int timeout, const char *func_name) +{ + int ret; + struct timespec ts_timeout; + + if (!mutex) { + ERRINT; + return -1; + } + + if (timeout > 0) { + /* get absolute time for timeout */ + nc_timeouttime_get(&ts_timeout, timeout); + + /* acquire lock with timeout */ + ret = pthread_mutex_clocklock(mutex, COMPAT_CLOCK_ID, &ts_timeout); + } else if (!timeout) { + /* try to acquire lock without waiting */ + ret = pthread_mutex_trylock(mutex); + } else { + /* acquire lock without timeout */ + ret = pthread_mutex_lock(mutex); + } + + if (ret) { + if ((ret == EBUSY) || (ret == ETIMEDOUT)) { + /* timeout */ + return 0; + } + + ERR(NULL, "%s: failed to lock mutex (%s).", func_name, strerror(ret)); + return -1; + } + + return 1; +} + +void +nc_mutex_unlock(pthread_mutex_t *mutex, const char *func_name) +{ + int r; + + if (!mutex) { + ERRINT; + return; + } + + r = pthread_mutex_unlock(mutex); + if (r) { + ERR(NULL, "%s: failed to unlock mutex (%s).", func_name, strerror(r)); + } +} + API NC_STATUS nc_session_get_status(const struct nc_session *session) { diff --git a/src/session_p.h b/src/session_p.h index 254430c1..bacb00ce 100644 --- a/src/session_p.h +++ b/src/session_p.h @@ -133,6 +133,15 @@ enum nc_operation { NC_OP_REPLACE }; +/** + * @brief Enumeration of rwlock mode. + */ +enum nc_rwlock_mode { + NC_RWLOCK_NONE = 0, /**< Lock not held */ + NC_RWLOCK_READ, /**< Read lock mode */ + NC_RWLOCK_WRITE /**< Write lock mode */ +}; + /** * Enumeration of key or certificate store type. */ @@ -1126,6 +1135,56 @@ int nc_session_client_msgs_lock(struct nc_session *session, int *timeout, const */ int nc_session_client_msgs_unlock(struct nc_session *session, const char *func); +/** + * @brief Lock a pthread_rwlock with timeout support. + * + * @param[in] rwlock Lock to be acquired. + * @param[in] mode Lock mode (NC_RWLOCK_READ for read lock, NC_RWLOCK_WRITE for write lock). + * @param[in] timeout Timeout in milliseconds: + * - Positive value: Wait up to this many milliseconds for the lock. + * - 0: Try to acquire the lock without waiting (trylock). + * - Negative value: Wait indefinitely for the lock. + * @param[in] func_name Caller function name for logging purposes. + * @return 1 on success (lock acquired); + * @return 0 on timeout; + * @return -1 on error. + */ +int nc_rwlock_lock(pthread_rwlock_t *rwlock, enum nc_rwlock_mode mode, int timeout, const char *func_name); + +/** + * @brief Unlock a previously locked pthread_rwlock. + * + * This function unlocks a previously locked pthread_rwlock regardless of whether + * it was locked in read or write mode. + * + * @param[in] rwlock Lock to be unlocked. + * @param[in] func_name Caller function name for logging purposes. + */ +void nc_rwlock_unlock(pthread_rwlock_t *rwlock, const char *func_name); + +/** + * @brief Lock a pthread_mutex with timeout support. + * + * @param[in] mutex Mutex to be acquired. + * @param[in] timeout Timeout in milliseconds: + * - Positive value: Wait up to this many milliseconds for the lock. + * - 0: Try to acquire the lock without waiting (trylock). + * - Negative value: Wait indefinitely for the lock. + * @param[in] func_name Caller function name for logging purposes. + * @return 1 on success (lock acquired); + * @return 0 on timeout; + * @return -1 on error. + */ +int nc_mutex_lock(pthread_mutex_t *mutex, int timeout, const char *func_name); + +/** + * @brief Unlock a previously locked pthread_mutex. + * + * @param[in] mutex Mutex to be unlocked. + * @param[in] func_name Caller function name for logging purposes. + */ +void nc_mutex_unlock(pthread_mutex_t *mutex, const char *func_name); + int nc_ps_lock(struct nc_pollsession *ps, uint8_t *id, const char *func); int nc_ps_unlock(struct nc_pollsession *ps, uint8_t id, const char *func); From 3d504f1312ff9dbb82ef79a3e7628a93bd990d4f Mon Sep 17 00:00:00 2001 From: Roytak Date: Tue, 9 Dec 2025 16:58:39 +0900 Subject: [PATCH 2/2] session UPDATE use locks with timeout --- src/io.c | 14 +- src/server_config.c | 132 +++++++------- src/session.c | 131 ++------------ src/session_client.c | 76 ++------ src/session_client_ssh.c | 4 +- src/session_p.h | 92 +++++----- src/session_server.c | 382 +++++++++++++++++++++++---------------- src/session_server.h | 1 + src/session_server_ssh.c | 18 +- src/session_server_tls.c | 6 +- 10 files changed, 400 insertions(+), 456 deletions(-) diff --git a/src/io.c b/src/io.c index b9837c64..4d75f8cf 100644 --- a/src/io.c +++ b/src/io.c @@ -263,7 +263,7 @@ nc_read_msg_io(struct nc_session *session, int io_timeout, int passing_io_lock, if (!passing_io_lock) { /* SESSION IO LOCK */ - ret = nc_session_io_lock(session, io_timeout, __func__); + ret = nc_mutex_lock(session->io_lock, io_timeout, __func__); if (ret < 1) { goto cleanup; } @@ -347,7 +347,7 @@ nc_read_msg_io(struct nc_session *session, int io_timeout, int passing_io_lock, cleanup: if (io_locked) { /* SESSION IO UNLOCK */ - nc_session_io_unlock(session, __func__); + nc_mutex_unlock(session->io_lock, __func__); } free(frame_size_buf); return ret; @@ -468,7 +468,7 @@ nc_read_msg_poll_io(struct nc_session *session, int io_timeout, struct ly_in **m } /* SESSION IO LOCK */ - ret = nc_session_io_lock(session, io_timeout, __func__); + ret = nc_mutex_lock(session->io_lock, io_timeout, __func__); if (ret < 1) { return ret; } @@ -478,7 +478,7 @@ nc_read_msg_poll_io(struct nc_session *session, int io_timeout, struct ly_in **m /* timed out or error */ /* SESSION IO UNLOCK */ - nc_session_io_unlock(session, __func__); + nc_mutex_unlock(session->io_lock, __func__); return ret; } @@ -486,7 +486,7 @@ nc_read_msg_poll_io(struct nc_session *session, int io_timeout, struct ly_in **m ret = nc_read_msg_io(session, io_timeout, 1, &buf, &buf_len); /* SESSION IO UNLOCK */ - nc_session_io_unlock(session, __func__); + nc_mutex_unlock(session->io_lock, __func__); if (ret > 0) { ret = 1; @@ -840,7 +840,7 @@ nc_write_msg_io(struct nc_session *session, int io_timeout, int type, ...) arg.len = 0; /* SESSION IO LOCK */ - ret = nc_session_io_lock(session, io_timeout, __func__); + ret = nc_mutex_lock(session->io_lock, io_timeout, __func__); if (ret < 0) { return NC_MSG_ERROR; } else if (!ret) { @@ -1049,7 +1049,7 @@ nc_write_msg_io(struct nc_session *session, int io_timeout, int type, ...) cleanup: va_end(ap); - nc_session_io_unlock(session, __func__); + nc_mutex_unlock(session->io_lock, __func__); return ret; } diff --git a/src/server_config.c b/src/server_config.c index 9f856551..bace5b36 100644 --- a/src/server_config.c +++ b/src/server_config.c @@ -3897,7 +3897,9 @@ 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 */ - NC_CHECK_RET(pthread_rwlock_rdlock(&server_opts.ch_threads_lock), 1); + 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) { @@ -3908,14 +3910,15 @@ config_netconf_client(const struct lyd_node *node, enum nc_operation parent_op, if (j < LY_ARRAY_COUNT(server_opts.ch_threads)) { /* the CH thread is running, notify it to stop */ - pthread_mutex_lock(&server_opts.ch_threads[j]->cond_lock); - server_opts.ch_threads[j]->thread_running = 0; - pthread_cond_signal(&server_opts.ch_threads[j]->cond); - pthread_mutex_unlock(&server_opts.ch_threads[j]->cond_lock); + 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 */ - pthread_rwlock_unlock(&server_opts.ch_threads_lock); + nc_rwlock_unlock(&server_opts.ch_threads_lock, __func__); /* we can use 'i' from above */ if (i < LY_ARRAY_COUNT(config->ch_clients) - 1) { @@ -5256,7 +5259,10 @@ nc_server_config_reconcile_chclients_dispatch(struct nc_server_config *old_cfg, found = 0; /* CH THREADS LOCK (reading server_opts.ch_threads) */ - pthread_rwlock_rdlock(&server_opts.ch_threads_lock); + 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)) { @@ -5267,7 +5273,7 @@ nc_server_config_reconcile_chclients_dispatch(struct nc_server_config *old_cfg, } /* CH THREADS UNLOCK */ - pthread_rwlock_unlock(&server_opts.ch_threads_lock); + nc_rwlock_unlock(&server_opts.ch_threads_lock, __func__); if (!found) { /* this is a new Call Home client, dispatch it */ @@ -5308,19 +5314,23 @@ nc_server_config_reconcile_chclients_dispatch(struct nc_server_config *old_cfg, ch_thread_arg = NULL; /* CH THREADS LOCK (reading server_opts.ch_threads) */ - pthread_rwlock_rdlock(&server_opts.ch_threads_lock); + 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 */ - pthread_mutex_lock(&(*ch_thread_arg)->cond_lock); - (*ch_thread_arg)->thread_running = 0; - pthread_cond_signal(&(*ch_thread_arg)->cond); - pthread_mutex_unlock(&(*ch_thread_arg)->cond_lock); + 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; } } /* CH THREADS UNLOCK */ - pthread_rwlock_unlock(&server_opts.ch_threads_lock); + 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. */ } } @@ -5339,19 +5349,22 @@ nc_server_config_reconcile_chclients_dispatch(struct nc_server_config *old_cfg, ch_thread_arg = NULL; /* CH THREADS LOCK (reading server_opts.ch_threads) */ - pthread_rwlock_rdlock(&server_opts.ch_threads_lock); + 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 */ - pthread_mutex_lock(&(*ch_thread_arg)->cond_lock); + 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); - pthread_mutex_unlock(&(*ch_thread_arg)->cond_lock); + nc_mutex_unlock(&(*ch_thread_arg)->cond_lock, __func__); break; } } /* CH THREADS UNLOCK */ - pthread_rwlock_unlock(&server_opts.ch_threads_lock); + nc_rwlock_unlock(&server_opts.ch_threads_lock, __func__); } /* rc is already set to non-zero from the failure point */ @@ -6014,11 +6027,13 @@ nc_server_config_dup(const struct nc_server_config *src, struct nc_server_config static void nc_server_config_cert_exp_notif_thread_wakeup(void) { - pthread_mutex_lock(&server_opts.cert_exp_notif.lock); + if (nc_mutex_lock(&server_opts.cert_exp_notif.lock, NC_CERT_EXP_LOCK_TIMEOUT, __func__) != 1) { + return; + } if (server_opts.cert_exp_notif.thread_running) { pthread_cond_signal(&server_opts.cert_exp_notif.cond); } - pthread_mutex_unlock(&server_opts.cert_exp_notif.lock); + nc_mutex_unlock(&server_opts.cert_exp_notif.lock, __func__); } #endif /* NC_ENABLED_SSH_TLS */ @@ -6031,38 +6046,18 @@ nc_server_config_cert_exp_notif_thread_wakeup(void) static int nc_server_config_update_start(void) { - int r; - struct timespec ts_timeout; - - /* get the time point of timeout */ - nc_timeouttime_get(&ts_timeout, NC_SERVER_CONFIG_UPDATE_WAIT_TIMEOUT_SEC * 1000); - - while (nc_timeouttime_cur_diff(&ts_timeout) > 0) { - /* WR LOCK */ - r = pthread_rwlock_clockwrlock(&server_opts.config_lock, COMPAT_CLOCK_ID, &ts_timeout); - if (r == ETIMEDOUT) { - break; - } else if (r) { - ERR(NULL, "Failed to acquire server configuration write lock (%s).", strerror(r)); - return 1; - } - - if (!server_opts.applying_config) { - /* set the flag and end */ - server_opts.applying_config = 1; - - /* UNLOCK */ - pthread_rwlock_unlock(&server_opts.config_lock); - return 0; - } + if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_WRITE, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + return 1; + } - /* UNLOCK and wait */ - pthread_rwlock_unlock(&server_opts.config_lock); - usleep(NC_TIMEOUT_STEP); + if (!server_opts.applying_config) { + /* set the flag */ + server_opts.applying_config = 1; } - ERR(NULL, "Timeout expired while waiting for the server to apply the previous configuration."); - return 1; + /* UNLOCK */ + nc_rwlock_unlock(&server_opts.config_lock, __func__); + return 0; } /** @@ -6078,18 +6073,14 @@ nc_server_config_update_end(void) nc_timeouttime_get(&ts_timeout, NC_SERVER_CONFIG_UPDATE_WAIT_TIMEOUT_SEC * 1000); /* WR LOCK */ - r = pthread_rwlock_clockwrlock(&server_opts.config_lock, COMPAT_CLOCK_ID, &ts_timeout); - if (r) { - /* just log the error, there is nothing we can do */ - ERR(NULL, "Failed to acquire server configuration write lock (%s).", strerror(r)); - } + r = nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_WRITE, NC_CONFIG_LOCK_TIMEOUT, __func__); /* clear the flag */ server_opts.applying_config = 0; - if (!r) { - /* UNLOCK only if we locked it */ - pthread_rwlock_unlock(&server_opts.config_lock); + if (r == 1) { + /* UNLOCK */ + nc_rwlock_unlock(&server_opts.config_lock, __func__); } } @@ -6105,14 +6096,17 @@ nc_server_config_setup_diff(const struct lyd_node *data) NC_CHECK_RET(nc_server_config_update_start()); /* CONFIG RD LOCK */ - pthread_rwlock_rdlock(&server_opts.config_lock); + if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_READ, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + ret = 1; + goto cleanup; + } /* create a copy of the current config to work with, so that we can revert to it in case of error */ NC_CHECK_ERR_GOTO(ret = nc_server_config_dup(&server_opts.config, &config_copy), ERR(NULL, "Duplicating current server configuration failed."), cleanup_unlock); /* UNLOCK */ - pthread_rwlock_unlock(&server_opts.config_lock); + nc_rwlock_unlock(&server_opts.config_lock, __func__); #ifdef NC_ENABLED_SSH_TLS /* configure keystore */ @@ -6133,7 +6127,10 @@ nc_server_config_setup_diff(const struct lyd_node *data) ERR(NULL, "Applying libnetconf2-netconf-server configuration failed."), cleanup); /* CONFIG WR LOCK */ - pthread_rwlock_wrlock(&server_opts.config_lock); + if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_WRITE, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + ret = 1; + goto cleanup; + } /* start listening on new endpoints */ NC_CHECK_ERR_GOTO(ret = nc_server_config_reconcile_sockets_listen(&server_opts.config, &config_copy), @@ -6157,7 +6154,7 @@ nc_server_config_setup_diff(const struct lyd_node *data) cleanup_unlock: /* CONFIG UNLOCK */ - pthread_rwlock_unlock(&server_opts.config_lock); + nc_rwlock_unlock(&server_opts.config_lock, __func__); cleanup: if (ret) { @@ -6217,7 +6214,10 @@ nc_server_config_setup_data(const struct lyd_node *data) ERR(NULL, "Applying libnetconf2-netconf-server configuration failed."), cleanup); /* CONFIG LOCK */ - pthread_rwlock_wrlock(&server_opts.config_lock); + if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_WRITE, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + ret = 1; + goto cleanup; + } /* start listening on new endpoints */ NC_CHECK_ERR_GOTO(ret = nc_server_config_reconcile_sockets_listen(&server_opts.config, &config), @@ -6241,7 +6241,7 @@ nc_server_config_setup_data(const struct lyd_node *data) cleanup_unlock: /* CONFIG UNLOCK */ - pthread_rwlock_unlock(&server_opts.config_lock); + nc_rwlock_unlock(&server_opts.config_lock, __func__); cleanup: if (ret) { @@ -6504,7 +6504,9 @@ nc_server_config_oper_get_user_password_last_modified(const char *ch_client, con *last_modified = 0; /* LOCK */ - pthread_rwlock_rdlock(&server_opts.config_lock); + if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_READ, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + return 1; + } if (ch_client) { /* find the call-home client */ @@ -6566,7 +6568,7 @@ nc_server_config_oper_get_user_password_last_modified(const char *ch_client, con cleanup: /* UNLOCK */ - pthread_rwlock_unlock(&server_opts.config_lock); + nc_rwlock_unlock(&server_opts.config_lock, __func__); return rc; } diff --git a/src/session.c b/src/session.c index 55401b4c..dc07b300 100644 --- a/src/session.c +++ b/src/session.c @@ -427,109 +427,6 @@ nc_session_rpc_unlock(struct nc_session *session, int timeout, const char *func) return 1; } -int -nc_session_io_lock(struct nc_session *session, int timeout, const char *func) -{ - int ret; - struct timespec ts_timeout; - - if (timeout > 0) { - nc_timeouttime_get(&ts_timeout, timeout); - - ret = pthread_mutex_clocklock(session->io_lock, COMPAT_CLOCK_ID, &ts_timeout); - } else if (!timeout) { - ret = pthread_mutex_trylock(session->io_lock); - } else { /* timeout == -1 */ - ret = pthread_mutex_lock(session->io_lock); - } - - if (ret) { - if ((ret == EBUSY) || (ret == ETIMEDOUT)) { - /* timeout */ - return 0; - } - - /* error */ - ERR(session, "%s: failed to IO lock a session (%s).", func, strerror(ret)); - return -1; - } - - return 1; -} - -int -nc_session_io_unlock(struct nc_session *session, const char *func) -{ - int ret; - - ret = pthread_mutex_unlock(session->io_lock); - if (ret) { - /* error */ - ERR(session, "%s: failed to IO unlock a session (%s).", func, strerror(ret)); - return -1; - } - - return 1; -} - -int -nc_session_client_msgs_lock(struct nc_session *session, int *timeout, const char *func) -{ - int ret; - int32_t diff_msec; - struct timespec ts_timeout, ts_start; - - assert(session->side == NC_CLIENT); - - if (*timeout > 0) { - /* get current time */ - nc_timeouttime_get(&ts_start, 0); - - nc_timeouttime_get(&ts_timeout, *timeout); - - ret = pthread_mutex_clocklock(&session->opts.client.msgs_lock, COMPAT_CLOCK_ID, &ts_timeout); - if (!ret) { - /* update timeout based on what was elapsed */ - diff_msec = nc_timeouttime_cur_diff(&ts_start); - *timeout -= diff_msec; - } - } else if (!*timeout) { - ret = pthread_mutex_trylock(&session->opts.client.msgs_lock); - } else { /* timeout == -1 */ - ret = pthread_mutex_lock(&session->opts.client.msgs_lock); - } - - if (ret) { - if ((ret == EBUSY) || (ret == ETIMEDOUT)) { - /* timeout */ - return 0; - } - - /* error */ - ERR(session, "%s: failed to MSGS lock a session (%s).", func, strerror(ret)); - return -1; - } - - return 1; -} - -int -nc_session_client_msgs_unlock(struct nc_session *session, const char *func) -{ - int ret; - - assert(session->side == NC_CLIENT); - - ret = pthread_mutex_unlock(&session->opts.client.msgs_lock); - if (ret) { - /* error */ - ERR(session, "%s: failed to MSGS unlock a session (%s).", func, strerror(ret)); - return -1; - } - - return 1; -} - int nc_rwlock_lock(pthread_rwlock_t *rwlock, enum nc_rwlock_mode mode, int timeout, const char *func_name) { @@ -909,7 +806,7 @@ nc_session_free_transport(struct nc_session *session, int *multisession) * it. Also, avoid concurrent free by multiple threads of sessions that share the SSH session. */ /* SESSION IO LOCK */ - r = nc_session_io_lock(session, NC_SESSION_FREE_LOCK_TIMEOUT, __func__); + r = nc_mutex_lock(session->io_lock, NC_SESSION_FREE_LOCK_TIMEOUT, __func__); if (session->ti.libssh.next) { for (siter = session->ti.libssh.next; siter != session; siter = siter->ti.libssh.next) { @@ -964,7 +861,7 @@ nc_session_free_transport(struct nc_session *session, int *multisession) /* SESSION IO UNLOCK */ if (r == 1) { - nc_session_io_unlock(session, __func__); + nc_mutex_unlock(session->io_lock, __func__); } break; } @@ -1002,7 +899,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, msgs_locked = 0, timeout; + int r, i, rpc_locked = 0, msgs_locked = 0; int multisession = 0; /* flag for more NETCONF sessions on a single SSH session */ struct nc_msg_cont *contiter; struct ly_in *msg; @@ -1016,14 +913,14 @@ nc_session_free(struct nc_session *session, void (*data_free)(void *)) if ((session->side == NC_SERVER) && (session->flags & NC_SESSION_CALLHOME)) { /* CH LOCK */ - pthread_mutex_lock(&session->opts.server.ch_lock); + nc_mutex_lock(&session->opts.server.ch_lock, NC_SESSION_CH_LOCK_TIMEOUT, __func__); } status = session->status; if ((session->side == NC_SERVER) && (session->flags & NC_SESSION_CALLHOME)) { /* CH UNLOCK */ - pthread_mutex_unlock(&session->opts.server.ch_lock); + nc_mutex_unlock(&session->opts.server.ch_lock, __func__); } if (status == NC_STATUS_CLOSING) { @@ -1066,10 +963,8 @@ nc_session_free(struct nc_session *session, void (*data_free)(void *)) } if (session->side == NC_CLIENT) { - timeout = NC_SESSION_FREE_LOCK_TIMEOUT; - /* MSGS LOCK */ - r = nc_session_client_msgs_lock(session, &timeout, __func__); + r = nc_mutex_lock(&session->opts.client.msgs_lock, NC_SESSION_FREE_LOCK_TIMEOUT, __func__); if (r == -1) { return; } else if (r) { @@ -1090,7 +985,7 @@ nc_session_free(struct nc_session *session, void (*data_free)(void *)) if (msgs_locked) { /* MSGS UNLOCK */ - nc_session_client_msgs_unlock(session, __func__); + nc_mutex_unlock(&session->opts.client.msgs_lock, __func__); } if ((session->status == NC_STATUS_RUNNING) && nc_session_is_connected(session)) { @@ -1118,7 +1013,7 @@ nc_session_free(struct nc_session *session, void (*data_free)(void *)) if ((session->side == NC_SERVER) && (session->flags & NC_SESSION_CALLHOME)) { /* CH LOCK */ - pthread_mutex_lock(&session->opts.server.ch_lock); + nc_mutex_lock(&session->opts.server.ch_lock, NC_SESSION_CH_LOCK_TIMEOUT, __func__); } /* mark session for closing */ @@ -1141,7 +1036,7 @@ nc_session_free(struct nc_session *session, void (*data_free)(void *)) if ((session->side == NC_SERVER) && (session->flags & NC_SESSION_CALLHOME)) { /* CH UNLOCK */ - pthread_mutex_unlock(&session->opts.server.ch_lock); + nc_mutex_unlock(&session->opts.server.ch_lock, __func__); } /* transport implementation cleanup */ @@ -1277,7 +1172,9 @@ nc_server_get_cpblts_version(const struct ly_ctx *ctx, LYS_VERSION version) } /* HELLO LOCK */ - pthread_rwlock_rdlock(&server_opts.hello_lock); + if (nc_rwlock_lock(&server_opts.hello_lock, NC_RWLOCK_READ, NC_HELLO_LOCK_TIMEOUT, __func__) != 1) { + goto error; + } mod = ly_ctx_get_module_implemented(ctx, "ietf-netconf-with-defaults"); if (mod) { @@ -1421,7 +1318,7 @@ nc_server_get_cpblts_version(const struct ly_ctx *ctx, LYS_VERSION version) } /* HELLO UNLOCK */ - pthread_rwlock_unlock(&server_opts.hello_lock); + nc_rwlock_unlock(&server_opts.hello_lock, __func__); /* ending NULL capability */ add_cpblt(NULL, &cpblts, &size, &count); @@ -1430,7 +1327,7 @@ nc_server_get_cpblts_version(const struct ly_ctx *ctx, LYS_VERSION version) unlock_error: /* HELLO UNLOCK */ - pthread_rwlock_unlock(&server_opts.hello_lock); + nc_rwlock_unlock(&server_opts.hello_lock, __func__); error: free(cpblts); diff --git a/src/session_client.c b/src/session_client.c index e1014980..aaf4f058 100644 --- a/src/session_client.c +++ b/src/session_client.c @@ -2127,7 +2127,7 @@ recv_msg(struct nc_session *session, int timeout, NC_MSG_TYPE expected, struct l *message = NULL; /* MSGS LOCK */ - r = nc_session_client_msgs_lock(session, &timeout, __func__); + r = nc_mutex_lock(&session->opts.client.msgs_lock, timeout, __func__); if (!r) { ret = NC_MSG_WOULDBLOCK; goto cleanup; @@ -2189,7 +2189,7 @@ recv_msg(struct nc_session *session, int timeout, NC_MSG_TYPE expected, struct l cleanup_unlock: /* MSGS UNLOCK */ - nc_session_client_msgs_unlock(session, __func__); + nc_mutex_unlock(&session->opts.client.msgs_lock, __func__); cleanup: if (ret == expected) { @@ -3260,55 +3260,15 @@ nc_client_monitoring_get_session_fd(struct nc_session *session) return fd; } -/** - * @brief Lock the client monitoring data. - * - * @param[in] timeout Timeout in msec to use. - * - * @return 1 on success; - * @return 0 on timeout; - * @return -1 on error. - */ -static int -nc_client_monitoring_lock(int timeout) -{ - int r; - struct timespec ts; - - if (timeout > 0) { - nc_timeouttime_get(&ts, timeout); - r = pthread_mutex_clocklock(&client_opts.monitoring_thread_data.lock, COMPAT_CLOCK_ID, &ts); - } else if (!timeout) { - r = pthread_mutex_trylock(&client_opts.monitoring_thread_data.lock); - } else { - r = pthread_mutex_lock(&client_opts.monitoring_thread_data.lock); - } - - if (r) { - ERR(NULL, "Failed to lock the client monitoring data lock (%s).", strerror(r)); - - if ((r == EBUSY) || (r == ETIMEDOUT)) { - /* timeout */ - return 0; - } - - /* error */ - return -1; - } - - return 1; -} - int nc_client_monitoring_session_start(struct nc_session *session) { - int ret = 0, fd, r; + int ret = 0, fd; struct nc_client_monitoring_thread_arg *mtarg; void *tmp, *tmp2; /* LOCK */ - r = nc_client_monitoring_lock(NC_CLIENT_MONITORING_LOCK_TIMEOUT); - if (r < 1) { + if (nc_mutex_lock(&client_opts.monitoring_thread_data.lock, NC_CLIENT_MONITORING_LOCK_TIMEOUT, __func__) != 1) { return 1; } @@ -3344,20 +3304,19 @@ nc_client_monitoring_session_start(struct nc_session *session) cleanup: /* UNLOCK */ - pthread_mutex_unlock(&client_opts.monitoring_thread_data.lock); + nc_mutex_unlock(&client_opts.monitoring_thread_data.lock, __func__); return ret; } void nc_client_monitoring_session_stop(struct nc_session *session, int lock) { - int i, r; + int i; struct nc_client_monitoring_thread_arg *mtarg; if (lock) { /* LOCK */ - r = nc_client_monitoring_lock(NC_CLIENT_MONITORING_LOCK_TIMEOUT); - if (r < 1) { + if (nc_mutex_lock(&client_opts.monitoring_thread_data.lock, NC_CLIENT_MONITORING_LOCK_TIMEOUT, __func__) != 1) { return; } } @@ -3399,7 +3358,7 @@ nc_client_monitoring_session_stop(struct nc_session *session, int lock) cleanup: if (lock) { /* UNLOCK */ - pthread_mutex_unlock(&mtarg->lock); + nc_mutex_unlock(&mtarg->lock, __func__); } } @@ -3421,8 +3380,7 @@ nc_client_monitoring_thread(void *arg) nc_client_set_thread_context(arg); /* LOCK */ - r = nc_client_monitoring_lock(NC_CLIENT_MONITORING_LOCK_TIMEOUT); - if (r < 1) { + if (nc_mutex_lock(&client_opts.monitoring_thread_data.lock, NC_CLIENT_MONITORING_LOCK_TIMEOUT, __func__) != 1) { goto cleanup; } locked = 1; @@ -3473,8 +3431,7 @@ nc_client_monitoring_thread(void *arg) usleep(NC_CLIENT_MONITORING_BACKOFF * 1000); /* LOCK */ - r = nc_client_monitoring_lock(NC_CLIENT_MONITORING_LOCK_TIMEOUT); - if (r < 1) { + if (nc_mutex_lock(&client_opts.monitoring_thread_data.lock, NC_CLIENT_MONITORING_LOCK_TIMEOUT, __func__) != 1) { goto cleanup; } locked = 1; @@ -3498,8 +3455,7 @@ nc_client_monitoring_thread_start(nc_client_monitoring_clb monitoring_clb, void NC_CHECK_ARG_RET(NULL, monitoring_clb, 1); /* LOCK */ - r = nc_client_monitoring_lock(NC_CLIENT_MONITORING_LOCK_TIMEOUT); - if (r < 1) { + if (nc_mutex_lock(&client_opts.monitoring_thread_data.lock, NC_CLIENT_MONITORING_LOCK_TIMEOUT, __func__) != 1) { return 1; } @@ -3533,7 +3489,7 @@ nc_client_monitoring_thread_start(nc_client_monitoring_clb monitoring_clb, void cleanup: /* UNLOCK */ - pthread_mutex_unlock(&client_opts.monitoring_thread_data.lock); + nc_mutex_unlock(&client_opts.monitoring_thread_data.lock, __func__); return ret; } @@ -3545,8 +3501,7 @@ nc_client_monitoring_thread_stop(void) struct nc_client_monitoring_thread_arg *mtarg; /* LOCK */ - r = nc_client_monitoring_lock(NC_CLIENT_MONITORING_LOCK_TIMEOUT); - if (r < 1) { + if (nc_mutex_lock(&client_opts.monitoring_thread_data.lock, NC_CLIENT_MONITORING_LOCK_TIMEOUT, __func__) != 1) { return; } @@ -3554,7 +3509,7 @@ nc_client_monitoring_thread_stop(void) ERR(NULL, "Client monitoring thread is not running."); /* UNLOCK */ - pthread_mutex_unlock(&client_opts.monitoring_thread_data.lock); + nc_mutex_unlock(&client_opts.monitoring_thread_data.lock, __func__); return; } @@ -3577,8 +3532,7 @@ nc_client_monitoring_thread_stop(void) } /* LOCK */ - r = nc_client_monitoring_lock(NC_CLIENT_MONITORING_LOCK_TIMEOUT); - if (r < 1) { + if (nc_mutex_lock(&client_opts.monitoring_thread_data.lock, NC_CLIENT_MONITORING_LOCK_TIMEOUT, __func__) != 1) { return; } diff --git a/src/session_client_ssh.c b/src/session_client_ssh.c index b1bb6cac..adfbf4f5 100644 --- a/src/session_client_ssh.c +++ b/src/session_client_ssh.c @@ -1915,13 +1915,13 @@ nc_connect_ssh_channel(struct nc_session *session, struct ly_ctx *ctx) } /* create the channel safely */ - if (nc_session_io_lock(new_session, -1, __func__) != 1) { + if (nc_mutex_lock(new_session->io_lock, -1, __func__) != 1) { goto fail; } if (open_netconf_channel(new_session, NC_TRANSPORT_TIMEOUT) != 1) { goto fail; } - nc_session_io_unlock(new_session, __func__); + nc_mutex_unlock(new_session->io_lock, __func__); if (nc_client_session_new_ctx(new_session, ctx) != EXIT_SUCCESS) { goto fail; diff --git a/src/session_p.h b/src/session_p.h index bacb00ce..260cd987 100644 --- a/src/session_p.h +++ b/src/session_p.h @@ -99,6 +99,54 @@ extern struct nc_server_opts server_opts; */ #define NC_CH_CONNECT_TIMEOUT 500 +/** + * @brief Timeout in msec for acquiring the hello_lock + * (iterating through all YANG modules + building capability strings) + */ +#define NC_HELLO_LOCK_TIMEOUT 5000 + +/** + * @brief Timeout in msec for acquiring the ch_threads_lock + * (simple array iteration and thread signaling operations) + */ +#define NC_CH_THREADS_LOCK_TIMEOUT 1000 + +/** + * @brief Timeout in msec for acquiring the Call Home thread cond_lock + * (short critical sections used to signal thread termination and gate condition waits) + */ +#define NC_CH_COND_LOCK_TIMEOUT 1000 + +/** + * @brief Timeout in msec for acquiring the certificate expiration notification lock + * (short critical sections for flag updates and condition signaling) + */ +#define NC_CERT_EXP_LOCK_TIMEOUT 1000 + +/** + * @brief Timeout in msec for acquiring the config_lock + * (socket binding and Call Home client dispatching can involve network operations) + */ +#define NC_CONFIG_LOCK_TIMEOUT 10000 + +/** + * @brief Timeout in msec for acquiring session's ch_lock + * (just simple session flag checks and updates) + */ +#define NC_SESSION_CH_LOCK_TIMEOUT 1000 + +/** + * @brief Timeout in msec for acquiring the pollsession's lock + * (only O(n) array manipulation, where n is number of sessions (small usually)) + */ +#define NC_PS_LOCK_TIMEOUT 1000 + +/** + * @brief Timeout in msec for acquiring the notification status lock + * (short critical sections for incrementing/decrementing notification status) + */ +#define NC_SESSION_NTF_STATUS_LOCK_TIMEOUT 500 + /** * Number of sockets kept waiting to be accepted. */ @@ -1091,50 +1139,6 @@ int nc_session_rpc_lock(struct nc_session *session, int timeout, const char *fun int nc_session_rpc_unlock(struct nc_session *session, int timeout, const char *func); -/** - * @brief Lock IO lock on a session. - * - * @param[in] session Session to lock. - * @param[in] timeout Timeout in msec to use. - * @param[in] func Caller function for logging. - * @return 1 on success; - * @return 0 on timeout; - * @return -1 on error. - */ -int nc_session_io_lock(struct nc_session *session, int timeout, const char *func); - -/** - * @brief Unlock IO lock on a session. - * - * @param[in] session Session to unlock. - * @param[in] func Caller function for logging. - * @return 1 on success; - * @return -1 on error. - */ -int nc_session_io_unlock(struct nc_session *session, const char *func); - -/** - * @brief Lock MSGS lock on a session. - * - * @param[in] session Session to lock. - * @param[in,out] timeout Timeout in msec to use. If positive and on successful lock, is updated based on what was elapsed. - * @param[in] func Caller function for logging. - * @return 1 on success; - * @return 0 on timeout; - * @return -1 on error. - */ -int nc_session_client_msgs_lock(struct nc_session *session, int *timeout, const char *func); - -/** - * @brief Unlock MSGS lock on a session. - * - * @param[in] session Session to unlock. - * @param[in] func Caller function for logging. - * @return 1 on success; - * @return -1 on error. - */ -int nc_session_client_msgs_unlock(struct nc_session *session, const char *func); - /** * @brief Lock a pthread_rwlock with timeout support. * diff --git a/src/session_server.c b/src/session_server.c index 929f8bf2..767ed4c7 100644 --- a/src/session_server.c +++ b/src/session_server.c @@ -241,7 +241,9 @@ nc_server_ch_set_dispatch_data(nc_server_ch_session_acquire_ctx_cb acquire_ctx_c NC_CHECK_ARG_RET(NULL, acquire_ctx_cb, release_ctx_cb, new_session_cb, ); /* CONFIG WRITE LOCK */ - pthread_rwlock_wrlock(&server_opts.config_lock); + if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_WRITE, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + return; + } server_opts.ch_dispatch_data.acquire_ctx_cb = acquire_ctx_cb; server_opts.ch_dispatch_data.release_ctx_cb = release_ctx_cb; @@ -250,7 +252,7 @@ nc_server_ch_set_dispatch_data(nc_server_ch_session_acquire_ctx_cb acquire_ctx_c server_opts.ch_dispatch_data.new_session_cb_data = new_session_cb_data; /* CONFIG WRITE UNLOCK */ - pthread_rwlock_unlock(&server_opts.config_lock); + nc_rwlock_unlock(&server_opts.config_lock, __func__); } #endif @@ -662,7 +664,10 @@ nc_sock_accept_binds(struct nc_endpt *endpt, struct nc_bind *binds, uint16_t bin NC_CHECK_ERRMEM_RET(!pfd, -1); /* LOCK */ - pthread_mutex_lock(bind_lock); + if (nc_mutex_lock(bind_lock, timeout, __func__) != 1) { + free(pfd); + return -1; + } for (i = 0, pfd_count = 0; i < bind_count; ++i) { if (binds[i].sock < 0) { @@ -689,7 +694,7 @@ nc_sock_accept_binds(struct nc_endpt *endpt, struct nc_bind *binds, uint16_t bin free(pfd); /* UNLOCK */ - pthread_mutex_unlock(bind_lock); + nc_mutex_unlock(bind_lock, __func__); return ret; } @@ -718,7 +723,7 @@ nc_sock_accept_binds(struct nc_endpt *endpt, struct nc_bind *binds, uint16_t bin if (server_sock == -1) { ERRINT; /* UNLOCK */ - pthread_mutex_unlock(bind_lock); + nc_mutex_unlock(bind_lock, __func__); return -1; } @@ -727,7 +732,7 @@ nc_sock_accept_binds(struct nc_endpt *endpt, struct nc_bind *binds, uint16_t bin if (client_sock < 0) { ERR(NULL, "Accept failed (%s).", strerror(errno)); /* UNLOCK */ - pthread_mutex_unlock(bind_lock); + nc_mutex_unlock(bind_lock, __func__); return -1; } @@ -778,7 +783,7 @@ nc_sock_accept_binds(struct nc_endpt *endpt, struct nc_bind *binds, uint16_t bin *idx = i; } /* UNLOCK */ - pthread_mutex_unlock(bind_lock); + nc_mutex_unlock(bind_lock, __func__); *sock = client_sock; return 1; @@ -786,7 +791,7 @@ nc_sock_accept_binds(struct nc_endpt *endpt, struct nc_bind *binds, uint16_t bin fail: close(client_sock); /* UNLOCK */ - pthread_mutex_unlock(bind_lock); + nc_mutex_unlock(bind_lock, __func__); return -1; } @@ -1075,24 +1080,26 @@ nc_server_destroy(void) #endif /* NC_ENABLED_SSH_TLS */ /* CONFIG WR LOCK */ - pthread_rwlock_wrlock(&server_opts.config_lock); + nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_WRITE, NC_CONFIG_LOCK_TIMEOUT, __func__); /* destroy the server configuration */ nc_server_config_free(&server_opts.config); /* CH THREADS LOCK */ - pthread_rwlock_rdlock(&server_opts.ch_threads_lock); + nc_rwlock_lock(&server_opts.ch_threads_lock, NC_RWLOCK_READ, NC_CH_THREADS_LOCK_TIMEOUT, __func__); /* notify the CH threads to exit */ LY_ARRAY_FOR(server_opts.ch_threads, i) { - pthread_mutex_lock(&server_opts.ch_threads[i]->cond_lock); + if (nc_mutex_lock(&server_opts.ch_threads[i]->cond_lock, NC_CH_COND_LOCK_TIMEOUT, __func__) != 1) { + continue; + } server_opts.ch_threads[i]->thread_running = 0; pthread_cond_signal(&server_opts.ch_threads[i]->cond); - pthread_mutex_unlock(&server_opts.ch_threads[i]->cond_lock); + nc_mutex_unlock(&server_opts.ch_threads[i]->cond_lock, __func__); } /* CH THREADS UNLOCK */ - pthread_rwlock_unlock(&server_opts.ch_threads_lock); + nc_rwlock_unlock(&server_opts.ch_threads_lock, __func__); #ifdef NC_ENABLED_SSH_TLS free(server_opts.authkey_path_fmt); @@ -1115,7 +1122,7 @@ nc_server_destroy(void) server_opts.unix_paths = NULL; /* CONFIG WR UNLOCK */ - pthread_rwlock_unlock(&server_opts.config_lock); + nc_rwlock_unlock(&server_opts.config_lock, __func__); #ifdef NC_ENABLED_SSH_TLS curl_global_cleanup(); @@ -1141,13 +1148,15 @@ nc_server_set_capab_withdefaults(NC_WD_MODE basic_mode, int also_supported) } /* HELLO LOCK */ - pthread_rwlock_wrlock(&server_opts.hello_lock); + if (nc_rwlock_lock(&server_opts.hello_lock, NC_RWLOCK_WRITE, NC_HELLO_LOCK_TIMEOUT, __func__) != 1) { + return -1; + } server_opts.wd_basic_mode = basic_mode; server_opts.wd_also_supported = also_supported; /* HELLO UNLOCK */ - pthread_rwlock_unlock(&server_opts.hello_lock); + nc_rwlock_unlock(&server_opts.hello_lock, __func__); return 0; } @@ -1160,8 +1169,8 @@ nc_server_get_capab_withdefaults(NC_WD_MODE *basic_mode, int *also_supported) return; } - /* HELLO LOCK */ - pthread_rwlock_wrlock(&server_opts.hello_lock); + /* HELLO LOCK, nothing to do on failure */ + nc_rwlock_lock(&server_opts.hello_lock, NC_RWLOCK_WRITE, NC_HELLO_LOCK_TIMEOUT, __func__); if (basic_mode) { *basic_mode = server_opts.wd_basic_mode; @@ -1171,54 +1180,52 @@ nc_server_get_capab_withdefaults(NC_WD_MODE *basic_mode, int *also_supported) } /* HELLO UNLOCK */ - pthread_rwlock_unlock(&server_opts.hello_lock); + nc_rwlock_unlock(&server_opts.hello_lock, __func__); } API int nc_server_set_capability(const char *value) { + int rc = 0; void *mem; if (!value || !value[0]) { ERRARG(NULL, "value must not be empty"); - return EXIT_FAILURE; + return -1; } /* HELLO LOCK */ - pthread_rwlock_wrlock(&server_opts.hello_lock); + if (nc_rwlock_lock(&server_opts.hello_lock, NC_RWLOCK_WRITE, NC_HELLO_LOCK_TIMEOUT, __func__) != 1) { + return -1; + } mem = realloc(server_opts.capabilities, (server_opts.capabilities_count + 1) * sizeof *server_opts.capabilities); - if (!mem) { - /* HELLO UNLOCK */ - pthread_rwlock_unlock(&server_opts.hello_lock); - ERRMEM; - return EXIT_FAILURE; - } + NC_CHECK_ERRMEM_GOTO(!mem, rc = -1, cleanup); server_opts.capabilities = mem; server_opts.capabilities[server_opts.capabilities_count] = strdup(value); server_opts.capabilities_count++; +cleanup: /* HELLO UNLOCK */ - pthread_rwlock_unlock(&server_opts.hello_lock); - - return EXIT_SUCCESS; + nc_rwlock_unlock(&server_opts.hello_lock, __func__); + return rc; } API void nc_server_set_content_id_clb(char *(*content_id_clb)(void *user_data), void *user_data, void (*free_user_data)(void *user_data)) { - /* HELLO LOCK */ - pthread_rwlock_wrlock(&server_opts.hello_lock); + /* HELLO LOCK, nothing to do on failure */ + nc_rwlock_lock(&server_opts.hello_lock, NC_RWLOCK_WRITE, NC_HELLO_LOCK_TIMEOUT, __func__); server_opts.content_id_clb = content_id_clb; server_opts.content_id_data = user_data; server_opts.content_id_data_free = free_user_data; /* HELLO UNLOCK */ - pthread_rwlock_unlock(&server_opts.hello_lock); + nc_rwlock_unlock(&server_opts.hello_lock, __func__); } API NC_MSG_TYPE @@ -1336,20 +1343,18 @@ nc_ps_queue_remove_id(struct nc_pollsession *ps, uint8_t id) int nc_ps_lock(struct nc_pollsession *ps, uint8_t *id, const char *func) { - int ret; + int r, rc = 0; struct timespec ts; /* LOCK */ - ret = pthread_mutex_lock(&ps->lock); - if (ret) { - ERR(NULL, "%s: failed to lock a pollsession (%s).", func, strerror(ret)); + if (nc_mutex_lock(&ps->lock, NC_PS_LOCK_TIMEOUT, func) != 1) { return -1; } /* check that the queue is long enough */ if (ps->queue_len == NC_PS_QUEUE_SIZE) { ERR(NULL, "%s: pollsession queue size (%d) too small.", func, NC_PS_QUEUE_SIZE); - pthread_mutex_unlock(&ps->lock); + nc_mutex_unlock(&ps->lock, func); return -1; } @@ -1362,49 +1367,45 @@ nc_ps_lock(struct nc_pollsession *ps, uint8_t *id, const char *func) while (ps->queue[ps->queue_begin] != *id) { nc_timeouttime_get(&ts, NC_PS_QUEUE_TIMEOUT); - ret = pthread_cond_clockwait(&ps->cond, &ps->lock, COMPAT_CLOCK_ID, &ts); - if (ret) { + r = pthread_cond_clockwait(&ps->cond, &ps->lock, COMPAT_CLOCK_ID, &ts); + if (r) { /** * This may happen when another thread releases the lock and broadcasts the condition * and this thread had already timed out. When this thread is scheduled, it returns timed out error * but when actually this thread was ready for condition. */ - if ((ETIMEDOUT == ret) && (ps->queue[ps->queue_begin] == *id)) { + if ((ETIMEDOUT == r) && (ps->queue[ps->queue_begin] == *id)) { break; } - ERR(NULL, "%s: failed to wait for a pollsession condition (%s).", func, strerror(ret)); + ERR(NULL, "%s: failed to wait for a pollsession condition (%s).", func, strerror(r)); /* remove ourselves from the queue */ nc_ps_queue_remove_id(ps, *id); - pthread_mutex_unlock(&ps->lock); - return -1; + rc = -1; + break; } } /* UNLOCK */ - pthread_mutex_unlock(&ps->lock); + nc_mutex_unlock(&ps->lock, func); - return 0; + return rc; } int nc_ps_unlock(struct nc_pollsession *ps, uint8_t id, const char *func) { - int ret; + int r; - /* LOCK */ - ret = pthread_mutex_lock(&ps->lock); - if (ret) { - ERR(NULL, "%s: failed to lock a pollsession (%s).", func, strerror(ret)); - ret = -1; - } + /* LOCK, continue on error */ + r = nc_mutex_lock(&ps->lock, NC_PS_LOCK_TIMEOUT, func); /* we must be the first, it was our turn after all, right? */ if (ps->queue[ps->queue_begin] != id) { ERRINT; /* UNLOCK */ - if (!ret) { - pthread_mutex_unlock(&ps->lock); + if (r == 1) { + nc_mutex_unlock(&ps->lock, func); } return -1; } @@ -1418,11 +1419,11 @@ nc_ps_unlock(struct nc_pollsession *ps, uint8_t id, const char *func) pthread_cond_broadcast(&ps->cond); /* UNLOCK */ - if (!ret) { - pthread_mutex_unlock(&ps->lock); + if (r == 1) { + nc_mutex_unlock(&ps->lock, func); } - return ret; + return r == 1 ? 0 : -1; } API struct nc_pollsession * @@ -2053,9 +2054,8 @@ nc_ps_poll_session_io(struct nc_session *session, int io_timeout, time_t now_mon return NC_PSPOLL_SESSION_TERM | NC_PSPOLL_SESSION_ERROR; } - r = nc_session_io_lock(session, io_timeout, __func__); + r = nc_mutex_lock(session->io_lock, io_timeout, __func__); if (r < 0) { - sprintf(msg, "Session IO lock failed to be acquired"); return NC_PSPOLL_ERROR; } else if (!r) { return NC_PSPOLL_TIMEOUT; @@ -2189,7 +2189,7 @@ nc_ps_poll_session_io(struct nc_session *session, int io_timeout, time_t now_mon break; } - nc_session_io_unlock(session, __func__); + nc_mutex_unlock(session->io_lock, __func__); return ret; } @@ -2218,12 +2218,13 @@ nc_ps_poll_sess(struct nc_ps_session *ps_session, time_t now_mono) ps_session->state = NC_PS_STATE_BUSY; /* CONFIG READ LOCK */ - pthread_rwlock_rdlock(&server_opts.config_lock); - - ret = nc_ps_poll_session_io(ps_session->session, NC_SESSION_LOCK_TIMEOUT, now_mono, msg); - - /* CONFIG UNLOCK */ - pthread_rwlock_unlock(&server_opts.config_lock); + if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_READ, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + ret = NC_PSPOLL_ERROR; + } else { + ret = nc_ps_poll_session_io(ps_session->session, NC_SESSION_LOCK_TIMEOUT, now_mono, msg); + /* CONFIG UNLOCK */ + nc_rwlock_unlock(&server_opts.config_lock, __func__); + } switch (ret) { case NC_PSPOLL_SESSION_TERM | NC_PSPOLL_SESSION_ERROR: @@ -2693,12 +2694,14 @@ nc_server_endpt_count(void) uint32_t cnt; /* CONFIG READ LOCK */ - pthread_rwlock_rdlock(&server_opts.config_lock); + if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_READ, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + return 0; + } cnt = LY_ARRAY_COUNT(server_opts.config.endpts); /* CONFIG UNLOCK */ - pthread_rwlock_unlock(&server_opts.config_lock); + nc_rwlock_unlock(&server_opts.config_lock, __func__); return cnt; } @@ -2724,7 +2727,9 @@ nc_accept(int timeout, const struct ly_ctx *ctx, struct nc_session **session) nc_server_init_cb_ctx(ctx); /* CONFIG LOCK */ - pthread_rwlock_rdlock(&server_opts.config_lock); + if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_READ, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + return NC_MSG_ERROR; + } config = &server_opts.config; @@ -2809,7 +2814,7 @@ nc_accept(int timeout, const struct ly_ctx *ctx, struct nc_session **session) (*session)->data = NULL; /* CONFIG UNLOCK */ - pthread_rwlock_unlock(&server_opts.config_lock); + nc_rwlock_unlock(&server_opts.config_lock, __func__); /* assign new SID atomically */ (*session)->id = ATOMIC_INC_RELAXED(server_opts.new_session_id); @@ -2832,7 +2837,7 @@ nc_accept(int timeout, const struct ly_ctx *ctx, struct nc_session **session) cleanup: /* CONFIG UNLOCK */ - pthread_rwlock_unlock(&server_opts.config_lock); + nc_rwlock_unlock(&server_opts.config_lock, __func__); free(host); if (sock > -1) { @@ -2856,7 +2861,9 @@ nc_server_ch_is_client(const char *name) } /* CONFIG READ LOCK */ - pthread_rwlock_rdlock(&server_opts.config_lock); + if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_READ, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + return found; + } /* check name against all configured clients */ LY_ARRAY_FOR(server_opts.config.ch_clients, struct nc_ch_client, client) { @@ -2867,7 +2874,7 @@ nc_server_ch_is_client(const char *name) } /* CONFIG READ UNLOCK */ - pthread_rwlock_unlock(&server_opts.config_lock); + nc_rwlock_unlock(&server_opts.config_lock, __func__); return found; } @@ -2884,7 +2891,9 @@ nc_server_ch_client_is_endpt(const char *client_name, const char *endpt_name) } /* CONFIG READ LOCK */ - pthread_rwlock_rdlock(&server_opts.config_lock); + if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_READ, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + return found; + } client = nc_server_ch_client_get(client_name); if (!client) { @@ -2900,7 +2909,7 @@ nc_server_ch_client_is_endpt(const char *client_name, const char *endpt_name) cleanup: /* CONFIG READ UNLOCK */ - pthread_rwlock_unlock(&server_opts.config_lock); + nc_rwlock_unlock(&server_opts.config_lock, __func__); return found; } @@ -3024,7 +3033,9 @@ nc_server_ch_client_get_idle_timeout(const char *client_name, uint32_t *idle_tim struct nc_ch_client *client; /* CONFIG READ LOCK */ - pthread_rwlock_rdlock(&server_opts.config_lock); + if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_READ, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + return 1; + } client = nc_server_ch_client_get(client_name); if (!client) { @@ -3040,7 +3051,7 @@ nc_server_ch_client_get_idle_timeout(const char *client_name, uint32_t *idle_tim cleanup: /* CONFIG READ UNLOCK */ - pthread_rwlock_unlock(&server_opts.config_lock); + nc_rwlock_unlock(&server_opts.config_lock, __func__); return ret; } @@ -3059,23 +3070,27 @@ nc_server_ch_client_thread_session_cond_wait(struct nc_server_ch_thread_arg *dat int rc = 0, r; uint32_t idle_timeout; struct timespec ts; - const char *client_name; /* CH LOCK */ - pthread_mutex_lock(&session->opts.server.ch_lock); - - /* save the client name - it can not be modified by any other thread */ - client_name = data->client_name; + if (nc_mutex_lock(&session->opts.server.ch_lock, NC_SESSION_CH_LOCK_TIMEOUT, __func__) != 1) { + return -1; + } session->flags |= NC_SESSION_CH_THREAD; + /* CH UNLOCK */ + nc_mutex_unlock(&session->opts.server.ch_lock, __func__); + /* give the session to the user */ if (data->new_session_cb(data->client_name, session, data->new_session_cb_data)) { + /* CH LOCK, continue on error */ + nc_mutex_lock(&session->opts.server.ch_lock, NC_SESSION_CH_LOCK_TIMEOUT, __func__); + /* something is wrong, free the session */ session->flags &= ~NC_SESSION_CH_THREAD; /* CH UNLOCK */ - pthread_mutex_unlock(&session->opts.server.ch_lock); + nc_mutex_unlock(&session->opts.server.ch_lock, __func__); /* session terminated, free it and release its context */ nc_session_free(session, NULL); @@ -3083,6 +3098,11 @@ nc_server_ch_client_thread_session_cond_wait(struct nc_server_ch_thread_arg *dat return 0; } + /* CH LOCK */ + if (nc_mutex_lock(&session->opts.server.ch_lock, NC_SESSION_CH_LOCK_TIMEOUT, __func__) != 1) { + return -1; + } + /* entering the loop with locked ch_lock */ do { nc_timeouttime_get(&ts, NC_CH_THREAD_IDLE_TIMEOUT_SLEEP); @@ -3101,27 +3121,31 @@ nc_server_ch_client_thread_session_cond_wait(struct nc_server_ch_thread_arg *dat } /* CH UNLOCK */ - pthread_mutex_unlock(&session->opts.server.ch_lock); + nc_mutex_unlock(&session->opts.server.ch_lock, __func__); /* check if the client still exists and get its idle timeout */ - r = nc_server_ch_client_get_idle_timeout(client_name, &idle_timeout); + r = nc_server_ch_client_get_idle_timeout(data->client_name, &idle_timeout); if (r) { /* client was removed, finish thread */ VRB(session, "Call Home client \"%s\" removed, but an established session will not be terminated.", - client_name); + data->client_name); rc = 1; /* CH LOCK - to remain consistent */ - pthread_mutex_lock(&session->opts.server.ch_lock); + if (nc_mutex_lock(&session->opts.server.ch_lock, NC_SESSION_CH_LOCK_TIMEOUT, __func__) != 1) { + return -1; + } break; } /* CH LOCK */ - pthread_mutex_lock(&session->opts.server.ch_lock); + if (nc_mutex_lock(&session->opts.server.ch_lock, NC_SESSION_CH_LOCK_TIMEOUT, __func__) != 1) { + return -1; + } nc_timeouttime_get(&ts, 0); if (!nc_session_get_notif_status(session) && idle_timeout && (ts.tv_sec >= session->opts.server.last_rpc + idle_timeout)) { - VRB(session, "Call Home client \"%s\": session idle timeout elapsed.", client_name); + VRB(session, "Call Home client \"%s\": session idle timeout elapsed.", data->client_name); session->status = NC_STATUS_INVALID; session->term_reason = NC_SESSION_TERM_TIMEOUT; } @@ -3133,7 +3157,7 @@ nc_server_ch_client_thread_session_cond_wait(struct nc_server_ch_thread_arg *dat pthread_cond_signal(&session->opts.server.ch_cond); /* CH UNLOCK */ - pthread_mutex_unlock(&session->opts.server.ch_lock); + nc_mutex_unlock(&session->opts.server.ch_lock, __func__); return rc; } @@ -3154,7 +3178,9 @@ nc_server_ch_client_thread_is_running_wait(struct nc_session *session, struct nc int ret = 0, thread_running; /* COND LOCK */ - pthread_mutex_lock(&data->cond_lock); + if (nc_mutex_lock(&data->cond_lock, NC_CH_COND_LOCK_TIMEOUT, __func__) != 1) { + return 0; + } /* get reconnect timeout in ms */ nc_timeouttime_get(&ts, cond_wait_time * 1000); while (!ret && data->thread_running) { @@ -3163,7 +3189,7 @@ nc_server_ch_client_thread_is_running_wait(struct nc_session *session, struct nc thread_running = data->thread_running; /* COND UNLOCK */ - pthread_mutex_unlock(&data->cond_lock); + nc_mutex_unlock(&data->cond_lock, __func__); if (!thread_running) { /* thread is terminating */ @@ -3197,13 +3223,15 @@ nc_server_ch_client_thread_is_running(struct nc_server_ch_thread_arg *data) int ret = -1; /* COND LOCK */ - pthread_mutex_lock(&data->cond_lock); + if (nc_mutex_lock(&data->cond_lock, NC_CH_COND_LOCK_TIMEOUT, __func__) != 1) { + return ret; + } if (!data->thread_running) { /* thread should stop running */ ret = 0; } /* COND UNLOCK */ - pthread_mutex_unlock(&data->cond_lock); + nc_mutex_unlock(&data->cond_lock, __func__); return ret; } @@ -3235,13 +3263,15 @@ nc_server_ch_client_with_endpt_get(struct nc_server_ch_thread_arg *data, const c } /* CONFIG READ UNLOCK - allow another thread to modify the configuration */ - pthread_rwlock_unlock(&server_opts.config_lock); + nc_rwlock_unlock(&server_opts.config_lock, __func__); /* no endpoints defined yet, wait a little bit */ usleep(NC_CH_NO_ENDPT_WAIT * 1000); /* CONFIG READ LOCK */ - pthread_rwlock_rdlock(&server_opts.config_lock); + if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_READ, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + return NULL; + } } /* thread is not running */ @@ -3269,12 +3299,16 @@ nc_ch_client_thread(void *arg) LY_ARRAY_COUNT_TYPE i; /* mark the thread as running */ - pthread_mutex_lock(&data->cond_lock); + if (nc_mutex_lock(&data->cond_lock, NC_CH_COND_LOCK_TIMEOUT, __func__) != 1) { + goto cleanup; + } data->thread_running = 1; - pthread_mutex_unlock(&data->cond_lock); + nc_mutex_unlock(&data->cond_lock, __func__); /* CONFIG READ LOCK */ - pthread_rwlock_rdlock(&server_opts.config_lock); + if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_READ, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + goto cleanup; + } /* get the client once it has at least one endpoint */ client = nc_server_ch_client_with_endpt_get(data, data->client_name); @@ -3296,7 +3330,7 @@ nc_ch_client_thread(void *arg) msgtype = nc_connect_ch_endpt(cur_endpt, data->acquire_ctx_cb, data->release_ctx_cb, data->ctx_cb_data, &session); if (msgtype == NC_MSG_HELLO) { /* CONFIG READ UNLOCK - session established */ - pthread_rwlock_unlock(&server_opts.config_lock); + nc_rwlock_unlock(&server_opts.config_lock, __func__); if (!nc_server_ch_client_thread_is_running(data)) { /* thread should stop running */ @@ -3317,7 +3351,9 @@ nc_ch_client_thread(void *arg) } /* CONFIG READ LOCK */ - pthread_rwlock_rdlock(&server_opts.config_lock); + if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_READ, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + goto cleanup; + } /* get the client again, it may have been removed */ client = nc_server_ch_client_with_endpt_get(data, data->client_name); @@ -3338,7 +3374,7 @@ nc_ch_client_thread(void *arg) } /* CONFIG READ UNLOCK */ - pthread_rwlock_unlock(&server_opts.config_lock); + nc_rwlock_unlock(&server_opts.config_lock, __func__); /* wait for the timeout to elapse, so we can try to reconnect */ VRB(session, "Call Home client \"%s\" reconnecting in %" PRIu32 " seconds.", data->client_name, reconnect_in); @@ -3347,7 +3383,9 @@ nc_ch_client_thread(void *arg) } /* CONFIG READ LOCK */ - pthread_rwlock_rdlock(&server_opts.config_lock); + if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_READ, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + goto cleanup; + } client = nc_server_ch_client_with_endpt_get(data, data->client_name); if (!client) { @@ -3379,7 +3417,7 @@ nc_ch_client_thread(void *arg) max_wait = client->max_wait; /* CONFIG READ UNLOCK */ - pthread_rwlock_unlock(&server_opts.config_lock); + nc_rwlock_unlock(&server_opts.config_lock, __func__); /* wait for max_wait seconds */ if (!nc_server_ch_client_thread_is_running_wait(session, data, max_wait)) { @@ -3388,7 +3426,9 @@ nc_ch_client_thread(void *arg) } /* CONFIG READ LOCK */ - pthread_rwlock_rdlock(&server_opts.config_lock); + if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_READ, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + goto cleanup; + } /* get the client */ client = nc_server_ch_client_with_endpt_get(data, data->client_name); @@ -3441,14 +3481,16 @@ nc_ch_client_thread(void *arg) cleanup_unlock: /* CONFIG READ UNLOCK */ - pthread_rwlock_unlock(&server_opts.config_lock); + nc_rwlock_unlock(&server_opts.config_lock, __func__); cleanup: VRB(session, "Call Home client \"%s\" thread exit.", data->client_name); free(cur_endpt_name); /* CH THREADS WR LOCK */ - pthread_rwlock_wrlock(&server_opts.ch_threads_lock); + if (nc_rwlock_lock(&server_opts.ch_threads_lock, NC_RWLOCK_WRITE, NC_CH_THREADS_LOCK_TIMEOUT, __func__) != 1) { + goto unlock_skip; + } /* remove the thread data from the global array */ LY_ARRAY_FOR(server_opts.ch_threads, i) { @@ -3462,12 +3504,16 @@ nc_ch_client_thread(void *arg) LY_ARRAY_DECREMENT_FREE(server_opts.ch_threads); /* CH THREADS UNLOCK */ - pthread_rwlock_unlock(&server_opts.ch_threads_lock); + nc_rwlock_unlock(&server_opts.ch_threads_lock, __func__); +unlock_skip: free(data->client_name); - pthread_mutex_lock(&data->cond_lock); - pthread_cond_destroy(&data->cond); - pthread_mutex_unlock(&data->cond_lock); + if (nc_mutex_lock(&data->cond_lock, NC_CH_COND_LOCK_TIMEOUT, __func__) != 1) { + pthread_cond_destroy(&data->cond); + nc_mutex_unlock(&data->cond_lock, __func__); + } else { + pthread_cond_destroy(&data->cond); + } pthread_mutex_destroy(&data->cond_lock); free(data); return NULL; @@ -3478,7 +3524,8 @@ _nc_connect_ch_client_dispatch(const struct nc_ch_client *ch_client, nc_server_c nc_server_ch_session_release_ctx_cb release_ctx_cb, void *ctx_cb_data, nc_server_ch_new_session_cb new_session_cb, void *new_session_cb_data) { - int rc = 0, r, ch_threads_locked = 0; + int rc = 0, r; + enum nc_rwlock_mode ch_threads_lock = NC_RWLOCK_NONE; pthread_t tid; struct nc_server_ch_thread_arg *arg = NULL, **new_item; pthread_attr_t attr; @@ -3510,8 +3557,11 @@ _nc_connect_ch_client_dispatch(const struct nc_ch_client *ch_client, nc_server_c pthread_mutex_init(&arg->cond_lock, NULL); /* CH THREADS WR LOCK */ - pthread_rwlock_wrlock(&server_opts.ch_threads_lock); - ch_threads_locked = 1; + if (nc_rwlock_lock(&server_opts.ch_threads_lock, NC_RWLOCK_WRITE, NC_CH_THREADS_LOCK_TIMEOUT, __func__) != 1) { + rc = -1; + goto cleanup; + } + ch_threads_lock = NC_RWLOCK_WRITE; /* Append the thread data pointer to the global array. * Pointer instead of struct so that server_opts.ch_thread_lock does not have to be @@ -3520,8 +3570,8 @@ _nc_connect_ch_client_dispatch(const struct nc_ch_client *ch_client, nc_server_c *new_item = arg; /* CH THREADS UNLOCK */ - pthread_rwlock_unlock(&server_opts.ch_threads_lock); - ch_threads_locked = 0; + nc_rwlock_unlock(&server_opts.ch_threads_lock, __func__); + ch_threads_lock = NC_RWLOCK_NONE; /* create the CH thread */ if ((r = pthread_create(&tid, &attr, nc_ch_client_thread, arg))) { @@ -3534,9 +3584,9 @@ _nc_connect_ch_client_dispatch(const struct nc_ch_client *ch_client, nc_server_c arg = NULL; cleanup: - if (ch_threads_locked) { + if (ch_threads_lock) { /* CH THREADS UNLOCK */ - pthread_rwlock_unlock(&server_opts.ch_threads_lock); + nc_rwlock_unlock(&server_opts.ch_threads_lock, __func__); } pthread_attr_destroy(&attr); if (arg) { @@ -3559,7 +3609,9 @@ nc_connect_ch_client_dispatch(const char *client_name, nc_server_ch_session_acqu NC_CHECK_SRV_INIT_RET(-1); /* CONFIG READ LOCK */ - pthread_rwlock_rdlock(&server_opts.config_lock); + if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_READ, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + return -1; + } /* check ch client existence */ ch_client = nc_server_ch_client_get(client_name); @@ -3570,7 +3622,7 @@ nc_connect_ch_client_dispatch(const char *client_name, nc_server_ch_session_acqu cleanup: /* CONFIG READ UNLOCK */ - pthread_rwlock_unlock(&server_opts.config_lock); + nc_rwlock_unlock(&server_opts.config_lock, __func__); return rc; } @@ -3599,13 +3651,13 @@ nc_session_inc_notif_status(struct nc_session *session) return; } - /* NTF STATUS LOCK */ - pthread_mutex_lock(&session->opts.server.ntf_status_lock); + /* NTF STATUS LOCK, continue on error */ + nc_mutex_lock(&session->opts.server.ntf_status_lock, NC_SESSION_NTF_STATUS_LOCK_TIMEOUT, __func__); ++session->opts.server.ntf_status; /* NTF STATUS UNLOCK */ - pthread_mutex_unlock(&session->opts.server.ntf_status_lock); + nc_mutex_unlock(&session->opts.server.ntf_status_lock, __func__); } API void @@ -3616,15 +3668,15 @@ nc_session_dec_notif_status(struct nc_session *session) return; } - /* NTF STATUS LOCK */ - pthread_mutex_lock(&session->opts.server.ntf_status_lock); + /* NTF STATUS LOCK, continue on error */ + nc_mutex_lock(&session->opts.server.ntf_status_lock, NC_SESSION_NTF_STATUS_LOCK_TIMEOUT, __func__); if (session->opts.server.ntf_status) { --session->opts.server.ntf_status; } /* NTF STATUS UNLOCK */ - pthread_mutex_unlock(&session->opts.server.ntf_status_lock); + nc_mutex_unlock(&session->opts.server.ntf_status_lock, __func__); } API int @@ -3638,12 +3690,15 @@ nc_session_get_notif_status(const struct nc_session *session) } /* NTF STATUS LOCK */ - pthread_mutex_lock(&((struct nc_session *)session)->opts.server.ntf_status_lock); + if (nc_mutex_lock(&((struct nc_session *)session)->opts.server.ntf_status_lock, NC_SESSION_NTF_STATUS_LOCK_TIMEOUT, + __func__) != 1) { + return 0; + } ntf_status = session->opts.server.ntf_status; /* NTF STATUS UNLOCK */ - pthread_mutex_unlock(&((struct nc_session *)session)->opts.server.ntf_status_lock); + nc_mutex_unlock(&((struct nc_session *)session)->opts.server.ntf_status_lock, __func__); return ntf_status; } @@ -4055,7 +4110,9 @@ nc_server_notif_cert_exp_dates_get(struct nc_cert_exp_time_interval *intervals, *exp_date_count = 0; /* CONFIG READ LOCK */ - pthread_rwlock_rdlock(&server_opts.config_lock); + if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_READ, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + return 1; + } /* first go through listen certs */ LY_ARRAY_FOR(server_opts.config.endpts, struct nc_endpt, endpt) { @@ -4105,7 +4162,7 @@ nc_server_notif_cert_exp_dates_get(struct nc_cert_exp_time_interval *intervals, cleanup: /* CONFIG READ UNLOCK */ - pthread_rwlock_unlock(&server_opts.config_lock); + nc_rwlock_unlock(&server_opts.config_lock, __func__); return ret; } @@ -4186,14 +4243,16 @@ nc_server_notif_cert_exp_thread_is_running() int ret = 0; /* LOCK */ - pthread_mutex_lock(&server_opts.cert_exp_notif.lock); + if (nc_mutex_lock(&server_opts.cert_exp_notif.lock, NC_CERT_EXP_LOCK_TIMEOUT, __func__) != 1) { + return 0; + } if (server_opts.cert_exp_notif.thread_running) { ret = 1; } /* UNLOCK */ - pthread_mutex_unlock(&server_opts.cert_exp_notif.lock); + nc_mutex_unlock(&server_opts.cert_exp_notif.lock, __func__); return ret; } @@ -4216,7 +4275,9 @@ nc_server_notif_cert_exp_intervals_get(struct nc_cert_exp_time_interval *default int rc = 0; /* CONFIG LOCK */ - pthread_rwlock_rdlock(&server_opts.config_lock); + if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_READ, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + return 1; + } if (!server_opts.config.cert_exp_notif_intervals) { /* dup the default intervals */ @@ -4235,7 +4296,7 @@ nc_server_notif_cert_exp_intervals_get(struct nc_cert_exp_time_interval *default cleanup: /* CONFIG UNLOCK */ - pthread_rwlock_unlock(&server_opts.config_lock); + nc_rwlock_unlock(&server_opts.config_lock, __func__); return rc; } @@ -4280,10 +4341,12 @@ nc_server_notif_cert_exp_thread(void *arg) wakeup_time.tv_sec = nc_server_notif_cert_exp_wakeup_time_get(exp_dates, exp_date_count, &curr_cert); /* sleep until the next notification time or until the thread is woken up */ - pthread_mutex_lock(&server_opts.cert_exp_notif.lock); + if (nc_mutex_lock(&server_opts.cert_exp_notif.lock, NC_CERT_EXP_LOCK_TIMEOUT, __func__) != 1) { + break; + } r = pthread_cond_clockwait(&server_opts.cert_exp_notif.cond, &server_opts.cert_exp_notif.lock, CLOCK_REALTIME, &wakeup_time); - pthread_mutex_unlock(&server_opts.cert_exp_notif.lock); + nc_mutex_unlock(&server_opts.cert_exp_notif.lock, __func__); if (!r) { /* we were woken up */ @@ -4357,7 +4420,10 @@ nc_server_notif_cert_expiration_thread_start(nc_cert_exp_notif_clb cert_exp_noti arg->clb_free_data = free_data; /* LOCK */ - pthread_mutex_lock(&server_opts.cert_exp_notif.lock); + if (nc_mutex_lock(&server_opts.cert_exp_notif.lock, NC_CERT_EXP_LOCK_TIMEOUT, __func__) != 1) { + ret = 1; + goto cleanup; + } /* check if the thread is already running */ if (server_opts.cert_exp_notif.thread_running) { @@ -4378,7 +4444,7 @@ nc_server_notif_cert_expiration_thread_start(nc_cert_exp_notif_clb cert_exp_noti cleanup: /* UNLOCK */ - pthread_mutex_unlock(&server_opts.cert_exp_notif.lock); + nc_mutex_unlock(&server_opts.cert_exp_notif.lock, __func__); if (ret) { free(arg); } @@ -4392,7 +4458,9 @@ nc_server_notif_cert_expiration_thread_stop(int wait) pthread_t tid; /* LOCK */ - pthread_mutex_lock(&server_opts.cert_exp_notif.lock); + if (nc_mutex_lock(&server_opts.cert_exp_notif.lock, NC_CERT_EXP_LOCK_TIMEOUT, __func__) != 1) { + return; + } tid = server_opts.cert_exp_notif.tid; if (server_opts.cert_exp_notif.thread_running) { @@ -4402,7 +4470,7 @@ nc_server_notif_cert_expiration_thread_stop(int wait) pthread_cond_signal(&server_opts.cert_exp_notif.cond); /* UNLOCK */ - pthread_mutex_unlock(&server_opts.cert_exp_notif.lock); + nc_mutex_unlock(&server_opts.cert_exp_notif.lock, __func__); if (wait) { r = pthread_join(tid, NULL); } else { @@ -4414,7 +4482,7 @@ nc_server_notif_cert_expiration_thread_stop(int wait) } else { /* thread is not running */ /* UNLOCK */ - pthread_mutex_unlock(&server_opts.cert_exp_notif.lock); + nc_mutex_unlock(&server_opts.cert_exp_notif.lock, __func__); } } @@ -4427,7 +4495,9 @@ nc_server_is_mod_ignored(const char *mod_name) LY_ARRAY_COUNT_TYPE i; /* LOCK */ - pthread_rwlock_rdlock(&server_opts.config_lock); + if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_READ, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + return 0; + } LY_ARRAY_FOR(server_opts.config.ignored_modules, i) { if (!strcmp(server_opts.config.ignored_modules[i], mod_name)) { @@ -4437,7 +4507,7 @@ nc_server_is_mod_ignored(const char *mod_name) } /* UNLOCK */ - pthread_rwlock_unlock(&server_opts.config_lock); + nc_rwlock_unlock(&server_opts.config_lock, __func__); return ignored; } @@ -4452,7 +4522,9 @@ nc_server_set_unix_socket_path(const char *endpoint_name, const char *socket_pat NC_CHECK_ARG_RET(NULL, endpoint_name, socket_path, 1); /* CONFIG WRITE LOCK */ - pthread_rwlock_wrlock(&server_opts.config_lock); + if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_WRITE, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + return 1; + } /* try to see if the path for this endpoint already exists */ LY_ARRAY_FOR(server_opts.unix_paths, i) { @@ -4476,7 +4548,7 @@ nc_server_set_unix_socket_path(const char *endpoint_name, const char *socket_pat cleanup: /* CONFIG WRITE UNLOCK */ - pthread_rwlock_unlock(&server_opts.config_lock); + nc_rwlock_unlock(&server_opts.config_lock, __func__); return rc; } @@ -4492,7 +4564,9 @@ nc_server_get_unix_socket_path(const char *endpoint_name, char **socket_path) *socket_path = NULL; /* CONFIG READ LOCK */ - pthread_rwlock_rdlock(&server_opts.config_lock); + if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_READ, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + return 1; + } /* try to find the path for this endpoint */ LY_ARRAY_FOR(server_opts.unix_paths, i) { @@ -4511,7 +4585,7 @@ nc_server_get_unix_socket_path(const char *endpoint_name, char **socket_path) cleanup: /* CONFIG READ UNLOCK */ - pthread_rwlock_unlock(&server_opts.config_lock); + nc_rwlock_unlock(&server_opts.config_lock, __func__); return rc; } @@ -4521,7 +4595,9 @@ nc_server_set_unix_socket_dir(const char *dir) int rc = 0; /* CONFIG WRITE LOCK */ - pthread_rwlock_wrlock(&server_opts.config_lock); + if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_WRITE, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + return 1; + } free(server_opts.unix_socket_dir); server_opts.unix_socket_dir = strdup(dir); @@ -4529,7 +4605,7 @@ nc_server_set_unix_socket_dir(const char *dir) cleanup: /* CONFIG WRITE UNLOCK */ - pthread_rwlock_unlock(&server_opts.config_lock); + nc_rwlock_unlock(&server_opts.config_lock, __func__); return rc; } @@ -4541,7 +4617,9 @@ nc_server_get_unix_socket_dir(char **dir) *dir = NULL; /* CONFIG READ LOCK */ - pthread_rwlock_rdlock(&server_opts.config_lock); + if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_READ, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + return 1; + } if (server_opts.unix_socket_dir) { *dir = strdup(server_opts.unix_socket_dir); @@ -4550,6 +4628,6 @@ nc_server_get_unix_socket_dir(char **dir) cleanup: /* CONFIG READ UNLOCK */ - pthread_rwlock_unlock(&server_opts.config_lock); + nc_rwlock_unlock(&server_opts.config_lock, __func__); return rc; } diff --git a/src/session_server.h b/src/session_server.h index 18f925f2..59b2483d 100644 --- a/src/session_server.h +++ b/src/session_server.h @@ -199,6 +199,7 @@ void nc_server_get_capab_withdefaults(NC_WD_MODE *basic_mode, int *also_supporte * string. * * @param[in] value Capability string to be advertised in server's \ messages. + * @return 0 on success, -1 on error. */ int nc_server_set_capability(const char *value); diff --git a/src/session_server_ssh.c b/src/session_server_ssh.c index baed6e38..dda826aa 100644 --- a/src/session_server_ssh.c +++ b/src/session_server_ssh.c @@ -1010,14 +1010,16 @@ nc_server_ssh_set_interactive_auth_clb(int (*interactive_auth_clb)(const struct void *user_data, void (*free_user_data)(void *user_data)) { /* CONFIG LOCK */ - pthread_rwlock_wrlock(&server_opts.config_lock); + if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_WRITE, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + return; + } server_opts.interactive_auth_clb = interactive_auth_clb; server_opts.interactive_auth_data = user_data; server_opts.interactive_auth_data_free = free_user_data; /* CONFIG UNLOCK */ - pthread_rwlock_unlock(&server_opts.config_lock); + nc_rwlock_unlock(&server_opts.config_lock, __func__); } #ifdef HAVE_LIBPAM @@ -1030,7 +1032,9 @@ nc_server_ssh_set_pam_conf_filename(const char *filename) NC_CHECK_ARG_RET(NULL, filename, 1); /* CONFIG LOCK */ - pthread_rwlock_wrlock(&server_opts.config_lock); + if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_WRITE, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + return 1; + } free(server_opts.pam_config_name); server_opts.pam_config_name = strdup(filename); @@ -1040,7 +1044,7 @@ nc_server_ssh_set_pam_conf_filename(const char *filename) } /* CONFIG UNLOCK */ - pthread_rwlock_unlock(&server_opts.config_lock); + nc_rwlock_unlock(&server_opts.config_lock, __func__); return ret; } @@ -1064,7 +1068,9 @@ nc_server_ssh_set_authkey_path_format(const char *path) NC_CHECK_ARG_RET(NULL, path, 1); /* CONFIG LOCK */ - pthread_rwlock_wrlock(&server_opts.config_lock); + if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_WRITE, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + return 1; + } free(server_opts.authkey_path_fmt); server_opts.authkey_path_fmt = strdup(path); @@ -1074,7 +1080,7 @@ nc_server_ssh_set_authkey_path_format(const char *path) } /* CONFIG UNLOCK */ - pthread_rwlock_unlock(&server_opts.config_lock); + nc_rwlock_unlock(&server_opts.config_lock, __func__); return ret; } diff --git a/src/session_server_tls.c b/src/session_server_tls.c index b51d8316..df2f1551 100644 --- a/src/session_server_tls.c +++ b/src/session_server_tls.c @@ -662,12 +662,14 @@ API void nc_server_tls_set_verify_clb(int (*verify_clb)(const struct nc_session *session)) { /* CONFIG LOCK */ - pthread_rwlock_wrlock(&server_opts.config_lock); + if (nc_rwlock_lock(&server_opts.config_lock, NC_RWLOCK_WRITE, NC_CONFIG_LOCK_TIMEOUT, __func__) != 1) { + return; + } server_opts.user_verify_clb = verify_clb; /* CONFIG UNLOCK */ - pthread_rwlock_unlock(&server_opts.config_lock); + nc_rwlock_unlock(&server_opts.config_lock, __func__); } int