Hello,

Add wrappers for isc_task_*exclusive().

This patch replaces scattered isc_task_* calls and associated locking to one place. It helps with debugging sometimes.

--
Petr^2 Spacek
From cd44761094c9c9f2d2d9991195e866f3a361fb36 Mon Sep 17 00:00:00 2001
From: Petr Spacek <pspa...@redhat.com>
Date: Wed, 4 Jun 2014 17:19:11 +0200
Subject: [PATCH] Add wrappers for isc_task_*exclusive().

Signed-off-by: Petr Spacek <pspa...@redhat.com>
---
 src/Makefile.am   |  2 ++
 src/ldap_helper.c | 36 ++++++++++--------------------
 src/lock.c        | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/lock.h        | 32 +++++++++++++++++++++++++++
 src/settings.c    | 17 ++++++--------
 5 files changed, 118 insertions(+), 35 deletions(-)
 create mode 100644 src/lock.c
 create mode 100644 src/lock.h

diff --git a/src/Makefile.am b/src/Makefile.am
index 6856957f48dbe32750009ab8a32487add05d5c1c..4cccabab285b43e9e76bd3cca0184d4d87941e8a 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -11,6 +11,7 @@ HDRS =				\
 	ldap_driver.h		\
 	ldap_entry.h		\
 	ldap_helper.h		\
+	lock.h			\
 	log.h			\
 	rbt_helper.h		\
 	rdlist.h		\
@@ -33,6 +34,7 @@ ldap_la_SOURCES =		\
 	ldap_driver.c		\
 	ldap_entry.c		\
 	ldap_helper.c		\
+	lock.c			\
 	log.c			\
 	rbt_helper.c		\
 	rdlist.c		\
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index 7b8ea86b3f8fccdbcebc6ee50aaad8438ebb984b..7b09e2376706230d96d20f0d1bf005d0a16ee2fa 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -76,6 +76,7 @@
 #include "ldap_driver.h"
 #include "ldap_entry.h"
 #include "ldap_helper.h"
+#include "lock.h"
 #include "log.h"
 #include "rdlist.h"
 #include "semaphore.h"
@@ -1048,7 +1049,7 @@ publish_zone(isc_task_t *task, ldap_instance_t *inst, dns_zone_t *zone)
 	isc_boolean_t freeze = ISC_FALSE;
 	dns_zone_t *zone_in_view = NULL;
 	dns_view_t *view_in_zone = NULL;
-	isc_boolean_t unlock = ISC_FALSE;
+	isc_result_t lock_state = ISC_R_IGNORE;
 
 	REQUIRE(ISCAPI_TASK_VALID(task));
 	REQUIRE(inst != NULL);
@@ -1078,11 +1079,7 @@ publish_zone(isc_task_t *task, ldap_instance_t *inst, dns_zone_t *zone)
 	} /* else if (zone_in_view == NULL && view_in_zone == NULL)
 	     Publish the zone. */
 
-	result = isc_task_beginexclusive(task);
-	RUNTIME_CHECK(result == ISC_R_SUCCESS ||
-		      result == ISC_R_LOCKBUSY);
-	unlock = (result == ISC_R_SUCCESS);
-
+	run_exclusive_enter(task, &lock_state);
 	if (inst->view->frozen) {
 		freeze = ISC_TRUE;
 		dns_view_thaw(inst->view);
@@ -1096,8 +1093,7 @@ cleanup:
 		dns_zone_detach(&zone_in_view);
 	if (freeze)
 		dns_view_freeze(inst->view);
-	if (unlock)
-		isc_task_endexclusive(task);
+	run_exclusive_exit(task, lock_state);
 
 	return result;
 }
@@ -1281,22 +1277,17 @@ ldap_delete_zone2(ldap_instance_t *inst, dns_name_t *name, isc_boolean_t lock,
 {
 	isc_result_t result;
 	isc_result_t isforward = ISC_R_NOTFOUND;
-	isc_boolean_t unlock = ISC_FALSE;
+	isc_result_t lock_state = ISC_R_IGNORE;
 	isc_boolean_t freeze = ISC_FALSE;
 	dns_zone_t *raw = NULL;
 	dns_zone_t *secure = NULL;
 	dns_zone_t *foundzone = NULL;
 	char zone_name_char[DNS_NAME_FORMATSIZE];
 
 	dns_name_format(name, zone_name_char, DNS_NAME_FORMATSIZE);
 	log_debug(1, "deleting zone '%s'", zone_name_char);
-	if (lock) {
-		result = isc_task_beginexclusive(inst->task);
-		RUNTIME_CHECK(result == ISC_R_SUCCESS ||
-			      result == ISC_R_LOCKBUSY);
-		if (result == ISC_R_SUCCESS)
-			unlock = ISC_TRUE;
-	}
+	if (lock)
+		run_exclusive_enter(inst->task, &lock_state);
 
 	if (!preserve_forwarding) {
 		CHECK(delete_forwarding_table(inst, name, "zone",
@@ -1339,8 +1330,7 @@ ldap_delete_zone2(ldap_instance_t *inst, dns_name_t *name, isc_boolean_t lock,
 cleanup:
 	if (freeze)
 		dns_view_freeze(inst->view);
-	if (unlock)
-		isc_task_endexclusive(inst->task);
+	run_exclusive_exit(inst->task, lock_state);
 
 	return result;
 }
@@ -2215,7 +2205,7 @@ ldap_parse_master_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst,
 	dns_zone_t *secure = NULL;
 	dns_zone_t *toview = NULL;
 	isc_result_t result;
-	isc_boolean_t unlock = ISC_FALSE;
+	isc_result_t lock_state = ISC_R_IGNORE;
 	isc_boolean_t new_zone = ISC_FALSE;
 	isc_boolean_t want_secure = ISC_FALSE;
 	isc_boolean_t configured = ISC_FALSE;
@@ -2242,10 +2232,7 @@ ldap_parse_master_zoneentry(ldap_entry_t *entry, ldap_instance_t *inst,
 	dn = entry->dn;
 	CHECK(dn_to_dnsname(inst->mctx, dn, &name, NULL));
 
-	result = isc_task_beginexclusive(task);
-	RUNTIME_CHECK(result == ISC_R_SUCCESS || result == ISC_R_LOCKBUSY);
-	if (result == ISC_R_SUCCESS)
-		unlock = ISC_TRUE;
+	run_exclusive_enter(task, &lock_state);
 
 	/*
 	 * TODO: Remove this hack, most probably before Fedora 20.
@@ -2363,8 +2350,7 @@ cleanup:
 		if (result != ISC_R_SUCCESS)
 			log_error_r("zone '%s': rollback failed: ", entry->dn);
 	}
-	if (unlock)
-		isc_task_endexclusive(task);
+	run_exclusive_exit(task, lock_state);
 	if (dns_name_dynamic(&name))
 		dns_name_free(&name, inst->mctx);
 	if (raw != NULL)
diff --git a/src/lock.c b/src/lock.c
new file mode 100644
index 0000000000000000000000000000000000000000..7099dd36808f1993e0fce666163ddc9ad1b8e1d3
--- /dev/null
+++ b/src/lock.c
@@ -0,0 +1,66 @@
+/*
+ * Authors: Petr Spacek <pspa...@redhat.com>
+ *
+ * Copyright (C) 2014 Red Hat
+ * see file 'COPYING' for use and warranty information
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; version 2 or later
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#include <isc/task.h>
+#include <isc/util.h>
+
+#include "lock.h"
+
+/**
+ * Lock BIND dispatcher and allow only single task to run. This function
+ * blocks until task-exclusive mode is entered.
+ *
+ * Recursive locking is allowed. Auxiliary variable pointed to by "statep"
+ * stores information if last run_exclusive_enter() operation really locked
+ * something or if the lock was called recursively and was no-op.
+ *
+ * The pair (task, state) used for run_exclusive_enter() has to be
+ * used for run_exclusive_exit().
+ *
+ * @param[in]  	  task   The only task allowed to run.
+ * @param[in,out] statep Lock state: ISC_R_SUCCESS or ISC_R_LOCKBUSY
+ */
+void
+run_exclusive_enter(isc_task_t *task, isc_result_t *statep)
+{
+	REQUIRE(statep != NULL);
+	REQUIRE(*statep == ISC_R_IGNORE);
+
+	*statep = isc_task_beginexclusive(task);
+	RUNTIME_CHECK(*statep == ISC_R_SUCCESS || *statep == ISC_R_LOCKBUSY);
+}
+
+/**
+ * Exit task-exclusive mode.
+ *
+ * @param task[in]  The only task allowed to run at the moment.
+ * @param state[in] Lock state as returned by run_exclusive_enter().
+ */
+void
+run_exclusive_exit(isc_task_t *task, isc_result_t state)
+{
+	if (state == ISC_R_SUCCESS)
+		isc_task_endexclusive(task);
+	else
+		/* Unlocking recursive lock or the lock was never locked. */
+		INSIST(state == ISC_R_LOCKBUSY || state == ISC_R_IGNORE);
+
+	return;
+}
diff --git a/src/lock.h b/src/lock.h
new file mode 100644
index 0000000000000000000000000000000000000000..38ce8bf7b41b4205b24e1943f17b0f46db4d9db3
--- /dev/null
+++ b/src/lock.h
@@ -0,0 +1,32 @@
+/*
+ * Authors: Petr Spacek <pspa...@redhat.com>
+ *
+ * Copyright (C) 2013 Red Hat
+ * see file 'COPYING' for use and warranty information
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; version 2 or later
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#ifndef LOCK_H_
+#define LOCK_H_
+
+#include "util.h"
+
+void ATTR_NONNULLS
+run_exclusive_enter(isc_task_t *task, isc_result_t *statep);
+
+void ATTR_NONNULLS
+run_exclusive_exit(isc_task_t *task, isc_result_t state);
+
+#endif /* LOCK_H_ */
diff --git a/src/settings.c b/src/settings.c
index f6696b9c4eeab5f3c143158208dc4b0abe1d28e4..c38c15d6c6b32dee9125951cd5de1c9c02ed5733 100644
--- a/src/settings.c
+++ b/src/settings.c
@@ -32,6 +32,7 @@
 #include <string.h>
 #include <strings.h>
 
+#include "lock.h"
 #include "log.h"
 #include "settings.h"
 #include "str.h"
@@ -234,7 +235,7 @@ set_value(isc_mem_t *mctx, setting_t *setting, const char *value,
 	  isc_task_t *task)
 {
 	isc_result_t result;
-	isc_result_t lock = ISC_R_IGNORE;
+	isc_result_t lock_state = ISC_R_IGNORE;
 	isc_uint32_t numeric_value;
 	isc_uint32_t len;
 
@@ -289,8 +290,7 @@ set_value(isc_mem_t *mctx, setting_t *setting, const char *value,
 	}
 
 	/* Switch to single thread mode and write new value. */
-	lock = isc_task_beginexclusive(task);
-	RUNTIME_CHECK(lock == ISC_R_SUCCESS || lock == ISC_R_LOCKBUSY);
+	run_exclusive_enter(task, &lock_state);
 
 	switch (setting->type) {
 	case ST_STRING:
@@ -318,8 +318,7 @@ set_value(isc_mem_t *mctx, setting_t *setting, const char *value,
 	result = ISC_R_SUCCESS;
 
 cleanup:
-	if (lock == ISC_R_SUCCESS)
-		isc_task_endexclusive(task);
+	run_exclusive_exit(task, lock_state);
 	return result;
 }
 
@@ -377,18 +376,17 @@ setting_unset(const char *const name, const settings_set_t *set,
 	      isc_task_t *task)
 {
 	isc_result_t result;
-	isc_result_t lock = ISC_R_IGNORE;
+	isc_result_t lock_state = ISC_R_IGNORE;
 	setting_t *setting = NULL;
 
 	REQUIRE(task != NULL);
 
 	CHECK(setting_find(name, set, ISC_FALSE, ISC_FALSE, &setting));
 
 	if (!setting->filled)
 		return ISC_R_IGNORE;
 
-	lock = isc_task_beginexclusive(task);
-	RUNTIME_CHECK(lock == ISC_R_SUCCESS || lock == ISC_R_LOCKBUSY);
+	run_exclusive_enter(task, &lock_state);
 
 	switch (setting->type) {
 	case ST_STRING:
@@ -408,8 +406,7 @@ setting_unset(const char *const name, const settings_set_t *set,
 	setting->filled = 0;
 
 cleanup:
-	if (lock == ISC_R_SUCCESS)
-		isc_task_endexclusive(task);
+	run_exclusive_exit(task, lock_state);
 	if (result == ISC_R_NOTFOUND)
 		log_bug("setting '%s' was not found in set of settings '%s'",
 			name, set->name);
-- 
1.9.3

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to