Skip to content

Commit

Permalink
session client BUGFIX make mon thread arg static
Browse files Browse the repository at this point in the history
  • Loading branch information
roman committed Oct 23, 2024
1 parent 40c41b3 commit 51faf71
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 85 deletions.
116 changes: 58 additions & 58 deletions src/session_client.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ static struct nc_client_context context_main = {
.max_probes = 10,
.probe_interval = 5
},
.opts.monitoring_thread_data.lock = PTHREAD_MUTEX_INITIALIZER,
#ifdef NC_ENABLED_SSH_TLS
.ssh_opts = {
.auth_pref = {{NC_SSH_AUTH_INTERACTIVE, 1}, {NC_SSH_AUTH_PASSWORD, 2}, {NC_SSH_AUTH_PUBLICKEY, 3}},
Expand Down Expand Up @@ -192,6 +193,10 @@ nc_client_context_location(void)
e->ssh_ch_opts.auth_pref[2].value = 3;
#endif /* NC_ENABLED_SSH_TLS */
}

/* init the monitoring thread data lock */
pthread_mutex_init(&e->opts.monitoring_thread_data.lock, NULL);

pthread_setspecific(nc_client_context_key, e);
}

Expand Down Expand Up @@ -1460,11 +1465,9 @@ nc_connect_inout(int fdin, int fdout, struct ly_ctx *ctx)
goto fail;
}

/* start monitoring the session */
if (client_opts.monitoring_thread_data) {
if (nc_client_monitoring_session_start(session)) {
goto fail;
}
/* start monitoring the session if the monitoring thread is running */
if (nc_client_monitoring_session_start(session)) {
goto fail;
}

return session;
Expand Down Expand Up @@ -1544,11 +1547,9 @@ nc_connect_unix(const char *address, struct ly_ctx *ctx)
goto fail;
}

/* start monitoring the session */
if (client_opts.monitoring_thread_data) {
if (nc_client_monitoring_session_start(session)) {
goto fail;
}
/* start monitoring the session if the monitoring thread is running */
if (nc_client_monitoring_session_start(session)) {
goto fail;
}

return session;
Expand Down Expand Up @@ -3278,22 +3279,24 @@ nc_client_monitoring_session_start(struct nc_session *session)
struct nc_client_monitoring_thread_arg *mtarg;
void *tmp, *tmp2;

mtarg = client_opts.monitoring_thread_data;
if (!mtarg) {
ERRINT;
return 1;
/* LOCK */
pthread_mutex_lock(&client_opts.monitoring_thread_data.lock);

/* check if the monitoring thread is running */
if (!client_opts.monitoring_thread_data.thread_running) {
goto cleanup;
}

mtarg = &client_opts.monitoring_thread_data;

/* get the session's file descriptor */
fd = nc_client_monitoring_get_session_fd(session);
if (fd == -1) {
ERR(session, "Unable to monitor an invalid file descriptor.");
return 1;
ret = 1;
goto cleanup;
}

/* LOCK */
pthread_mutex_lock(&mtarg->lock);

/* if realloc fails, keep the original sessions without adding the new one */
tmp = realloc(mtarg->sessions, (mtarg->session_count + 1) * sizeof *mtarg->sessions);
NC_CHECK_ERRMEM_GOTO(!tmp, ret = 1, cleanup);
Expand Down Expand Up @@ -3325,17 +3328,13 @@ nc_client_monitoring_session_stop(struct nc_session *session, int lock)
int i;
struct nc_client_monitoring_thread_arg *mtarg;

mtarg = client_opts.monitoring_thread_data;
if (!mtarg) {
ERRINT;
return;
}

if (lock) {
/* LOCK */
pthread_mutex_lock(&mtarg->lock);
pthread_mutex_lock(&client_opts.monitoring_thread_data.lock);
}

mtarg = &client_opts.monitoring_thread_data;

/* session is no longer monitored (is being freed) */
if (!(session->flags & NC_SESSION_CLIENT_MONITORED)) {
goto cleanup;
Expand Down Expand Up @@ -3392,15 +3391,10 @@ nc_client_monitoring_thread(void *arg)
/* set this thread's context to the one from the thread that started the monitoring thread */
nc_client_set_thread_context(arg);

/* so that both threads share the same opts */
mtarg = client_opts.monitoring_thread_data;
if (!mtarg) {
ERRINT;
return NULL;
}

/* LOCK */
pthread_mutex_lock(&mtarg->lock);
pthread_mutex_lock(&client_opts.monitoring_thread_data.lock);

mtarg = &client_opts.monitoring_thread_data;

while (mtarg->thread_running) {
if (!mtarg->session_count) {
Expand Down Expand Up @@ -3463,19 +3457,17 @@ nc_client_monitoring_thread_start(nc_client_monitoring_clb monitoring_clb, void

NC_CHECK_ARG_RET(NULL, monitoring_clb, 1);

if (client_opts.monitoring_thread_data) {
/* LOCK */
pthread_mutex_lock(&client_opts.monitoring_thread_data.lock);
if (client_opts.monitoring_thread_data.thread_running) {
ERR(NULL, "Client monitoring thread is already running.");
return 1;
ret = 1;
goto cleanup;
}

client_opts.monitoring_thread_data = calloc(1, sizeof *client_opts.monitoring_thread_data);
NC_CHECK_ERRMEM_RET(!client_opts.monitoring_thread_data, 1);

client_opts.monitoring_thread_data->clb = monitoring_clb;
client_opts.monitoring_thread_data->clb_data = user_data;
client_opts.monitoring_thread_data->clb_free_data = free_data;

pthread_mutex_init(&client_opts.monitoring_thread_data->lock, NULL);
client_opts.monitoring_thread_data.clb = monitoring_clb;
client_opts.monitoring_thread_data.clb_data = user_data;

Check warning

Code scanning / CodeQL

Local variable address stored in non-local memory Warning

A stack address which arrived via a
parameter
Loading
may be assigned to a non-local variable.
client_opts.monitoring_thread_data.clb_free_data = free_data;

/* get the current thread context, so that the monitoring thread can use it */
ctx = nc_client_get_thread_context();
Expand All @@ -3485,9 +3477,9 @@ nc_client_monitoring_thread_start(nc_client_monitoring_clb monitoring_clb, void
goto cleanup;
}

client_opts.monitoring_thread_data->thread_running = 1;
client_opts.monitoring_thread_data.thread_running = 1;

r = pthread_create(&client_opts.monitoring_thread_data->tid, NULL,
r = pthread_create(&client_opts.monitoring_thread_data.tid, NULL,
nc_client_monitoring_thread, ctx);
if (r) {
ERR(NULL, "Failed to create the client monitoring thread (%s).", strerror(r));
Expand All @@ -3496,11 +3488,8 @@ nc_client_monitoring_thread_start(nc_client_monitoring_clb monitoring_clb, void
}

cleanup:
if (ret) {
pthread_mutex_destroy(&client_opts.monitoring_thread_data->lock);
free(client_opts.monitoring_thread_data);
}

/* UNLOCK */
pthread_mutex_unlock(&client_opts.monitoring_thread_data.lock);
return ret;
}

Expand All @@ -3511,19 +3500,20 @@ nc_client_monitoring_thread_stop(void)
pthread_t tid;
struct nc_client_monitoring_thread_arg *mtarg;

if (!client_opts.monitoring_thread_data) {
/* LOCK */
pthread_mutex_lock(&client_opts.monitoring_thread_data.lock);
if (!client_opts.monitoring_thread_data.thread_running) {
ERR(NULL, "Client monitoring thread is not running.");

/* UNLOCK */
pthread_mutex_unlock(&client_opts.monitoring_thread_data.lock);
return;
}

mtarg = client_opts.monitoring_thread_data;

/* LOCK */
pthread_mutex_lock(&mtarg->lock);
mtarg = &client_opts.monitoring_thread_data;

mtarg->thread_running = 0;
tid = mtarg->tid;
client_opts.monitoring_thread_data = NULL;

/* remove the flag from the session while the lock is held */
for (i = 0; i < mtarg->session_count; i++) {
Expand All @@ -3538,14 +3528,24 @@ nc_client_monitoring_thread_stop(void)
ERR(NULL, "Failed to join the client monitoring thread (%s).", strerror(r));
}

/* LOCK */
pthread_mutex_lock(&mtarg->lock);

if (mtarg->clb_free_data) {
mtarg->clb_free_data(mtarg->clb_data);
}
mtarg->clb = NULL;
mtarg->clb_data = NULL;
mtarg->clb_free_data = NULL;

free(mtarg->sessions);
mtarg->sessions = NULL;

free(mtarg->pfds);
pthread_mutex_destroy(&mtarg->lock);
mtarg->pfds = NULL;

free(mtarg);
/* UNLOCK */
pthread_mutex_unlock(&mtarg->lock);
}

API void
Expand Down
2 changes: 1 addition & 1 deletion src/session_client.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -630,7 +630,7 @@ typedef void (*nc_client_monitoring_clb)(struct nc_session *session, void *user_
/**
* @brief Start a thread that monitors client sessions.
*
* If the thread is running, new sessions will be monitored automatically.
* Once the thread is running, new sessions will be monitored automatically.
*
* Note that once you start the monitoring thread, any other client thread that
* calls ::nc_session_free() needs to share the same thread context (or be the same thread)
Expand Down
24 changes: 9 additions & 15 deletions src/session_client_ssh.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1742,11 +1742,9 @@ _nc_connect_libssh(ssh_session ssh_session, struct ly_ctx *ctx, struct nc_keepal
goto fail;
}

/* start monitoring the session */
if (client_opts.monitoring_thread_data) {
if (nc_client_monitoring_session_start(session)) {
goto fail;
}
/* start monitoring the session if the monitoring thread is running */
if (nc_client_monitoring_session_start(session)) {
goto fail;
}

/* store information if not previously */
Expand Down Expand Up @@ -1855,11 +1853,9 @@ nc_connect_ssh(const char *host, uint16_t port, struct ly_ctx *ctx)
goto fail;
}

/* start monitoring the session */
if (client_opts.monitoring_thread_data) {
if (nc_client_monitoring_session_start(session)) {
goto fail;
}
/* start monitoring the session if the monitoring thread is running */
if (nc_client_monitoring_session_start(session)) {
goto fail;
}

/* update information */
Expand Down Expand Up @@ -1934,11 +1930,9 @@ nc_connect_ssh_channel(struct nc_session *session, struct ly_ctx *ctx)
goto fail;
}

/* start monitoring the session */
if (client_opts.monitoring_thread_data) {
if (nc_client_monitoring_session_start(session)) {
goto fail;
}
/* start monitoring the session if the monitoring thread is running */
if (nc_client_monitoring_session_start(session)) {
goto fail;
}

/* store information into session */
Expand Down
16 changes: 6 additions & 10 deletions src/session_client_tls.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -421,11 +421,9 @@ nc_connect_tls(const char *host, unsigned short port, struct ly_ctx *ctx)
goto fail;
}

/* start monitoring the session */
if (client_opts.monitoring_thread_data) {
if (nc_client_monitoring_session_start(session)) {
goto fail;
}
/* start monitoring the session if the monitoring thread is running */
if (nc_client_monitoring_session_start(session)) {
goto fail;
}

/* store information into session */
Expand Down Expand Up @@ -488,11 +486,9 @@ nc_accept_callhome_tls_sock(int sock, const char *host, uint16_t port, struct ly

session->flags |= NC_SESSION_CALLHOME;

/* start monitoring the session */
if (client_opts.monitoring_thread_data) {
if (nc_client_monitoring_session_start(session)) {
goto fail;
}
/* start monitoring the session if the monitoring thread is running */
if (nc_client_monitoring_session_start(session)) {
goto fail;
}

/* store information into session */
Expand Down
2 changes: 1 addition & 1 deletion src/session_p.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ struct nc_client_opts {
} *ch_binds_aux;
uint16_t ch_bind_count;

struct nc_client_monitoring_thread_arg *monitoring_thread_data; /**< Data of the monitoring thread. */
struct nc_client_monitoring_thread_arg monitoring_thread_data; /**< Data of the monitoring thread. */
};

/* ACCESS unlocked */
Expand Down

0 comments on commit 51faf71

Please sign in to comment.