session CHANGE ctx lock removed, dict access is now always thread-safe
diff --git a/src/messages_server.c b/src/messages_server.c
index c94bd96..3d018b9 100644
--- a/src/messages_server.c
+++ b/src/messages_server.c
@@ -65,9 +65,7 @@
ret->type = NC_RPL_DATA;
if (paramtype == NC_PARAMTYPE_DUP_AND_FREE) {
- nc_ctx_lock(-1, NULL);
ret->data = lyd_dup(data, 1);
- nc_ctx_unlock();
} else {
ret->data = data;
}
@@ -308,12 +306,10 @@
return -1;
}
- nc_ctx_lock(-1, NULL);
if (err->apptag) {
lydict_remove(server_opts.ctx, err->apptag);
}
err->apptag = lydict_insert(server_opts.ctx, error_app_tag, 0);
- nc_ctx_unlock();
return 0;
}
@@ -326,12 +322,10 @@
return -1;
}
- nc_ctx_lock(-1, NULL);
if (err->path) {
lydict_remove(server_opts.ctx, err->path);
}
err->path = lydict_insert(server_opts.ctx, error_path, 0);
- nc_ctx_unlock();
return 0;
}
@@ -344,7 +338,6 @@
return -1;
}
- nc_ctx_lock(-1, NULL);
if (err->message) {
lydict_remove(server_opts.ctx, err->apptag);
}
@@ -358,7 +351,6 @@
} else {
lang = NULL;
}
- nc_ctx_unlock();
return 0;
}
@@ -385,10 +377,7 @@
++err->attr_count;
err->attr = realloc(err->attr, err->attr_count * sizeof *err->attr);
-
- nc_ctx_lock(-1, NULL);
err->attr[err->attr_count - 1] = lydict_insert(server_opts.ctx, attr_name, 0);
- nc_ctx_unlock();
return 0;
}
@@ -403,10 +392,7 @@
++err->elem_count;
err->elem = realloc(err->elem, err->elem_count * sizeof *err->elem);
-
- nc_ctx_lock(-1, NULL);
err->elem[err->elem_count - 1] = lydict_insert(server_opts.ctx, elem_name, 0);
- nc_ctx_unlock();
return 0;
}
@@ -421,10 +407,7 @@
++err->ns_count;
err->ns = realloc(err->ns, err->ns_count * sizeof *err->ns);
-
- nc_ctx_lock(-1, NULL);
err->ns[err->ns_count - 1] = lydict_insert(server_opts.ctx, ns_name, 0);
- nc_ctx_unlock();
return 0;
}
@@ -450,10 +433,8 @@
return;
}
- nc_ctx_lock(-1, NULL);
lyxml_free(ctx, rpc->root);
lyd_free(rpc->tree);
- nc_ctx_unlock();
free(rpc);
}
@@ -473,9 +454,7 @@
case NC_RPL_DATA:
data_rpl = (struct nc_server_reply_data *)reply;
if (data_rpl->free) {
- nc_ctx_lock(-1, NULL);
lyd_free_withsiblings(data_rpl->data);
- nc_ctx_unlock();
}
break;
case NC_RPL_OK:
@@ -503,7 +482,6 @@
return;
}
- nc_ctx_lock(-1, NULL);
lydict_remove(server_opts.ctx, err->apptag);
lydict_remove(server_opts.ctx, err->path);
lydict_remove(server_opts.ctx, err->message);
@@ -523,7 +501,6 @@
for (i = 0; i < err->other_count; ++i) {
lyxml_free(server_opts.ctx, err->other[i]);
}
- nc_ctx_unlock();
free(err->other);
free(err);
}
diff --git a/src/session.c b/src/session.c
index d66374e..a703a83 100644
--- a/src/session.c
+++ b/src/session.c
@@ -381,14 +381,8 @@
session->ti.libssh.next = siter->ti.libssh.next;
/* free starting SSH NETCONF session (channel will be freed in ssh_free()) */
- if (session->side == NC_SERVER) {
- nc_ctx_lock(-1, NULL);
- }
lydict_remove(session->ctx, session->username);
lydict_remove(session->ctx, session->host);
- if (session->side == NC_SERVER) {
- nc_ctx_unlock();
- }
if (!(session->flags & NC_SESSION_SHAREDCTX)) {
ly_ctx_destroy(session->ctx, NULL);
}
@@ -440,14 +434,8 @@
break;
}
- if (session->side == NC_SERVER) {
- nc_ctx_lock(-1, NULL);
- }
lydict_remove(session->ctx, session->username);
lydict_remove(session->ctx, session->host);
- if (session->side == NC_SERVER) {
- nc_ctx_unlock();
- }
/* final cleanup */
if (session->ti_lock) {
@@ -491,11 +479,8 @@
int size = 10, count, feat_count = 0, i, str_len;
char str[512];
- nc_ctx_lock(-1, NULL);
-
yanglib = ly_ctx_info(ctx);
if (!yanglib) {
- nc_ctx_unlock();
return NULL;
}
@@ -637,8 +622,6 @@
lyd_free(yanglib);
- nc_ctx_unlock();
-
/* ending NULL capability */
add_cpblt(ctx, NULL, &cpblts, &size, &count);
@@ -733,11 +716,9 @@
r = nc_write_msg(session, NC_MSG_HELLO, cpblts, &session->id);
- nc_ctx_lock(-1, NULL);
for (i = 0; cpblts[i]; ++i) {
lydict_remove(session->ctx, cpblts[i]);
}
- nc_ctx_unlock();
free(cpblts);
if (r) {
@@ -871,9 +852,7 @@
}
cleanup:
- nc_ctx_lock(-1, NULL);
lyxml_free(session->ctx, xml);
- nc_ctx_unlock();
return msgtype;
}
diff --git a/src/session_p.h b/src/session_p.h
index ee7b7c7..5a0f57f 100644
--- a/src/session_p.h
+++ b/src/session_p.h
@@ -131,9 +131,8 @@
};
struct nc_server_opts {
- /* ACCESS locked with ctx_lock */
+ /* ACCESS unlocked (dictionary locked internally in libyang) */
struct ly_ctx *ctx;
- pthread_mutex_t ctx_lock;
/* ACCESS unlocked */
NC_WD_MODE wd_basic_mode;
diff --git a/src/session_server.c b/src/session_server.c
index 6be7f2d..1fa5fe5 100644
--- a/src/session_server.c
+++ b/src/session_server.c
@@ -306,9 +306,7 @@
sdata = ly_ctx_get_node(server_opts.ctx, "/ietf-netconf-monitoring:get-schema/output/data");
if (model_data && sdata) {
- nc_ctx_lock(-1, NULL);
data = lyd_output_new_anyxml(sdata, model_data);
- nc_ctx_unlock();
}
free(model_data);
if (!data) {
@@ -556,10 +554,7 @@
case NC_MSG_RPC:
*rpc = malloc(sizeof **rpc);
- 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;
@@ -826,27 +821,6 @@
}
}
-API int
-nc_ctx_lock(int timeout, int *elapsed)
-{
- return nc_timedlock(&server_opts.ctx_lock, timeout, elapsed);
-}
-
-API int
-nc_ctx_unlock(void)
-{
- int ret;
-
- ret = pthread_mutex_unlock(&server_opts.ctx_lock);
-
- if (ret) {
- ERR("Mutex unlock failed (%s).", strerror(ret));
- return -1;
- }
-
- return 0;
-}
-
#if defined(ENABLE_SSH) || defined(ENABLE_TLS)
int
@@ -887,10 +861,8 @@
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);
- nc_ctx_lock(-1, NULL);
server_opts.endpts[server_opts.endpt_count - 1].name = lydict_insert(server_opts.ctx, name, 0);
server_opts.binds[server_opts.endpt_count - 1].address = lydict_insert(server_opts.ctx, address, 0);
- nc_ctx_unlock();
server_opts.binds[server_opts.endpt_count - 1].port = port;
server_opts.binds[server_opts.endpt_count - 1].sock = sock;
server_opts.binds[server_opts.endpt_count - 1].ti = ti;
@@ -995,10 +967,8 @@
if (!name && !ti) {
/* remove all */
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);
@@ -1033,11 +1003,8 @@
if ((server_opts.binds[i].ti == ti) &&
(!name || !strcmp(server_opts.endpts[i].name, name))) {
- 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) {
@@ -1130,9 +1097,7 @@
(*session)->side = NC_SERVER;
(*session)->ctx = server_opts.ctx;
(*session)->flags = NC_SESSION_SHAREDCTX;
- nc_ctx_lock(-1, NULL);
(*session)->host = lydict_insert_zc(server_opts.ctx, host);
- nc_ctx_unlock();
(*session)->port = port;
/* transport lock */
@@ -1225,9 +1190,7 @@
(*session)->side = NC_SERVER;
(*session)->ctx = server_opts.ctx;
(*session)->flags = NC_SESSION_SHAREDCTX | NC_SESSION_CALLHOME;
- nc_ctx_lock(-1, NULL);
(*session)->host = lydict_insert(server_opts.ctx, host, 0);
- nc_ctx_unlock();
(*session)->port = port;
/* transport lock */
diff --git a/src/session_server.h b/src/session_server.h
index 2ac9b3d..11baf34 100644
--- a/src/session_server.h
+++ b/src/session_server.h
@@ -53,9 +53,8 @@
* @brief Initialize the server using a libyang context.
*
* The context is not modified internally, only its dictionary is used for holding
- * all the strings. When the dictionary is being written to or removed from,
- * libnetconf2 always holds ctx lock using nc_ctx_lock(). Reading models is considered
- * thread-safe as models cannot be removed and are rarely modified.
+ * all the strings, which is thread-safe. Reading models is considered thread-safe
+ * as models cannot be removed and are rarely modified (augments or deviations).
*
* Server capabilities are generated based on its content. Changing the context
* in ways that result in changed capabilities (adding models, changing features)
@@ -71,9 +70,6 @@
* received. Callbacks for ietf-netconf:get-schema (supporting YANG and YIN format
* only) and ietf-netconf:close-session are set internally if left unset.
*
- * Access to the context in libnetconf2 functions is not managed in any way,
- * the application is responsible for handling it in a thread-safe manner.
- *
* @param[in] ctx Core NETCONF server context.
* @return 0 on success, -1 on error.
*/
@@ -205,24 +201,6 @@
*/
void nc_ps_clear(struct nc_pollsession *ps);
-/**
- * @brief Lock server context.
- *
- * @param[in] timeout Timeout in milliseconds. 0 for non-blocking call, -1 for
- * infinite waiting.
- * @param[out] elapsed Elapsed milliseconds will be added to this variable.
- * Can be NULL.
- * @return 1 on success, 0 on elapsed timeout, -1 on error.
- */
-int nc_ctx_lock(int timeout, int *elapsed);
-
-/**
- * @brief Unlock server context.
- *
- * @return 0 on success, -1 on error.
- */
-int nc_ctx_unlock(void);
-
#if defined(ENABLE_SSH) || defined(ENABLE_TLS)
/**
diff --git a/src/session_server_ssh.c b/src/session_server_ssh.c
index c405ac4..bbcb0e3 100644
--- a/src/session_server_ssh.c
+++ b/src/session_server_ssh.c
@@ -322,11 +322,8 @@
++opts->authkey_count;
opts->authkeys = realloc(opts->authkeys, opts->authkey_count * sizeof *opts->authkeys);
-
- nc_ctx_lock(-1, NULL);
opts->authkeys[opts->authkey_count - 1].path = lydict_insert(server_opts.ctx, pubkey_path, 0);
opts->authkeys[opts->authkey_count - 1].username = lydict_insert(server_opts.ctx, username, 0);
- nc_ctx_unlock();
return 0;
}
@@ -370,14 +367,12 @@
int ret = -1;
if (!pubkey_path && !username) {
- nc_ctx_lock(-1, NULL);
for (i = 0; i < opts->authkey_count; ++i) {
lydict_remove(server_opts.ctx, opts->authkeys[i].path);
lydict_remove(server_opts.ctx, opts->authkeys[i].username);
ret = 0;
}
- nc_ctx_unlock();
free(opts->authkeys);
opts->authkeys = NULL;
opts->authkey_count = 0;
@@ -385,10 +380,8 @@
for (i = 0; i < opts->authkey_count; ++i) {
if ((!pubkey_path || !strcmp(opts->authkeys[i].path, pubkey_path))
&& (!username || !strcmp(opts->authkeys[i].username, username))) {
- nc_ctx_lock(-1, NULL);
lydict_remove(server_opts.ctx, opts->authkeys[i].path);
lydict_remove(server_opts.ctx, opts->authkeys[i].username);
- nc_ctx_unlock();
--opts->authkey_count;
if (i < opts->authkey_count) {
diff --git a/src/session_server_tls.c b/src/session_server_tls.c
index 45a065b..77a246d 100644
--- a/src/session_server_tls.c
+++ b/src/session_server_tls.c
@@ -626,9 +626,7 @@
/* cert-to-name match, now to extract the specific field from the peer cert */
if (map_type == NC_TLS_CTN_SPECIFIED) {
- nc_ctx_lock(-1, NULL);
session->username = lydict_insert(server_opts.ctx, username, 0);
- nc_ctx_unlock();
} else {
rc = nc_tls_ctn_get_username_from_cert(session->tls_cert, map_type, &cp);
if (rc) {
@@ -637,9 +635,7 @@
}
goto fail;
}
- nc_ctx_lock(-1, NULL);
session->username = lydict_insert_zc(server_opts.ctx, cp);
- nc_ctx_unlock();
}
VRB("Cert verify CTN: new client username recognized as \"%s\".", session->username);
@@ -1329,10 +1325,8 @@
new = malloc(sizeof *new);
- nc_ctx_lock(-1, NULL);
new->fingerprint = lydict_insert(server_opts.ctx, fingerprint, 0);
new->name = lydict_insert(server_opts.ctx, name, 0);
- nc_ctx_unlock();
new->id = id;
new->map_type = map_type;
new->next = NULL;
@@ -1394,7 +1388,6 @@
if ((id < 0) && !fingerprint && !map_type && !name) {
ctn = opts->ctn;
- nc_ctx_lock(-1, NULL);
while (ctn) {
lydict_remove(server_opts.ctx, ctn->fingerprint);
lydict_remove(server_opts.ctx, ctn->name);
@@ -1405,7 +1398,6 @@
ret = 0;
}
- nc_ctx_unlock();
opts->ctn = NULL;
} else {
prev = NULL;
@@ -1415,10 +1407,8 @@
&& (!fingerprint || !strcmp(ctn->fingerprint, fingerprint))
&& (!map_type || (ctn->map_type == map_type))
&& (!name || (ctn->name && !strcmp(ctn->name, name)))) {
- nc_ctx_lock(-1, NULL);
lydict_remove(server_opts.ctx, ctn->fingerprint);
lydict_remove(server_opts.ctx, ctn->name);
- nc_ctx_unlock();
if (prev) {
prev->next = ctn->next;