mutex: lock-order checker compares latches within one environment only

src/dbinc/lock_order.h

@@ -170,6 +170,7 @@ extern "C" {
 #define	DB_LO_STACK_MAX	128
 
 typedef struct __db_lo_held {
+	const ENV	*env;		/* Whose mutex region the index is in. */
 	db_mutex_t	 mutex;		/* Which latch (region-relative). */
 	int		 alloc_id;	/* MTX_* class. */
 	int		 rank;		/* Cached rank of alloc_id. */

src/mutex/mut_order.c

@@ -438,6 +438,7 @@ __db_lo_check(env, mutex, file, line)
 	if (mutex == MUTEX_INVALID || env == NULL || lo_reported)
 		return;
 
+	nw.env = env;
 	nw.mutex = mutex;
 	nw.alloc_id = lo_alloc_id(env, mutex);
 	nw.rank = __db_lo_rank(nw.alloc_id);
@@ -448,6 +449,17 @@ __db_lo_check(env, mutex, file, line)
 		return;
 
 	for (i = 0; i < lo_depth; i++) {
+		/*
+		 * A mutex index is relative to one environment's mutex region,
+		 * so the same index in another environment is another latch,
+		 * and two environments have no order between them.  A thread
+		 * that holds a latch in one and takes one in another (the TCL
+		 * replication transport does, from inside the send callback)
+		 * is not a violation.
+		 */
+		if (lo_stack[i].env != env)
+			continue;
+
 		/*
 		 * Same latch twice on one thread.  Rank-independent: these are
 		 * not recursive latches, so this is an unconditional
@@ -498,6 +510,7 @@ __db_lo_acquire(env, mutex, file, line)
 	if (mutex == MUTEX_INVALID || env == NULL || lo_reported)
 		return;
 
+	nw.env = env;
 	nw.mutex = mutex;
 	nw.alloc_id = lo_alloc_id(env, mutex);
 	nw.rank = __db_lo_rank(nw.alloc_id);
@@ -553,12 +566,11 @@ __db_lo_release(env, mutex)
 {
 	int i;
 
-	COMPQUIET(env, NULL);
 	if (mutex == MUTEX_INVALID || lo_reported)
 		return;
 
 	for (i = lo_depth; i-- > 0; )
-		if (lo_stack[i].mutex == mutex) {
+		if (lo_stack[i].env == env && lo_stack[i].mutex == mutex) {
 			if (i != lo_depth - 1)
 				memmove(&lo_stack[i], &lo_stack[i + 1],
 				    (size_t)(lo_depth - i - 1) *

test/c/lock_order_check.c

@@ -43,6 +43,8 @@
  * Verdict lines are explicit ("PASS:" / "FAIL:") so a harness can assert a
  * real verdict was produced rather than trusting the exit status alone.
  */
+#include "db_config.h"
+
 #include <sys/types.h>
 #include <sys/wait.h>
 #include <signal.h>
@@ -51,7 +53,8 @@
 #include <string.h>
 #include <unistd.h>
 
-#include "db.h"
+#include "db_int.h"
+#include "dbinc/mp.h"
 
 #define	HOME_PREFIX	"LOCK_ORDER_CHECK"
 #define	NSEED		400
@@ -62,6 +65,7 @@
  */
 #define	SUBJECT_TIMEOUT	60
 
+
 static int
 ck(int ret, const char *what, int line)
 {
@@ -158,6 +162,62 @@ workload(u_int32_t nparts)
 	CK(env->close(env, 0));
 }
 
+/*
+ * TWO-ENV arm.  A mutex index is relative to one environment's mutex region,
+ * so two environments created the same way hand out the same indices.  The
+ * checker used to compare indices across environments, so a thread holding a
+ * latch in one environment and taking the same-numbered latch in another was
+ * reported as a self-deadlock and the environment panicked.  The TCL
+ * replication harness hits this on every election under --enable-diagnostic:
+ * __rep_update_setup holds the rep region latch while the send callback puts
+ * the message into a queue database in a second environment.
+ *
+ * Reproduce it directly: hold env A's mpool region latch, take env B's.  They
+ * are different latches with the same index.
+ */
+static DB_ENV *
+two_env_open(const char *tag)
+{
+	DB_ENV *dbenv;
+	char cmd[512], home[256];
+
+	(void)snprintf(home, sizeof(home), "%s_2ENV_%s_%lu",
+	    HOME_PREFIX, tag, (unsigned long)getpid());
+	(void)snprintf(cmd, sizeof(cmd), "mkdir -p %s", home);
+	if (system(cmd) != 0)
+		exit(3);
+	CK(db_env_create(&dbenv, 0));
+	CK(dbenv->open(dbenv, home, DB_CREATE | DB_INIT_LOCK | DB_INIT_LOG |
+	    DB_INIT_MPOOL | DB_INIT_TXN, 0));
+	return (dbenv);
+}
+
+/* int, not void: MUTEX_LOCK returns DB_RUNRECOVERY from its caller on panic. */
+static int
+two_env(void)
+{
+	DB_ENV *a, *b;
+	db_mutex_t ma, mb;
+
+	a = two_env_open("A");
+	b = two_env_open("B");
+	ma = ((DB_MPOOL *)a->env->mp_handle)->reginfo[0].mtx_alloc;
+	mb = ((DB_MPOOL *)b->env->mp_handle)->reginfo[0].mtx_alloc;
+	if (ma != mb) {
+		/* The arm has no teeth unless the indices collide. */
+		printf("two-env: mpool latch indices differ (%lu, %lu)\n",
+		    (unsigned long)ma, (unsigned long)mb);
+		exit(4);
+	}
+	MUTEX_LOCK(a->env, ma);
+	MUTEX_LOCK(b->env, mb);
+	MUTEX_UNLOCK(b->env, mb);
+	MUTEX_UNLOCK(a->env, ma);
+	CK(b->close(b, 0));
+	CK(a->close(a, 0));
+	return (0);
+}
+
 int
 main(int argc, char **argv)
 {
@@ -169,7 +229,11 @@ main(int argc, char **argv)
 	 * the partition count.
 	 */
 	if (argc == 3 && strcmp(argv[1], "--arm") == 0) {
-		workload((u_int32_t)atoi(argv[2]));
+		if (strcmp(argv[2], "2env") == 0) {
+			if (two_env() != 0)
+				return (1);
+		} else
+			workload((u_int32_t)atoi(argv[2]));
 		printf("ARM_COMPLETED\n");
 		return (0);
 	}
@@ -266,6 +330,21 @@ main(int argc, char **argv)
 		fails++;
 	}
 
+	/* ---- TWO-ENV: latches in different environments never conflict. */
+	if ((pid = fork()) == 0) {
+		execl(argv[0], argv[0], "--arm", "2env", (char *)NULL);
+		_exit(127);
+	}
+	(void)waitpid(pid, &status, 0);
+	if (WIFEXITED(status) && WEXITSTATUS(status) == 0)
+		printf("PASS (two-env): latches with the same index in two "
+		    "environments were not reported\n");
+	else {
+		printf("FAIL (two-env): status 0x%x -- the checker compared "
+		    "mutex indices across environments\n", status);
+		fails++;
+	}
+
 	printf("%s: lock_order_check\n", fails == 0 ? "PASS" : "FAIL");
 	return (fails == 0 ? 0 : 1);
 }