6552729 db handle should be reused instead of opened per-request

6588930 idmapd crashes in processing AD LDAP response

6 files changed, 276 insertions, 137 deletions

diff --git a/usr/src/cmd/idmap/idmapd/adutils.c b/usr/src/cmd/idmap/idmapd/adutils.c
index 279179e13b..4cfa56e073 100644
--- a/usr/src/cmd/idmap/idmapd/adutils.c
+++ b/usr/src/cmd/idmap/idmapd/adutils.c
@@ -139,6 +139,8 @@ typedef struct idmap_q {
 struct idmap_query_state {
 	idmap_query_state_t	*next;
 	int			qcount;		/* how many queries */
+	int			ref_cnt;	/* reference count */
+	pthread_cond_t		cv;		/* Condition wait variable */
 	uint32_t		qlastsent;
 	uint32_t		qinflight;	/* how many queries in flight */
 	uint16_t		qdead;		/* oops, lost LDAP connection */
@@ -162,6 +164,12 @@ static ad_host_t	*host_head = NULL;
 static pthread_t	reaperid = 0;
 static pthread_mutex_t	adhostlock = PTHREAD_MUTEX_INITIALIZER;
 
+
+static void
+idmap_lookup_unlock_batch(idmap_query_state_t **state);
+
+
+
 /*ARGSUSED*/
 static int
 idmap_saslcallback(LDAP *ld, unsigned flags, void *defaults, void *prompts) {
@@ -943,11 +951,13 @@ idmap_lookup_batch_start(ad_t *ad, int nqueries, idmap_query_state_t **state)
 	if (new_state == NULL)
 		return (IDMAP_ERR_MEMORY);
 
+	new_state->ref_cnt = 1;
 	new_state->qadh = adh;
 	new_state->qcount = nqueries;
 	new_state->qadh_gen = adh->generation;
 	/* should be -1, but the atomic routines want unsigned */
 	new_state->qlastsent = 0;
+	(void) pthread_cond_init(&new_state->cv, NULL);
 
 	(void) pthread_mutex_lock(&qstatelock);
 	new_state->next = qstatehead;
@@ -961,7 +971,9 @@ idmap_lookup_batch_start(ad_t *ad, int nqueries, idmap_query_state_t **state)
 
 /*
  * Find the idmap_query_state_t to which a given LDAP result msgid on a
- * given connection belongs
+ * given connection belongs. This routine increaments the reference count
+ * so that the object can not be freed. idmap_lookup_unlock_batch()
+ * must be called to decreament the reference count.
  */
 static
 int
@@ -977,6 +989,7 @@ idmap_msgid2query(ad_host_t *adh, int msgid,
 			continue;
 		for (i = 0; i < p->qcount; i++) {
 			if ((p->queries[i]).msgid == msgid) {
+				p->ref_cnt++;
 				*state = p;
 				*qid = i;
 				(void) pthread_mutex_unlock(&qstatelock);
@@ -1215,6 +1228,7 @@ idmap_get_adobject_batch(ad_host_t *adh, struct timeval *timeout)
 			atomic_dec_32(&query_state->qinflight);
 			/* we saw at least one reply */
 			query_state->queries[qid].got_reply = 1;
+			idmap_lookup_unlock_batch(&query_state);
 		}
 		(void) ldap_msgfree(res);
 		ret = 0;
@@ -1238,6 +1252,7 @@ idmap_get_adobject_batch(ad_host_t *adh, struct timeval *timeout)
 			/* we saw at least one result */
 			query_state->queries[qid].got_reply = 1;
 			query_state->queries[qid].got_results = 1;
+			idmap_lookup_unlock_batch(&query_state);
 		}
 		(void) ldap_msgfree(res);
 		ret = 0;
@@ -1251,15 +1266,48 @@ idmap_get_adobject_batch(ad_host_t *adh, struct timeval *timeout)
 	return (ret);
 }
 
+/*
+ * This routine decreament the reference count of the
+ * idmap_query_state_t
+ */
+static void
+idmap_lookup_unlock_batch(idmap_query_state_t **state)
+{
+	/*
+	 * Decrement reference count with qstatelock locked
+	 */
+	(void) pthread_mutex_lock(&qstatelock);
+	(*state)->ref_cnt--;
+	/*
+	 * If there are no references wakup the allocating thread
+	 */
+	if ((*state)->ref_cnt == 0)
+		(void) pthread_cond_signal(&(*state)->cv);
+	(void) pthread_mutex_unlock(&qstatelock);
+	*state = NULL;
+}
+
+/*
+ * This routine frees the idmap_query_state_t structure
+ * If the reference count is greater than 1 it waits
+ * for the other threads to finish using it.
+ */
 void
-idmap_lookup_free_batch(idmap_query_state_t **state)
+idmap_lookup_release_batch(idmap_query_state_t **state)
 {
 	idmap_query_state_t **p;
 
-	idmap_release_conn((*state)->qadh);
+	/*
+	 * Decrement reference count with qstatelock locked
+	 * and wait for reference count to get to zero
+	 */
+	(void) pthread_mutex_lock(&qstatelock);
+	(*state)->ref_cnt--;
+	while ((*state)->ref_cnt > 0) {
+		(void) pthread_cond_wait(&(*state)->cv, &qstatelock);
+	}
 
 	/* Remove this state struct from the list of state structs */
-	(void) pthread_mutex_lock(&qstatelock);
 	for (p = &qstatehead; *p != NULL; p = &(*p)->next) {
 		if (*p == (*state)) {
 			*p = (*state)->next;
@@ -1268,6 +1316,10 @@ idmap_lookup_free_batch(idmap_query_state_t **state)
 	}
 	(void) pthread_mutex_unlock(&qstatelock);
 
+	(void) pthread_cond_destroy(&(*state)->cv);
+
+	idmap_release_conn((*state)->qadh);
+
 	free(*state);
 	*state = NULL;
 }
@@ -1307,7 +1359,7 @@ idmap_lookup_batch_end(idmap_query_state_t **state,
 		}
 	}
 
-	idmap_lookup_free_batch(state);
+	idmap_lookup_release_batch(state);
 
 	return (retcode);
 }
diff --git a/usr/src/cmd/idmap/idmapd/adutils.h b/usr/src/cmd/idmap/idmapd/adutils.h
index bbee846352..707ccddd2b 100644
--- a/usr/src/cmd/idmap/idmapd/adutils.h
+++ b/usr/src/cmd/idmap/idmapd/adutils.h
@@ -139,7 +139,7 @@ idmap_retcode idmap_lookup_batch_end(idmap_query_state_t **state,
 		struct timeval *timeout);
 
 /* Abandon a batch and release its idmap_query_state_t object */
-void idmap_lookup_free_batch(idmap_query_state_t **state);
+void idmap_lookup_release_batch(idmap_query_state_t **state);
 
 /*
  * Add a name->SID lookup
diff --git a/usr/src/cmd/idmap/idmapd/dbutils.c b/usr/src/cmd/idmap/idmapd/dbutils.c
index 7a014e91c7..88db3c7b86 100644
--- a/usr/src/cmd/idmap/idmapd/dbutils.c
+++ b/usr/src/cmd/idmap/idmapd/dbutils.c
@@ -40,12 +40,15 @@
 #include <time.h>
 #include <pwd.h>
 #include <grp.h>
+#include <pthread.h>
+#include <assert.h>
 
 #include "idmapd.h"
 #include "adutils.h"
 #include "string.h"
 #include "idmap_priv.h"
 
+
 static idmap_retcode sql_compile_n_step_once(sqlite *, char *,
 		sqlite_vm **, int *, int, const char ***);
 static idmap_retcode lookup_wksids_name2sid(const char *, char **,
@@ -65,12 +68,6 @@ static idmap_retcode lookup_wksids_name2sid(const char *, char **,
 
 #define	LOCALRID_MIN	1000
 
-#define	SLEEP_TIME	20
-
-#define	NANO_SLEEP(rqtp, nsec)\
-	rqtp.tv_sec = 0;\
-	rqtp.tv_nsec = nsec * (NANOSEC / MILLISEC);\
-	(void) nanosleep(&rqtp, NULL);
 
 
 typedef enum init_db_option {
@@ -78,6 +75,87 @@ typedef enum init_db_option {
 	REMOVE_IF_CORRUPT = 1
 } init_db_option_t;
 
+/*
+ * Thread specfic data to hold the database handles so that the
+ * databaes are not opened and closed for every request. It also
+ * contains the sqlite busy handler structure.
+ */
+
+struct idmap_busy {
+	const char *name;
+	const int *delays;
+	int delay_size;
+	int total;
+	int sec;
+};
+
+
+typedef struct idmap_tsd {
+	sqlite *db_db;
+	sqlite *cache_db;
+	struct idmap_busy cache_busy;
+	struct idmap_busy db_busy;
+} idmap_tsd_t;
+
+
+
+static const int cache_delay_table[] =
+		{ 1, 2, 5, 10, 15, 20, 25, 30,  35,  40,
+		50,  50, 60, 70, 80, 90, 100};
+
+static const int db_delay_table[] =
+		{ 5, 10, 15, 20, 30,  40,  55,  70, 100};
+
+
+static pthread_key_t	idmap_tsd_key;
+
+void
+idmap_tsd_destroy(void *key)
+{
+
+	idmap_tsd_t	*tsd = (idmap_tsd_t *)key;
+	if (tsd) {
+		if (tsd->db_db)
+			(void) sqlite_close(tsd->db_db);
+		if (tsd->cache_db)
+			(void) sqlite_close(tsd->cache_db);
+		free(tsd);
+	}
+}
+
+int
+idmap_init_tsd_key(void) {
+
+	return (pthread_key_create(&idmap_tsd_key, idmap_tsd_destroy));
+}
+
+
+
+idmap_tsd_t *
+idmap_get_tsd(void)
+{
+	idmap_tsd_t	*tsd;
+
+	if ((tsd = pthread_getspecific(idmap_tsd_key)) == NULL) {
+		/* No thread specific data so create it */
+		if ((tsd = malloc(sizeof (*tsd))) != NULL) {
+			/* Initialize thread specific data */
+			(void) memset(tsd, 0, sizeof (*tsd));
+			/* save the trhread specific data */
+			if (pthread_setspecific(idmap_tsd_key, tsd) != 0) {
+				/* Can't store key */
+				free(tsd);
+				tsd = NULL;
+			}
+		} else {
+			tsd = NULL;
+		}
+	}
+
+	return (tsd);
+}
+
+
 
 /*
  * Initialize 'dbname' using 'sql'
@@ -160,26 +238,79 @@ init_db_instance(const char *dbname, const char *sql, init_db_option_t opt,
 	return (rc);
 }
 
+
+/*
+ * This is the SQLite database busy handler that retries the SQL
+ * operation until it is successful.
+ */
+int
+/* LINTED E_FUNC_ARG_UNUSED */
+idmap_sqlite_busy_handler(void *arg, const char *table_name, int count)
+{
+	struct idmap_busy	*busy = arg;
+	int			delay;
+	struct timespec		rqtp;
+
+	if (count == 1)  {
+		busy->total = 0;
+		busy->sec = 2;
+	}
+	if (busy->total > 1000 * busy->sec) {
+		idmapdlog(LOG_ERR,
+		    "Thread %d waited %d sec for the %s database",
+		    pthread_self(), busy->sec, busy->name);
+		busy->sec++;
+	}
+
+	if (count <= busy->delay_size) {
+		delay = busy->delays[count-1];
+	} else {
+		delay = busy->delays[busy->delay_size - 1];
+	}
+	busy->total += delay;
+	rqtp.tv_sec = 0;
+	rqtp.tv_nsec = delay * (NANOSEC / MILLISEC);
+	(void) nanosleep(&rqtp, NULL);
+	return (1);
+}
+
+
 /*
  * Get the database handle
  */
 idmap_retcode
 get_db_handle(sqlite **db) {
 	char	*errmsg;
+	idmap_tsd_t *tsd;
 
 	/*
-	 * TBD RFE: Retrieve the db handle from thread-specific storage
+	 * Retrieve the db handle from thread-specific storage
 	 * If none exists, open and store in thread-specific storage.
 	 */
-
-	*db = sqlite_open(IDMAP_DBNAME, 0, &errmsg);
-	if (*db == NULL) {
+	if ((tsd = idmap_get_tsd()) == NULL) {
 		idmapdlog(LOG_ERR,
-			"Error opening database %s (%s)",
-			IDMAP_DBNAME, CHECK_NULL(errmsg));
-		sqlite_freemem(errmsg);
-		return (IDMAP_ERR_INTERNAL);
+			"Error getting thread specific data for %s",
+			IDMAP_DBNAME);
+		return (IDMAP_ERR_MEMORY);
 	}
+
+	if (tsd->db_db == NULL) {
+		tsd->db_db = sqlite_open(IDMAP_DBNAME, 0, &errmsg);
+		if (tsd->db_db == NULL) {
+			idmapdlog(LOG_ERR,
+				"Error opening database %s (%s)",
+				IDMAP_DBNAME, CHECK_NULL(errmsg));
+			sqlite_freemem(errmsg);
+			return (IDMAP_ERR_INTERNAL);
+		}
+		tsd->db_busy.name = IDMAP_DBNAME;
+		tsd->db_busy.delays = db_delay_table;
+		tsd->db_busy.delay_size = sizeof (db_delay_table) /
+		    sizeof (int);
+		sqlite_busy_handler(tsd->db_db, idmap_sqlite_busy_handler,
+		    &tsd->db_busy);
+	}
+	*db = tsd->db_db;
 	return (IDMAP_SUCCESS);
 }
 
@@ -187,22 +318,38 @@ get_db_handle(sqlite **db) {
  * Get the cache handle
  */
 idmap_retcode
-get_cache_handle(sqlite **db) {
+get_cache_handle(sqlite **cache) {
 	char	*errmsg;
+	idmap_tsd_t *tsd;
 
 	/*
-	 * TBD RFE: Retrieve the db handle from thread-specific storage
+	 * Retrieve the db handle from thread-specific storage
 	 * If none exists, open and store in thread-specific storage.
 	 */
-
-	*db = sqlite_open(IDMAP_CACHENAME, 0, &errmsg);
-	if (*db == NULL) {
+	if ((tsd = idmap_get_tsd()) == NULL) {
 		idmapdlog(LOG_ERR,
-			"Error opening database %s (%s)",
-			IDMAP_CACHENAME, CHECK_NULL(errmsg));
-		sqlite_freemem(errmsg);
-		return (IDMAP_ERR_INTERNAL);
+			"Error getting thread specific data for %s",
+			IDMAP_DBNAME);
+		return (IDMAP_ERR_MEMORY);
 	}
+
+	if (tsd->cache_db == NULL) {
+		tsd->cache_db = sqlite_open(IDMAP_CACHENAME, 0, &errmsg);
+		if (tsd->cache_db == NULL) {
+			idmapdlog(LOG_ERR,
+				"Error opening database %s (%s)",
+				IDMAP_CACHENAME, CHECK_NULL(errmsg));
+			sqlite_freemem(errmsg);
+			return (IDMAP_ERR_INTERNAL);
+		}
+		tsd->cache_busy.name = IDMAP_CACHENAME;
+		tsd->cache_busy.delays = cache_delay_table;
+		tsd->cache_busy.delay_size = sizeof (cache_delay_table) /
+		    sizeof (int);
+		sqlite_busy_handler(tsd->cache_db, idmap_sqlite_busy_handler,
+		    &tsd->cache_busy);
+	}
+	*cache = tsd->cache_db;
 	return (IDMAP_SUCCESS);
 }
 
@@ -305,31 +452,16 @@ idmapd_string2stat(const char *msg) {
 idmap_retcode
 sql_exec_no_cb(sqlite *db, char *sql) {
 	char		*errmsg = NULL;
-	int		r, i, s;
-	struct timespec	rqtp;
+	int		r;
 	idmap_retcode	retcode;
 
-	for (i = 0, s = SLEEP_TIME; i < MAX_TRIES; i++, s *= 2) {
-		if (errmsg != NULL) {
-			sqlite_freemem(errmsg);
-			errmsg = NULL;
-		}
-		r = sqlite_exec(db, sql, NULL, NULL, &errmsg);
-		if (r != SQLITE_BUSY)
-			break;
-		NANO_SLEEP(rqtp, s);
-	}
+	r = sqlite_exec(db, sql, NULL, NULL, &errmsg);
+	assert(r != SQLITE_LOCKED && r != SQLITE_BUSY);
 
 	if (r != SQLITE_OK) {
 		idmapdlog(LOG_ERR, "Database error during %s (%s)",
 			sql, CHECK_NULL(errmsg));
-		if (r == SQLITE_BUSY) {
-			retcode = IDMAP_ERR_BUSY;
-		} else {
-			retcode = idmap_string2stat(errmsg);
-			if (retcode == IDMAP_ERR_OTHER)
-				retcode = idmapd_string2stat(errmsg);
-		}
+		retcode = idmapd_string2stat(errmsg);
 		if (errmsg != NULL)
 			sqlite_freemem(errmsg);
 		return (retcode);
@@ -382,40 +514,28 @@ process_list_svc_sql(sqlite *db, char *sql, uint64_t limit,
 		list_svc_cb cb, void *result) {
 	list_cb_data_t	cb_data;
 	char		*errmsg = NULL;
-	int		r, i, s;
-	struct timespec	rqtp;
+	int		r;
 	idmap_retcode	retcode = IDMAP_ERR_INTERNAL;
 
 	(void) memset(&cb_data, 0, sizeof (cb_data));
 	cb_data.result = result;
 	cb_data.limit = limit;
 
-	for (i = 0, s = SLEEP_TIME; i < MAX_TRIES; i++, s *= 2) {
-		r = sqlite_exec(db, sql, cb, &cb_data, &errmsg);
-		switch (r) {
-		case SQLITE_OK:
-			retcode = IDMAP_SUCCESS;
-			goto out;
-		case SQLITE_BUSY:
-			if (errmsg != NULL) {
-				sqlite_freemem(errmsg);
-				errmsg = NULL;
-			}
-			retcode = IDMAP_ERR_BUSY;
-			idmapdlog(LOG_DEBUG,
-			"Database busy, %d retries remaining",
-				MAX_TRIES - i - 1);
-			NANO_SLEEP(rqtp, s);
-			continue;
-		default:
-			retcode = IDMAP_ERR_INTERNAL;
-			idmapdlog(LOG_ERR,
-				"Database error during %s (%s)",
-				sql, CHECK_NULL(errmsg));
-			goto out;
-		};
+
+	r = sqlite_exec(db, sql, cb, &cb_data, &errmsg);
+	assert(r != SQLITE_LOCKED && r != SQLITE_BUSY);
+	switch (r) {
+	case SQLITE_OK:
+		retcode = IDMAP_SUCCESS;
+		break;
+
+	default:
+		retcode = IDMAP_ERR_INTERNAL;
+		idmapdlog(LOG_ERR,
+			"Database error during %s (%s)",
+			sql, CHECK_NULL(errmsg));
+		break;
 	}
-out:
 	if (errmsg != NULL)
 		sqlite_freemem(errmsg);
 	return (retcode);
@@ -611,7 +731,7 @@ add_namerule(sqlite *db, idmap_namerule *rule) {
 		else
 			dom = "";
 	}
-	sql = sqlite_mprintf("INSERT OR ROLLBACK into namerules "
+	sql = sqlite_mprintf("INSERT into namerules "
 		"(is_user, windomain, winname, is_nt4, "
 		"unixname, w2u_order, u2w_order) "
 		"VALUES(%d, %Q, %Q, %d, %Q, %q, %q);",
@@ -762,7 +882,6 @@ out:
  *
  * Return values:
  * IDMAP_SUCCESS
- * IDMAP_ERR_BUSY
  * IDMAP_ERR_NOTFOUND
  * IDMAP_ERR_INTERNAL
  */
@@ -771,10 +890,9 @@ static idmap_retcode
 sql_compile_n_step_once(sqlite *db, char *sql, sqlite_vm **vm, int *ncol,
 		int reqcol, const char ***values) {
 	char		*errmsg = NULL;
-	struct timespec	rqtp;
-	int		i, r, s;
+	int		r;
 
-	if (sqlite_compile(db, sql, NULL, vm, &errmsg) != SQLITE_OK) {
+	if ((r = sqlite_compile(db, sql, NULL, vm, &errmsg)) != SQLITE_OK) {
 		idmapdlog(LOG_ERR,
 			"Database error during %s (%s)",
 			sql, CHECK_NULL(errmsg));
@@ -782,18 +900,10 @@ sql_compile_n_step_once(sqlite *db, char *sql, sqlite_vm **vm, int *ncol,
 		return (IDMAP_ERR_INTERNAL);
 	}
 
-	for (i = 0, s = SLEEP_TIME; i < MAX_TRIES; i++, s *= 2) {
-		r = sqlite_step(*vm, ncol, values, NULL);
-		if (r != SQLITE_BUSY)
-			break;
-		NANO_SLEEP(rqtp, s);
-	}
+	r = sqlite_step(*vm, ncol, values, NULL);
+	assert(r != SQLITE_LOCKED && r != SQLITE_BUSY);
 
-	if (r == SQLITE_BUSY) {
-		(void) sqlite_finalize(*vm, NULL);
-		*vm = NULL;
-		return (IDMAP_ERR_BUSY);
-	} else if (r == SQLITE_ROW) {
+	if (r == SQLITE_ROW) {
 		if (ncol != NULL && *ncol < reqcol) {
 			(void) sqlite_finalize(*vm, NULL);
 			*vm = NULL;
@@ -810,7 +920,7 @@ sql_compile_n_step_once(sqlite *db, char *sql, sqlite_vm **vm, int *ncol,
 	(void) sqlite_finalize(*vm, &errmsg);
 	*vm = NULL;
 	idmapdlog(LOG_ERR, "Database error during %s (%s)",
-		sql, CHECK_NULL(errmsg));
+	    sql, CHECK_NULL(errmsg));
 	sqlite_freemem(errmsg);
 	return (IDMAP_ERR_INTERNAL);
 }
@@ -1297,7 +1407,7 @@ retry:
 	}
 
 	if (retcode == IDMAP_ERR_RETRIABLE_NET_ERR)
-		idmap_lookup_free_batch(&state->ad_lookup);
+		idmap_lookup_release_batch(&state->ad_lookup);
 	else
 		retcode = idmap_lookup_batch_end(&state->ad_lookup, NULL);
 
@@ -1308,7 +1418,7 @@ retry:
 
 out:
 	idmapdlog(LOG_NOTICE, "Windows SID to user/group name lookup failed");
-	idmap_lookup_free_batch(&state->ad_lookup);
+	idmap_lookup_release_batch(&state->ad_lookup);
 	return (retcode);
 }
 
@@ -1567,9 +1677,8 @@ name_based_mapping_sid2pid(sqlite *db, idmap_mapping *req, idmap_id_res *res) {
 	char		*end;
 	const char	**values;
 	sqlite_vm	*vm = NULL;
-	struct timespec rqtp;
 	idmap_utf8str	*str;
-	int		ncol, r, i, s, is_user;
+	int		ncol, r, i, is_user;
 	const char	*me = "name_based_mapping_sid2pid";
 
 	winname = req->id1name.idmap_utf8str_val;
@@ -1613,18 +1722,11 @@ name_based_mapping_sid2pid(sqlite *db, idmap_mapping *req, idmap_id_res *res) {
 		goto out;
 	}
 
-	for (i = 0, s = SLEEP_TIME; ; ) {
+	for (; ; ) {
 		r = sqlite_step(vm, &ncol, &values, NULL);
+		assert(r != SQLITE_LOCKED && r != SQLITE_BUSY);
 
-		if (r == SQLITE_BUSY) {
-			if (++i < MAX_TRIES) {
-				NANO_SLEEP(rqtp, s);
-				s *= 2;
-				continue;
-			}
-			retcode = IDMAP_ERR_BUSY;
-			goto out;
-		} else if (r == SQLITE_ROW) {
+		if (r == SQLITE_ROW) {
 			if (ncol < 2) {
 				retcode = IDMAP_ERR_INTERNAL;
 				goto out;
@@ -2262,13 +2364,13 @@ retry:
 	if (retcode != IDMAP_SUCCESS) {
 		idmapdlog(LOG_ERR,
 		"Failed to batch name2sid for AD lookup");
-		idmap_lookup_free_batch(&qs);
+		idmap_lookup_release_batch(&qs);
 		return (IDMAP_ERR_INTERNAL);
 	}
 
 out:
 	if (retcode == IDMAP_ERR_RETRIABLE_NET_ERR)
-		idmap_lookup_free_batch(&qs);
+		idmap_lookup_release_batch(&qs);
 	else
 		retcode = idmap_lookup_batch_end(&qs, NULL);
 
@@ -2352,8 +2454,7 @@ name_based_mapping_pid2sid(sqlite *db, sqlite *cache, const char *unixname,
 	const char	**values;
 	sqlite_vm	*vm = NULL;
 	idmap_utf8str	*str;
-	struct timespec	rqtp;
-	int		ncol, r, i, s;
+	int		ncol, r;
 	const char	*me = "name_based_mapping_pid2sid";
 
 	RDLOCK_CONFIG();
@@ -2390,17 +2491,10 @@ name_based_mapping_pid2sid(sqlite *db, sqlite *cache, const char *unixname,
 		goto out;
 	}
 
-	for (i = 0, s = SLEEP_TIME; ; ) {
+	for (;;) {
 		r = sqlite_step(vm, &ncol, &values, NULL);
-		if (r == SQLITE_BUSY) {
-			if (++i < MAX_TRIES) {
-				NANO_SLEEP(rqtp, s);
-				s *= 2;
-				continue;
-			}
-			retcode = IDMAP_ERR_BUSY;
-			goto out;
-		} else if (r == SQLITE_ROW) {
+		assert(r != SQLITE_LOCKED && r != SQLITE_BUSY);
+		if (r == SQLITE_ROW) {
 			if (ncol < 3) {
 				retcode = IDMAP_ERR_INTERNAL;
 				goto out;
diff --git a/usr/src/cmd/idmap/idmapd/idmapd.c b/usr/src/cmd/idmap/idmapd/idmapd.c
index 765aebcd92..1ca4c1b0a1 100644
--- a/usr/src/cmd/idmap/idmapd/idmapd.c
+++ b/usr/src/cmd/idmap/idmapd/idmapd.c
@@ -243,6 +243,8 @@ main(int argc, char **argv)
 	} else
 		(void) umask(0077);
 
+	idmap_init_tsd_key();
+
 	init_idmapd();
 
 	if (__init_daemon_priv(PU_RESETGROUPS|PU_CLEARLIMITSET,
diff --git a/usr/src/cmd/idmap/idmapd/idmapd.h b/usr/src/cmd/idmap/idmapd/idmapd.h
index b5b40b6c86..b455c65418 100644
--- a/usr/src/cmd/idmap/idmapd/idmapd.h
+++ b/usr/src/cmd/idmap/idmapd/idmapd.h
@@ -154,6 +154,7 @@ extern void	fini_mapping_system();
 extern void	print_idmapdstate();
 extern int	create_directory(const char *, uid_t, gid_t);
 extern int	load_config();
+extern int	idmap_init_tsd_key(void);
 
 
 extern int		init_dbs();
diff --git a/usr/src/cmd/idmap/idmapd/server.c b/usr/src/cmd/idmap/idmapd/server.c
index 7f03e0d324..e48482cdc9 100644
--- a/usr/src/cmd/idmap/idmapd/server.c
+++ b/usr/src/cmd/idmap/idmapd/server.c
@@ -257,10 +257,6 @@ out:
 		result->ids.ids_len = 0;
 		result->ids.ids_val = NULL;
 	}
-	if (cache)
-		(void) sqlite_close(cache);
-	if (db)
-		(void) sqlite_close(db);
 	result->retcode = idmap_stat4prot(result->retcode);
 	return (TRUE);
 }
@@ -386,8 +382,6 @@ out:
 		sqlite_freemem(sql);
 	if (IDMAP_ERROR(result->retcode))
 		(void) xdr_free(xdr_idmap_mappings_res, (caddr_t)result);
-	if (cache)
-		(void) sqlite_close(cache);
 	result->retcode = idmap_stat4prot(result->retcode);
 	return (TRUE);
 }
@@ -559,8 +553,6 @@ out:
 		sqlite_freemem(sql);
 	if (IDMAP_ERROR(result->retcode))
 		(void) xdr_free(xdr_idmap_namerules_res, (caddr_t)result);
-	if (db)
-		(void) sqlite_close(db);
 	result->retcode = idmap_stat4prot(result->retcode);
 	return (TRUE);
 }
@@ -617,6 +609,7 @@ idmap_update_1_svc(idmap_update_batch batch, idmap_retcode *result,
 	sqlite		*db = NULL;
 	idmap_update_op	*up;
 	int		i;
+	int		trans = FALSE;
 
 	if (verify_rules_auth(rqstp) < 0) {
 		*result = IDMAP_ERR_PERMISSION_DENIED;
@@ -637,6 +630,7 @@ idmap_update_1_svc(idmap_update_batch batch, idmap_retcode *result,
 	*result = sql_exec_no_cb(db, "BEGIN TRANSACTION;");
 	if (*result != IDMAP_SUCCESS)
 		goto out;
+	trans = TRUE;
 
 	for (i = 0; i < batch.idmap_update_batch_len; i++) {
 		up = &batch.idmap_update_batch_val[i];
@@ -666,12 +660,12 @@ idmap_update_1_svc(idmap_update_batch batch, idmap_retcode *result,
 	}
 
 out:
-	if (*result == IDMAP_SUCCESS && db) {
-		*result = sql_exec_no_cb(db, "COMMIT TRANSACTION;");
+	if (trans) {
+		if (*result == IDMAP_SUCCESS)
+			*result = sql_exec_no_cb(db, "COMMIT TRANSACTION;");
+		else
+			(void) sql_exec_no_cb(db, "ROLLBACK TRANSACTION;");
 	}
-
-	if (db)
-		(void) sqlite_close(db);
 	*result = idmap_stat4prot(*result);
 	return (TRUE);
 }
@@ -735,10 +729,6 @@ out:
 		result->mappings.mappings_len = 0;
 		result->mappings.mappings_val = NULL;
 	}
-	if (cache)
-		(void) sqlite_close(cache);
-	if (db)
-		(void) sqlite_close(db);
 	result->retcode = idmap_stat4prot(result->retcode);
 	return (TRUE);
 }