Revert "lib: introduce a read-write lock for northbound configurations"

Adding a lock to protect the global running configuration doesn't
help much since the FRR daemons are not prepared to process
configuration changes in a pthread that is not the main one (a
whole lot of new protections would be necessary to prevent race
conditions).

This means the lock added by commit 83981138 only adds more
complexity for no benefit. Remove it now to simplify the code.

All northbound clients, including the gRPC one, should either run
in the main pthread or use synchronization primitives to process
configuration transactions in the main pthread.

This reverts commit 83981138fe8c1e0a40b8dede74eca65449dda5de.

14 files changed, 207 insertions, 350 deletions

diff --git a/bfdd/bfd.c b/bfdd/bfd.c
index feb9827ce..245f2dc5e 100644
--- a/bfdd/bfd.c
+++ b/bfdd/bfd.c
@@ -1791,16 +1791,11 @@ void bfd_session_update_vrf_name(struct bfd_session *bs, struct vrf *vrf)
 		snprintf(xpath + slen, sizeof(xpath) - slen, "[vrf='%s']/vrf",
 			 bs->key.vrfname);
 
-		pthread_rwlock_wrlock(&running_config->lock);
-		{
-			bfd_dnode = yang_dnode_get(
-						   running_config->dnode,
-						   xpath, bs->key.vrfname);
-			if (bfd_dnode) {
-				yang_dnode_change_leaf(bfd_dnode, vrf->name);
-				running_config->version++;
-			}
-			pthread_rwlock_unlock(&running_config->lock);
+		bfd_dnode = yang_dnode_get(running_config->dnode, xpath,
+					   bs->key.vrfname);
+		if (bfd_dnode) {
+			yang_dnode_change_leaf(bfd_dnode, vrf->name);
+			running_config->version++;
 		}
 	}
 	memset(bs->key.vrfname, 0, sizeof(bs->key.vrfname));
diff --git a/isisd/isis_cli.c b/isisd/isis_cli.c
index bd0628675..37f4dcfab 100644
--- a/isisd/isis_cli.c
+++ b/isisd/isis_cli.c
@@ -188,14 +188,10 @@ DEFPY(ip_router_isis, ip_router_isis_cmd, "ip router isis WORD$tag",
 	}
 
 	/* check if the interface is a loopback and if so set it as passive */
-	pthread_rwlock_rdlock(&running_config->lock);
-	{
-		ifp = nb_running_get_entry(NULL, VTY_CURR_XPATH, false);
-		if (ifp && if_is_loopback(ifp))
-			nb_cli_enqueue_change(vty, "./frr-isisd:isis/passive",
-					      NB_OP_MODIFY, "true");
-	}
-	pthread_rwlock_unlock(&running_config->lock);
+	ifp = nb_running_get_entry(NULL, VTY_CURR_XPATH, false);
+	if (ifp && if_is_loopback(ifp))
+		nb_cli_enqueue_change(vty, "./frr-isisd:isis/passive",
+				      NB_OP_MODIFY, "true");
 
 	return nb_cli_apply_changes(vty, NULL);
 }
@@ -262,14 +258,10 @@ DEFPY(ip6_router_isis, ip6_router_isis_cmd, "ipv6 router isis WORD$tag",
 	}
 
 	/* check if the interface is a loopback and if so set it as passive */
-	pthread_rwlock_rdlock(&running_config->lock);
-	{
-		ifp = nb_running_get_entry(NULL, VTY_CURR_XPATH, false);
-		if (ifp && if_is_loopback(ifp))
-			nb_cli_enqueue_change(vty, "./frr-isisd:isis/passive",
-					      NB_OP_MODIFY, "true");
-	}
-	pthread_rwlock_unlock(&running_config->lock);
+	ifp = nb_running_get_entry(NULL, VTY_CURR_XPATH, false);
+	if (ifp && if_is_loopback(ifp))
+		nb_cli_enqueue_change(vty, "./frr-isisd:isis/passive",
+				      NB_OP_MODIFY, "true");
 
 	return nb_cli_apply_changes(vty, NULL);
 }
@@ -410,26 +402,20 @@ DEFPY(no_is_type, no_is_type_cmd,
       "Act as both a station router and an area router\n"
       "Act as an area router only\n")
 {
-	const char *value;
-
-	pthread_rwlock_rdlock(&running_config->lock);
-	{
-		struct isis_area *area;
+	const char *value = NULL;
+	struct isis_area *area;
 
-		area = nb_running_get_entry(NULL, VTY_CURR_XPATH, false);
-
-		/*
-		 * Put the is-type back to defaults:
-		 * - level-1-2 on first area
-		 * - level-1 for the rest
-		 */
-		if (area && listgetdata(listhead(isis->area_list)) == area)
-			value = "level-1-2";
-		else
-			value = NULL;
-	}
-	pthread_rwlock_unlock(&running_config->lock);
+	area = nb_running_get_entry(NULL, VTY_CURR_XPATH, false);
 
+	/*
+	 * Put the is-type back to defaults:
+	 * - level-1-2 on first area
+	 * - level-1 for the rest
+	 */
+	if (area && listgetdata(listhead(isis->area_list)) == area)
+		value = "level-1-2";
+	else
+		value = NULL;
 	nb_cli_enqueue_change(vty, "./is-type", NB_OP_MODIFY, value);
 
 	return nb_cli_apply_changes(vty, NULL);
@@ -1817,45 +1803,52 @@ DEFPY(no_isis_circuit_type, no_isis_circuit_type_cmd,
       "Level-1-2 adjacencies are formed\n"
       "Level-2 only adjacencies are formed\n")
 {
-	const char *circ_type = NULL;
+	struct interface *ifp;
+	struct isis_circuit *circuit;
+	int is_type;
+	const char *circ_type;
 
 	/*
 	 * Default value depends on whether the circuit is part of an area,
 	 * and the is-type of the area if there is one. So we need to do this
 	 * here.
 	 */
-	pthread_rwlock_rdlock(&running_config->lock);
-	{
-		struct interface *ifp;
-		struct isis_circuit *circuit;
+	ifp = nb_running_get_entry(NULL, VTY_CURR_XPATH, false);
+	if (!ifp)
+		goto def_val;
 
-		ifp = nb_running_get_entry(NULL, VTY_CURR_XPATH, false);
-		if (!ifp)
-			goto unlock;
+	circuit = circuit_scan_by_ifp(ifp);
+	if (!circuit)
+		goto def_val;
 
-		circuit = circuit_scan_by_ifp(ifp);
-		if (!circuit || circuit->state != C_STATE_UP)
-			goto unlock;
+	if (circuit->state == C_STATE_UP)
+		is_type = circuit->area->is_type;
+	else
+		goto def_val;
 
-		switch (circuit->area->is_type) {
-		case IS_LEVEL_1:
-			circ_type = "level-1";
-			break;
-		case IS_LEVEL_2:
-			circ_type = "level-2";
-			break;
-		case IS_LEVEL_1_AND_2:
-			circ_type = "level-1-2";
-			break;
-		}
+	switch (is_type) {
+	case IS_LEVEL_1:
+		circ_type = "level-1";
+		break;
+	case IS_LEVEL_2:
+		circ_type = "level-2";
+		break;
+	case IS_LEVEL_1_AND_2:
+		circ_type = "level-1-2";
+		break;
+	default:
+		return CMD_ERR_NO_MATCH;
 	}
-unlock:
-	pthread_rwlock_unlock(&running_config->lock);
-
 	nb_cli_enqueue_change(vty, "./frr-isisd:isis/circuit-type",
 			      NB_OP_MODIFY, circ_type);
 
 	return nb_cli_apply_changes(vty, NULL);
+
+def_val:
+	nb_cli_enqueue_change(vty, "./frr-isisd:isis/circuit-type",
+			      NB_OP_MODIFY, NULL);
+
+	return nb_cli_apply_changes(vty, NULL);
 }
 
 void cli_show_ip_isis_circ_type(struct vty *vty, struct lyd_node *dnode,
diff --git a/lib/command.c b/lib/command.c
index eecca2a5f..04f2bd95a 100644
--- a/lib/command.c
+++ b/lib/command.c
@@ -1718,16 +1718,12 @@ static int vty_write_config(struct vty *vty)
 	vty_out(vty, "frr defaults %s\n", DFLT_NAME);
 	vty_out(vty, "!\n");
 
-	pthread_rwlock_rdlock(&running_config->lock);
-	{
-		for (i = 0; i < vector_active(cmdvec); i++)
-			if ((node = vector_slot(cmdvec, i)) && node->func
-			    && (node->vtysh || vty->type != VTY_SHELL)) {
-				if ((*node->func)(vty))
-					vty_out(vty, "!\n");
-			}
-	}
-	pthread_rwlock_unlock(&running_config->lock);
+	for (i = 0; i < vector_active(cmdvec); i++)
+		if ((node = vector_slot(cmdvec, i)) && node->func
+		    && (node->vtysh || vty->type != VTY_SHELL)) {
+			if ((*node->func)(vty))
+				vty_out(vty, "!\n");
+		}
 
 	if (vty->type == VTY_TERM) {
 		vty_out(vty, "end\n");
diff --git a/lib/if.c b/lib/if.c
index 5f9232756..a3e631366 100644
--- a/lib/if.c
+++ b/lib/if.c
@@ -207,21 +207,18 @@ void if_update_to_new_vrf(struct interface *ifp, vrf_id_t vrf_id)
 	if (yang_module_find("frr-interface")) {
 		struct lyd_node *if_dnode;
 
-		pthread_rwlock_wrlock(&running_config->lock);
-		{
-			if_dnode = yang_dnode_get(
-				running_config->dnode,
-				"/frr-interface:lib/interface[name='%s'][vrf='%s']/vrf",
-				ifp->name, old_vrf->name);
-			if (if_dnode) {
-				yang_dnode_change_leaf(if_dnode, vrf->name);
-				running_config->version++;
-			}
+		if_dnode = yang_dnode_get(
+			running_config->dnode,
+			"/frr-interface:lib/interface[name='%s'][vrf='%s']/vrf",
+			ifp->name, old_vrf->name);
+		if (if_dnode) {
+			yang_dnode_change_leaf(if_dnode, vrf->name);
+			running_config->version++;
 		}
-		pthread_rwlock_unlock(&running_config->lock);
 	}
 }
 
+
 /* Delete interface structure. */
 void if_delete_retain(struct interface *ifp)
 {
diff --git a/lib/libfrr.c b/lib/libfrr.c
index 478fe4e20..d4aa1f899 100644
--- a/lib/libfrr.c
+++ b/lib/libfrr.c
@@ -868,12 +868,7 @@ static int frr_config_read_in(struct thread *t)
 	/*
 	 * Update the shared candidate after reading the startup configuration.
 	 */
-	pthread_rwlock_rdlock(&running_config->lock);
-	{
-		nb_config_replace(vty_shared_candidate_config, running_config,
-				  true);
-	}
-	pthread_rwlock_unlock(&running_config->lock);
+	nb_config_replace(vty_shared_candidate_config, running_config, true);
 
 	return 0;
 }
diff --git a/lib/northbound.c b/lib/northbound.c
index fe7c8bfbc..bffe62f60 100644
--- a/lib/northbound.c
+++ b/lib/northbound.c
@@ -270,7 +270,6 @@ struct nb_config *nb_config_new(struct lyd_node *dnode)
 	else
 		config->dnode = yang_dnode_new(ly_native_ctx, true);
 	config->version = 0;
-	pthread_rwlock_init(&config->lock, NULL);
 
 	return config;
 }
@@ -279,7 +278,6 @@ void nb_config_free(struct nb_config *config)
 {
 	if (config->dnode)
 		yang_dnode_free(config->dnode);
-	pthread_rwlock_destroy(&config->lock);
 	XFREE(MTYPE_NB_CONFIG, config);
 }
 
@@ -290,7 +288,6 @@ struct nb_config *nb_config_dup(const struct nb_config *config)
 	dup = XCALLOC(MTYPE_NB_CONFIG, sizeof(*dup));
 	dup->dnode = yang_dnode_dup(config->dnode);
 	dup->version = config->version;
-	pthread_rwlock_init(&dup->lock, NULL);
 
 	return dup;
 }
@@ -540,28 +537,17 @@ int nb_candidate_edit(struct nb_config *candidate,
 
 bool nb_candidate_needs_update(const struct nb_config *candidate)
 {
-	bool ret = false;
-
-	pthread_rwlock_rdlock(&running_config->lock);
-	{
-		if (candidate->version < running_config->version)
-			ret = true;
-	}
-	pthread_rwlock_unlock(&running_config->lock);
+	if (candidate->version < running_config->version)
+		return true;
 
-	return ret;
+	return false;
 }
 
 int nb_candidate_update(struct nb_config *candidate)
 {
 	struct nb_config *updated_config;
 
-	pthread_rwlock_rdlock(&running_config->lock);
-	{
-		updated_config = nb_config_dup(running_config);
-	}
-	pthread_rwlock_unlock(&running_config->lock);
-
+	updated_config = nb_config_dup(running_config);
 	if (nb_config_merge(updated_config, candidate, true) != NB_OK)
 		return NB_ERR;
 
@@ -645,13 +631,9 @@ int nb_candidate_validate(struct nb_config *candidate)
 		return NB_ERR_VALIDATION;
 
 	RB_INIT(nb_config_cbs, &changes);
-	pthread_rwlock_rdlock(&running_config->lock);
-	{
-		nb_config_diff(running_config, candidate, &changes);
-		ret = nb_candidate_validate_code(candidate, &changes);
-		nb_config_diff_del_changes(&changes);
-	}
-	pthread_rwlock_unlock(&running_config->lock);
+	nb_config_diff(running_config, candidate, &changes);
+	ret = nb_candidate_validate_code(candidate, &changes);
+	nb_config_diff_del_changes(&changes);
 
 	return ret;
 }
@@ -671,35 +653,26 @@ int nb_candidate_commit_prepare(struct nb_config *candidate,
 	}
 
 	RB_INIT(nb_config_cbs, &changes);
-	pthread_rwlock_rdlock(&running_config->lock);
-	{
-		nb_config_diff(running_config, candidate, &changes);
-		if (RB_EMPTY(nb_config_cbs, &changes)) {
-			pthread_rwlock_unlock(&running_config->lock);
-			return NB_ERR_NO_CHANGES;
-		}
+	nb_config_diff(running_config, candidate, &changes);
+	if (RB_EMPTY(nb_config_cbs, &changes))
+		return NB_ERR_NO_CHANGES;
 
-		if (nb_candidate_validate_code(candidate, &changes) != NB_OK) {
-			flog_warn(
-				EC_LIB_NB_CANDIDATE_INVALID,
-				"%s: failed to validate candidate configuration",
-				__func__);
-			nb_config_diff_del_changes(&changes);
-			pthread_rwlock_unlock(&running_config->lock);
-			return NB_ERR_VALIDATION;
-		}
+	if (nb_candidate_validate_code(candidate, &changes) != NB_OK) {
+		flog_warn(EC_LIB_NB_CANDIDATE_INVALID,
+			  "%s: failed to validate candidate configuration",
+			  __func__);
+		nb_config_diff_del_changes(&changes);
+		return NB_ERR_VALIDATION;
+	}
 
-		*transaction = nb_transaction_new(candidate, &changes, client,
-						  user, comment);
-		if (*transaction == NULL) {
-			flog_warn(EC_LIB_NB_TRANSACTION_CREATION_FAILED,
-				  "%s: failed to create transaction", __func__);
-			nb_config_diff_del_changes(&changes);
-			pthread_rwlock_unlock(&running_config->lock);
-			return NB_ERR_LOCKED;
-		}
+	*transaction =
+		nb_transaction_new(candidate, &changes, client, user, comment);
+	if (*transaction == NULL) {
+		flog_warn(EC_LIB_NB_TRANSACTION_CREATION_FAILED,
+			  "%s: failed to create transaction", __func__);
+		nb_config_diff_del_changes(&changes);
+		return NB_ERR_LOCKED;
 	}
-	pthread_rwlock_unlock(&running_config->lock);
 
 	return nb_transaction_process(NB_EV_PREPARE, *transaction);
 }
@@ -718,11 +691,7 @@ void nb_candidate_commit_apply(struct nb_transaction *transaction,
 
 	/* Replace running by candidate. */
 	transaction->config->version++;
-	pthread_rwlock_wrlock(&running_config->lock);
-	{
-		nb_config_replace(running_config, transaction->config, true);
-	}
-	pthread_rwlock_unlock(&running_config->lock);
+	nb_config_replace(running_config, transaction->config, true);
 
 	/* Record transaction. */
 	if (save_transaction
@@ -996,52 +965,40 @@ static int nb_transaction_process(enum nb_event event,
 {
 	struct nb_config_cb *cb;
 
-	/*
-	 * Need to lock the running configuration since transaction->changes
-	 * can contain pointers to data nodes from the running configuration.
-	 */
-	pthread_rwlock_rdlock(&running_config->lock);
-	{
-		RB_FOREACH (cb, nb_config_cbs, &transaction->changes) {
-			struct nb_config_change *change =
-				(struct nb_config_change *)cb;
-			int ret;
+	RB_FOREACH (cb, nb_config_cbs, &transaction->changes) {
+		struct nb_config_change *change = (struct nb_config_change *)cb;
+		int ret;
 
+		/*
+		 * Only try to release resources that were allocated
+		 * successfully.
+		 */
+		if (event == NB_EV_ABORT && change->prepare_ok == false)
+			break;
+
+		/* Call the appropriate callback. */
+		ret = nb_callback_configuration(event, change);
+		switch (event) {
+		case NB_EV_PREPARE:
+			if (ret != NB_OK)
+				return ret;
+			change->prepare_ok = true;
+			break;
+		case NB_EV_ABORT:
+		case NB_EV_APPLY:
 			/*
-			 * Only try to release resources that were allocated
-			 * successfully.
+			 * At this point it's not possible to reject the
+			 * transaction anymore, so any failure here can lead to
+			 * inconsistencies and should be treated as a bug.
+			 * Operations prone to errors, like validations and
+			 * resource allocations, should be performed during the
+			 * 'prepare' phase.
 			 */
-			if (event == NB_EV_ABORT && change->prepare_ok == false)
-				break;
-
-			/* Call the appropriate callback. */
-			ret = nb_callback_configuration(event, change);
-			switch (event) {
-			case NB_EV_PREPARE:
-				if (ret != NB_OK) {
-					pthread_rwlock_unlock(
-						&running_config->lock);
-					return ret;
-				}
-				change->prepare_ok = true;
-				break;
-			case NB_EV_ABORT:
-			case NB_EV_APPLY:
-				/*
-				 * At this point it's not possible to reject the
-				 * transaction anymore, so any failure here can
-				 * lead to inconsistencies and should be treated
-				 * as a bug. Operations prone to errors, like
-				 * validations and resource allocations, should
-				 * be performed during the 'prepare' phase.
-				 */
-				break;
-			default:
-				break;
-			}
+			break;
+		default:
+			break;
 		}
 	}
-	pthread_rwlock_unlock(&running_config->lock);
 
 	return NB_OK;
 }
diff --git a/lib/northbound.h b/lib/northbound.h
index 502fbb9d6..50bddd157 100644
--- a/lib/northbound.h
+++ b/lib/northbound.h
@@ -449,19 +449,8 @@ enum nb_client {
 
 /* Northbound configuration. */
 struct nb_config {
-	/* Configuration data. */
 	struct lyd_node *dnode;
-
-	/* Configuration version. */
 	uint32_t version;
-
-	/*
-	 * Lock protecting this structure. The use of this lock is always
-	 * necessary when reading or modifying the global running configuration.
-	 * For candidate configurations, use of this lock is optional depending
-	 * on the threading scheme of the northbound plugin.
-	 */
-	pthread_rwlock_t lock;
 };
 
 /* Northbound configuration callback. */
diff --git a/lib/northbound_cli.c b/lib/northbound_cli.c
index 884c01a45..a15fe3d1c 100644
--- a/lib/northbound_cli.c
+++ b/lib/northbound_cli.c
@@ -193,13 +193,8 @@ int nb_cli_apply_changes(struct vty *vty, const char *xpath_base_fmt, ...)
 				"Please check the logs for more details.\n");
 
 			/* Regenerate candidate for consistency. */
-			pthread_rwlock_rdlock(&running_config->lock);
-			{
-				nb_config_replace(vty->candidate_config,
-						  running_config, true);
-			}
-			pthread_rwlock_unlock(&running_config->lock);
-
+			nb_config_replace(vty->candidate_config, running_config,
+					  true);
 			return CMD_WARNING_CONFIG_FAILED;
 		}
 	}
@@ -307,12 +302,7 @@ static int nb_cli_commit(struct vty *vty, bool force,
 
 	/* "confirm" parameter. */
 	if (confirmed_timeout) {
-		pthread_rwlock_rdlock(&running_config->lock);
-		{
-			vty->confirmed_commit_rollback =
-				nb_config_dup(running_config);
-		}
-		pthread_rwlock_unlock(&running_config->lock);
+		vty->confirmed_commit_rollback = nb_config_dup(running_config);
 
 		vty->t_confirmed_commit_timeout = NULL;
 		thread_add_timer(master, nb_cli_confirmed_commit_timeout, vty,
@@ -326,13 +316,8 @@ static int nb_cli_commit(struct vty *vty, bool force,
 	/* Map northbound return code to CLI return code. */
 	switch (ret) {
 	case NB_OK:
-		pthread_rwlock_rdlock(&running_config->lock);
-		{
-			nb_config_replace(vty->candidate_config_base,
-					  running_config, true);
-		}
-		pthread_rwlock_unlock(&running_config->lock);
-
+		nb_config_replace(vty->candidate_config_base, running_config,
+				  true);
 		vty_out(vty,
 			"%% Configuration committed successfully (Transaction ID #%u).\n\n",
 			transaction_id);
@@ -729,12 +714,7 @@ DEFPY (config_update,
 		return CMD_WARNING;
 	}
 
-	pthread_rwlock_rdlock(&running_config->lock);
-	{
-		nb_config_replace(vty->candidate_config_base, running_config,
-				  true);
-	}
-	pthread_rwlock_unlock(&running_config->lock);
+	nb_config_replace(vty->candidate_config_base, running_config, true);
 
 	vty_out(vty, "%% Candidate configuration updated successfully.\n\n");
 
@@ -834,12 +814,8 @@ DEFPY (show_config_running,
 		}
 	}
 
-	pthread_rwlock_rdlock(&running_config->lock);
-	{
-		nb_cli_show_config(vty, running_config, format, translator,
-				   !!with_defaults);
-	}
-	pthread_rwlock_unlock(&running_config->lock);
+	nb_cli_show_config(vty, running_config, format, translator,
+			   !!with_defaults);
 
 	return CMD_SUCCESS;
 }
@@ -953,68 +929,57 @@ DEFPY (show_config_compare,
 	struct nb_config *config2, *config_transaction2 = NULL;
 	int ret = CMD_WARNING;
 
-	/*
-	 * For simplicity, lock the running configuration regardless if it's
-	 * going to be used or not.
-	 */
-	pthread_rwlock_rdlock(&running_config->lock);
-	{
-		if (c1_candidate)
-			config1 = vty->candidate_config;
-		else if (c1_running)
-			config1 = running_config;
-		else {
-			config_transaction1 = nb_db_transaction_load(c1_tid);
-			if (!config_transaction1) {
-				vty_out(vty,
-					"%% Transaction %u does not exist\n\n",
-					(unsigned int)c1_tid);
-				goto exit;
-			}
-			config1 = config_transaction1;
+	if (c1_candidate)
+		config1 = vty->candidate_config;
+	else if (c1_running)
+		config1 = running_config;
+	else {
+		config_transaction1 = nb_db_transaction_load(c1_tid);
+		if (!config_transaction1) {
+			vty_out(vty, "%% Transaction %u does not exist\n\n",
+				(unsigned int)c1_tid);
+			goto exit;
 		}
+		config1 = config_transaction1;
+	}
 
-		if (c2_candidate)
-			config2 = vty->candidate_config;
-		else if (c2_running)
-			config2 = running_config;
-		else {
-			config_transaction2 = nb_db_transaction_load(c2_tid);
-			if (!config_transaction2) {
-				vty_out(vty,
-					"%% Transaction %u does not exist\n\n",
-					(unsigned int)c2_tid);
-				goto exit;
-			}
-			config2 = config_transaction2;
+	if (c2_candidate)
+		config2 = vty->candidate_config;
+	else if (c2_running)
+		config2 = running_config;
+	else {
+		config_transaction2 = nb_db_transaction_load(c2_tid);
+		if (!config_transaction2) {
+			vty_out(vty, "%% Transaction %u does not exist\n\n",
+				(unsigned int)c2_tid);
+			goto exit;
 		}
+		config2 = config_transaction2;
+	}
 
-		if (json)
-			format = NB_CFG_FMT_JSON;
-		else if (xml)
-			format = NB_CFG_FMT_XML;
-		else
-			format = NB_CFG_FMT_CMDS;
+	if (json)
+		format = NB_CFG_FMT_JSON;
+	else if (xml)
+		format = NB_CFG_FMT_XML;
+	else
+		format = NB_CFG_FMT_CMDS;
 
-		if (translator_family) {
-			translator = yang_translator_find(translator_family);
-			if (!translator) {
-				vty_out(vty,
-					"%% Module translator \"%s\" not found\n",
-					translator_family);
-				goto exit;
-			}
+	if (translator_family) {
+		translator = yang_translator_find(translator_family);
+		if (!translator) {
+			vty_out(vty, "%% Module translator \"%s\" not found\n",
+				translator_family);
+			goto exit;
 		}
-
-		ret = nb_cli_show_config_compare(vty, config1, config2, format,
-						 translator);
-	exit:
-		if (config_transaction1)
-			nb_config_free(config_transaction1);
-		if (config_transaction2)
-			nb_config_free(config_transaction2);
 	}
-	pthread_rwlock_unlock(&running_config->lock);
+
+	ret = nb_cli_show_config_compare(vty, config1, config2, format,
+					 translator);
+exit:
+	if (config_transaction1)
+		nb_config_free(config_transaction1);
+	if (config_transaction2)
+		nb_config_free(config_transaction2);
 
 	return ret;
 }
diff --git a/lib/northbound_confd.c b/lib/northbound_confd.c
index e9669fc7e..2fc3c81cf 100644
--- a/lib/northbound_confd.c
+++ b/lib/northbound_confd.c
@@ -289,11 +289,7 @@ static int frr_confd_cdb_read_cb_prepare(int fd, int *subp, int reslen)
 	struct cdb_iter_args iter_args;
 	int ret;
 
-	pthread_rwlock_rdlock(&running_config->lock);
-	{
-		candidate = nb_config_dup(running_config);
-	}
-	pthread_rwlock_unlock(&running_config->lock);
+	candidate = nb_config_dup(running_config);
 
 	/* Iterate over all configuration changes. */
 	iter_args.candidate = candidate;
diff --git a/lib/northbound_grpc.cpp b/lib/northbound_grpc.cpp
index a55da23dd..1d6317b00 100644
--- a/lib/northbound_grpc.cpp
+++ b/lib/northbound_grpc.cpp
@@ -702,15 +702,10 @@ class NorthboundImpl final : public frr::Northbound::Service
 	{
 		struct lyd_node *dnode;
 
-		pthread_rwlock_rdlock(&running_config->lock);
-		{
-			dnode = yang_dnode_get(running_config->dnode,
-					       path.empty() ? NULL
-							    : path.c_str());
-			if (dnode)
-				dnode = yang_dnode_dup(dnode);
-		}
-		pthread_rwlock_unlock(&running_config->lock);
+		dnode = yang_dnode_get(running_config->dnode,
+				       path.empty() ? NULL : path.c_str());
+		if (dnode)
+			dnode = yang_dnode_dup(dnode);
 
 		return dnode;
 	}
@@ -817,11 +812,7 @@ class NorthboundImpl final : public frr::Northbound::Service
 
 		struct candidate *candidate = &_candidates[candidate_id];
 		candidate->id = candidate_id;
-		pthread_rwlock_rdlock(&running_config->lock);
-		{
-			candidate->config = nb_config_dup(running_config);
-		}
-		pthread_rwlock_unlock(&running_config->lock);
+		candidate->config = nb_config_dup(running_config);
 		candidate->transaction = NULL;
 
 		return candidate;
diff --git a/lib/northbound_sysrepo.c b/lib/northbound_sysrepo.c
index 77183282b..b94c93976 100644
--- a/lib/northbound_sysrepo.c
+++ b/lib/northbound_sysrepo.c
@@ -256,11 +256,7 @@ static int frr_sr_config_change_cb_verify(sr_session_ctx_t *session,
 		return ret;
 	}
 
-	pthread_rwlock_rdlock(&running_config->lock);
-	{
-		candidate = nb_config_dup(running_config);
-	}
-	pthread_rwlock_unlock(&running_config->lock);
+	candidate = nb_config_dup(running_config);
 
 	while ((ret = sr_get_change_next(session, it, &sr_op, &sr_old_val,
 					 &sr_new_val))
diff --git a/lib/vty.c b/lib/vty.c
index deb9391bd..502d2c9d0 100644
--- a/lib/vty.c
+++ b/lib/vty.c
@@ -2582,22 +2582,17 @@ int vty_config_enter(struct vty *vty, bool private_config, bool exclusive)
 	vty->private_config = private_config;
 	vty->xpath_index = 0;
 
-	pthread_rwlock_rdlock(&running_config->lock);
-	{
-		if (private_config) {
-			vty->candidate_config = nb_config_dup(running_config);
+	if (private_config) {
+		vty->candidate_config = nb_config_dup(running_config);
+		vty->candidate_config_base = nb_config_dup(running_config);
+		vty_out(vty,
+			"Warning: uncommitted changes will be discarded on exit.\n\n");
+	} else {
+		vty->candidate_config = vty_shared_candidate_config;
+		if (frr_get_cli_mode() == FRR_CLI_TRANSACTIONAL)
 			vty->candidate_config_base =
 				nb_config_dup(running_config);
-			vty_out(vty,
-				"Warning: uncommitted changes will be discarded on exit.\n\n");
-		} else {
-			vty->candidate_config = vty_shared_candidate_config;
-			if (frr_get_cli_mode() == FRR_CLI_TRANSACTIONAL)
-				vty->candidate_config_base =
-					nb_config_dup(running_config);
-		}
 	}
-	pthread_rwlock_unlock(&running_config->lock);
 
 	return CMD_SUCCESS;
 }
diff --git a/ripd/ripd.c b/ripd/ripd.c
index cd2ddb1eb..1b5a582cb 100644
--- a/ripd/ripd.c
+++ b/ripd/ripd.c
@@ -3660,18 +3660,14 @@ static int rip_vrf_enable(struct vrf *vrf)
 		if (yang_module_find("frr-ripd") && old_vrf_name) {
 			struct lyd_node *rip_dnode;
 
-			pthread_rwlock_wrlock(&running_config->lock);
-			{
-				rip_dnode = yang_dnode_get(
-						   running_config->dnode,
-						   "/frr-ripd:ripd/instance[vrf='%s']/vrf",
-						   old_vrf_name);
-				if (rip_dnode) {
-					yang_dnode_change_leaf(rip_dnode, vrf->name);
-					running_config->version++;
-				}
+			rip_dnode = yang_dnode_get(
+				running_config->dnode,
+				"/frr-ripd:ripd/instance[vrf='%s']/vrf",
+				old_vrf_name);
+			if (rip_dnode) {
+				yang_dnode_change_leaf(rip_dnode, vrf->name);
+				running_config->version++;
 			}
-			pthread_rwlock_unlock(&running_config->lock);
 		}
 		if (old_vrf_name)
 			XFREE(MTYPE_RIP_VRF_NAME, old_vrf_name);
diff --git a/ripngd/ripngd.c b/ripngd/ripngd.c
index 120f46f0d..ef4e47473 100644
--- a/ripngd/ripngd.c
+++ b/ripngd/ripngd.c
@@ -2795,18 +2795,14 @@ static int ripng_vrf_enable(struct vrf *vrf)
 		if (yang_module_find("frr-ripngd") && old_vrf_name) {
 			struct lyd_node *ripng_dnode;
 
-			pthread_rwlock_wrlock(&running_config->lock);
-			{
-				ripng_dnode = yang_dnode_get(
-						   running_config->dnode,
-						   "/frr-ripngd:ripngd/instance[vrf='%s']/vrf",
-						   old_vrf_name);
-				if (ripng_dnode) {
-					yang_dnode_change_leaf(ripng_dnode, vrf->name);
-					running_config->version++;
-				}
+			ripng_dnode = yang_dnode_get(
+				running_config->dnode,
+				"/frr-ripngd:ripngd/instance[vrf='%s']/vrf",
+				old_vrf_name);
+			if (ripng_dnode) {
+				yang_dnode_change_leaf(ripng_dnode, vrf->name);
+				running_config->version++;
 			}
-			pthread_rwlock_unlock(&running_config->lock);
 		}
 		if (old_vrf_name)
 			XFREE(MTYPE_RIPNG_VRF_NAME, old_vrf_name);