server session BUGFIX lock fixes and optimizations
diff --git a/src/session_server.c b/src/session_server.c
index e8fac6d..08919a4 100644
--- a/src/session_server.c
+++ b/src/session_server.c
@@ -570,12 +570,12 @@
nc_ctx_lock(-1, NULL);
(*rpc)->tree = lyd_parse_xml(server_opts.ctx, &xml->child, LYD_OPT_DESTRUCT | LYD_OPT_RPC);
+ nc_ctx_unlock();
+
if (!(*rpc)->tree) {
ERR("Session %u: received message failed to be parsed into a known RPC.", session->id);
msgtype = NC_MSG_NONE;
}
- nc_ctx_unlock();
-
(*rpc)->root = xml;
break;
case NC_MSG_HELLO:
@@ -878,30 +878,26 @@
return -1;
}
- /* READ LOCK */
- pthread_rwlock_rdlock(&server_opts.endpt_array_lock);
+ /* WRITE LOCK */
+ pthread_rwlock_wrlock(&server_opts.endpt_array_lock);
/* check name uniqueness */
for (i = 0; i < server_opts.endpt_count; ++i) {
if (!strcmp(server_opts.endpts[i].name, name)) {
ERR("Endpoint \"%s\" already exists.", name);
- /* READ UNLOCK */
+ /* WRITE UNLOCK */
pthread_rwlock_unlock(&server_opts.endpt_array_lock);
return -1;
}
}
- /* READ UNLOCK */
- pthread_rwlock_unlock(&server_opts.endpt_array_lock);
-
sock = nc_sock_listen(address, port);
if (sock == -1) {
+ /* WRITE UNLOCK */
+ pthread_rwlock_unlock(&server_opts.endpt_array_lock);
return -1;
}
- /* WRITE LOCK */
- pthread_rwlock_wrlock(&server_opts.endpt_array_lock);
-
++server_opts.endpt_count;
server_opts.binds = realloc(server_opts.binds, server_opts.endpt_count * sizeof *server_opts.binds);
server_opts.endpts = realloc(server_opts.endpts, server_opts.endpt_count * sizeof *server_opts.endpts);
@@ -956,6 +952,7 @@
return -1;
}
+ /* LOCK */
endpt = nc_server_endpt_lock(endpt_name, ti);
if (!endpt) {
return -1;
@@ -969,8 +966,7 @@
}
if (!bind) {
ERRINT;
- nc_server_endpt_unlock(endpt);
- return -1;
+ goto fail;
}
if (address) {
@@ -979,8 +975,7 @@
sock = nc_sock_listen(bind->address, port);
}
if (sock == -1) {
- nc_server_endpt_unlock(endpt);
- return -1;
+ goto fail;
}
/* close old socket, update parameters */
@@ -993,8 +988,14 @@
bind->port = port;
}
+ /* UNLOCK */
nc_server_endpt_unlock(endpt);
return 0;
+
+fail:
+ /* UNLOCK */
+ nc_server_endpt_unlock(endpt);
+ return -1;
}
int
@@ -1008,10 +1009,12 @@
if (!name && !ti) {
/* remove all */
- nc_ctx_lock(-1, NULL);
for (i = 0; i < server_opts.endpt_count; ++i) {
+ nc_ctx_lock(-1, NULL);
lydict_remove(server_opts.ctx, server_opts.endpts[i].name);
lydict_remove(server_opts.ctx, server_opts.binds[i].address);
+ nc_ctx_unlock();
+
close(server_opts.binds[i].sock);
pthread_mutex_destroy(&server_opts.endpts[i].endpt_lock);
switch (server_opts.binds[i].ti) {
@@ -1033,7 +1036,6 @@
ret = 0;
}
- nc_ctx_unlock();
free(server_opts.endpts);
server_opts.endpts = NULL;
server_opts.endpt_count = 0;
@@ -1048,6 +1050,7 @@
lydict_remove(server_opts.ctx, server_opts.endpts[i].name);
lydict_remove(server_opts.ctx, server_opts.binds[i].address);
nc_ctx_unlock();
+
close(server_opts.binds[i].sock);
pthread_mutex_destroy(&server_opts.endpts[i].endpt_lock);
switch (server_opts.binds[i].ti) {
@@ -1101,26 +1104,32 @@
char *host = NULL;
uint16_t port, idx;
- if (!server_opts.ctx || !server_opts.endpt_count || !session) {
+ if (!server_opts.ctx || !session) {
ERRARG;
return -1;
}
- /* READ LOCK */
- pthread_rwlock_rdlock(&server_opts.endpt_array_lock);
+ /* we have to hold WRITE for the whole time, since there is not
+ * a way of downgrading the lock to READ */
+ /* WRITE LOCK */
+ pthread_rwlock_wrlock(&server_opts.endpt_array_lock);
+
+ if (!server_opts.endpt_count) {
+ ERRARG;
+ /* WRITE UNLOCK */
+ pthread_rwlock_unlock(&server_opts.endpt_array_lock);
+ return -1;
+ }
ret = nc_sock_accept_binds(server_opts.binds, server_opts.endpt_count, timeout, &host, &port, &idx);
if (ret < 1) {
- /* READ UNLOCK */
+ /* WRITE UNLOCK */
pthread_rwlock_unlock(&server_opts.endpt_array_lock);
return ret;
}
sock = ret;
- /* ENDPT LOCK */
- pthread_mutex_lock(&server_opts.endpts[idx].endpt_lock);
-
*session = calloc(1, sizeof **session);
if (!(*session)) {
ERRMEM;
@@ -1174,10 +1183,7 @@
goto fail;
}
- /* ENDPT UNLOCK */
- pthread_mutex_unlock(&server_opts.endpts[idx].endpt_lock);
-
- /* READ UNLOCK */
+ /* WRITE UNLOCK */
pthread_rwlock_unlock(&server_opts.endpt_array_lock);
/* assign new SID atomically */
@@ -1198,8 +1204,6 @@
return 1;
fail:
- /* ENDPT UNLOCK */
- pthread_mutex_unlock(&server_opts.endpts[idx].endpt_lock);
/* WRITE UNLOCK */
pthread_rwlock_unlock(&server_opts.endpt_array_lock);
diff --git a/src/session_server_ssh.c b/src/session_server_ssh.c
index a04de3b..0bb4d4b 100644
--- a/src/session_server_ssh.c
+++ b/src/session_server_ssh.c
@@ -100,11 +100,13 @@
int ret;
struct nc_endpt *endpt;
+ /* LOCK */
endpt = nc_server_endpt_lock(endpt_name, NC_TI_LIBSSH);
if (!endpt) {
return -1;
}
ret = nc_server_ssh_set_hostkey(privkey_path, endpt->ti_opts);
+ /* UNLOCK */
nc_server_endpt_unlock(endpt);
return ret;
@@ -151,11 +153,13 @@
int ret;
struct nc_endpt *endpt;
+ /* LOCK */
endpt = nc_server_endpt_lock(endpt_name, NC_TI_LIBSSH);
if (!endpt) {
return -1;
}
ret = nc_server_ssh_set_banner(banner, endpt->ti_opts);
+ /* UNLOCK */
nc_server_endpt_unlock(endpt);
return ret;
@@ -194,11 +198,13 @@
int ret;
struct nc_endpt *endpt;
+ /* LOCK */
endpt = nc_server_endpt_lock(endpt_name, NC_TI_LIBSSH);
if (!endpt) {
return -1;
}
ret = nc_server_ssh_set_auth_methods(auth_methods, endpt->ti_opts);
+ /* UNLOCK */
nc_server_endpt_unlock(endpt);
return ret;
@@ -236,11 +242,13 @@
int ret;
struct nc_endpt *endpt;
+ /* LOCK */
endpt = nc_server_endpt_lock(endpt_name, NC_TI_LIBSSH);
if (!endpt) {
return -1;
}
ret = nc_server_ssh_set_auth_attempts(auth_attempts, endpt->ti_opts);
+ /* UNLOCK */
nc_server_endpt_unlock(endpt);
return ret;
@@ -278,11 +286,13 @@
int ret;
struct nc_endpt *endpt;
+ /* LOCK */
endpt = nc_server_endpt_lock(endpt_name, NC_TI_LIBSSH);
if (!endpt) {
return -1;
}
ret = nc_server_ssh_set_auth_timeout(auth_timeout, endpt->ti_opts);
+ /* UNLOCK */
nc_server_endpt_unlock(endpt);
return ret;
@@ -327,11 +337,13 @@
int ret;
struct nc_endpt *endpt;
+ /* LOCK */
endpt = nc_server_endpt_lock(endpt_name, NC_TI_LIBSSH);
if (!endpt) {
return -1;
}
ret = nc_server_ssh_add_authkey(pubkey_path, username, endpt->ti_opts);
+ /* UNLOCK */
nc_server_endpt_unlock(endpt);
return ret;
@@ -400,11 +412,13 @@
int ret;
struct nc_endpt *endpt;
+ /* LOCK */
endpt = nc_server_endpt_lock(endpt_name, NC_TI_LIBSSH);
if (!endpt) {
return -1;
}
ret = nc_server_ssh_del_authkey(pubkey_path, username, endpt->ti_opts);
+ /* UNLOCK */
nc_server_endpt_unlock(endpt);
return ret;
diff --git a/src/session_server_tls.c b/src/session_server_tls.c
index 179f8ea..45a065b 100644
--- a/src/session_server_tls.c
+++ b/src/session_server_tls.c
@@ -719,11 +719,13 @@
int ret;
struct nc_endpt *endpt;
+ /* LOCK */
endpt = nc_server_endpt_lock(endpt_name, NC_TI_OPENSSL);
if (!endpt) {
return -1;
}
ret = nc_server_tls_set_cert(cert, endpt->ti_opts);
+ /* UNLOCK */
nc_server_endpt_unlock(endpt);
return ret;
@@ -777,11 +779,13 @@
int ret;
struct nc_endpt *endpt;
+ /* LOCK */
endpt = nc_server_endpt_lock(endpt_name, NC_TI_OPENSSL);
if (!endpt) {
return -1;
}
ret = nc_server_tls_set_cert_path(cert_path, endpt->ti_opts);
+ /* UNLOCK */
nc_server_endpt_unlock(endpt);
return ret;
@@ -840,11 +844,13 @@
int ret;
struct nc_endpt *endpt;
+ /* LOCK */
endpt = nc_server_endpt_lock(endpt_name, NC_TI_OPENSSL);
if (!endpt) {
return -1;
}
ret = nc_server_tls_set_key(privkey, is_rsa, endpt->ti_opts);
+ /* UNLOCK */
nc_server_endpt_unlock(endpt);
return ret;
@@ -898,11 +904,13 @@
int ret;
struct nc_endpt *endpt;
+ /* LOCK */
endpt = nc_server_endpt_lock(endpt_name, NC_TI_OPENSSL);
if (!endpt) {
return -1;
}
ret = nc_server_tls_set_key_path(privkey_path, endpt->ti_opts);
+ /* UNLOCK */
nc_server_endpt_unlock(endpt);
return ret;
@@ -968,11 +976,13 @@
int ret;
struct nc_endpt *endpt;
+ /* LOCK */
endpt = nc_server_endpt_lock(endpt_name, NC_TI_OPENSSL);
if (!endpt) {
return -1;
}
ret = nc_server_tls_add_trusted_cert(cert, endpt->ti_opts);
+ /* UNLOCK */
nc_server_endpt_unlock(endpt);
return ret;
@@ -1040,11 +1050,13 @@
int ret;
struct nc_endpt *endpt;
+ /* LOCK */
endpt = nc_server_endpt_lock(endpt_name, NC_TI_OPENSSL);
if (!endpt) {
return -1;
}
ret = nc_server_tls_add_trusted_cert_path(cert_path, endpt->ti_opts);
+ /* UNLOCK */
nc_server_endpt_unlock(endpt);
return ret;
@@ -1128,11 +1140,13 @@
int ret;
struct nc_endpt *endpt;
+ /* LOCK */
endpt = nc_server_endpt_lock(endpt_name, NC_TI_OPENSSL);
if (!endpt) {
return -1;
}
ret = nc_server_tls_set_trusted_ca_paths(ca_file, ca_dir, endpt->ti_opts);
+ /* UNLOCK */
nc_server_endpt_unlock(endpt);
return ret;
@@ -1168,11 +1182,13 @@
{
struct nc_endpt *endpt;
+ /* LOCK */
endpt = nc_server_endpt_lock(endpt_name, NC_TI_OPENSSL);
if (!endpt) {
return;
}
nc_server_tls_clear_certs(endpt->ti_opts);
+ /* UNLOCK */
nc_server_endpt_unlock(endpt);
}
@@ -1238,11 +1254,13 @@
int ret;
struct nc_endpt *endpt;
+ /* LOCK */
endpt = nc_server_endpt_lock(endpt_name, NC_TI_OPENSSL);
if (!endpt) {
return -1;
}
ret = nc_server_tls_set_crl_paths(crl_file, crl_dir, endpt->ti_opts);
+ /* UNLOCK */
nc_server_endpt_unlock(endpt);
return ret;
@@ -1278,11 +1296,13 @@
{
struct nc_endpt *endpt;
+ /* LOCK */
endpt = nc_server_endpt_lock(endpt_name, NC_TI_OPENSSL);
if (!endpt) {
return;
}
nc_server_tls_clear_crls(endpt->ti_opts);
+ /* UNLOCK */
nc_server_endpt_unlock(endpt);
}
@@ -1340,11 +1360,13 @@
int ret;
struct nc_endpt *endpt;
+ /* LOCK */
endpt = nc_server_endpt_lock(endpt_name, NC_TI_OPENSSL);
if (!endpt) {
return -1;
}
ret = nc_server_tls_add_ctn(id, fingerprint, map_type, name, endpt->ti_opts);
+ /* UNLOCK */
nc_server_endpt_unlock(endpt);
return ret;
@@ -1425,11 +1447,13 @@
int ret;
struct nc_endpt *endpt;
+ /* LOCK */
endpt = nc_server_endpt_lock(endpt_name, NC_TI_OPENSSL);
if (!endpt) {
return -1;
}
ret = nc_server_tls_del_ctn(id, fingerprint, map_type, name, endpt->ti_opts);
+ /* UNLOCK */
nc_server_endpt_unlock(endpt);
return ret;