From aab6239b7867c756bbe06b72ae819e20feafae60 Mon Sep 17 00:00:00 2001 From: Greg Burd Date: Wed, 4 Nov 2026 13:08:22 -0500 Subject: [PATCH 1/3] mutex: the lock-order checker compares latches within one environment only 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 kept one per-thread stack of held indices and compared them regardless of environment. A thread that held a latch in one environment and took the same-numbered latch in another was reported as a self-deadlock (BDB2077/BDB2080), or as a rank inversion when the classes differed (BDB2084), and the environment panicked. The TCL replication harness hits this on every election in an --enable-diagnostic build. __rep_update_setup holds mtx_clientdb and the rep region latch (rep_backup.c:1110-1112) and calls __rep_nextfile, which sends a message. The send callback puts it into a queue database in a second environment, and that put takes the queue environment's mpool region latch, mutex 3, the same index as the client's rep region latch. A non-diagnostic build is clean, and so is the engine: these are two different latches. Record the ENV with each held entry. The order and same-latch checks compare only entries from the same environment, and release pops only the entry from the releasing environment. lock_order_check gains a two-env arm: hold env A's mpool region latch, take env B's, with an exit if the indices do not collide. On the dev box (Debian 12, --enable-diagnostic) master reports BDB2077 and the arm fails with SIGABRT; with this change it passes, and the control and lk_partitions=1 arms are unchanged. (cherry picked from commit 6469eafa7481c7d6fc186b93b5c4878b239f2986) --- src/dbinc/lock_order.h | 1 + src/mutex/mut_order.c | 16 +++++++- test/c/lock_order_check.c | 79 ++++++++++++++++++++++++++++++++++++++- 3 files changed, 92 insertions(+), 4 deletions(-) diff --git a/src/dbinc/lock_order.h b/src/dbinc/lock_order.h index 34c52722db..ddfb12b782 100644 --- a/src/dbinc/lock_order.h +++ b/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. */ diff --git a/src/mutex/mut_order.c b/src/mutex/mut_order.c index 041014bafc..3582d37250 100644 --- a/src/mutex/mut_order.c +++ b/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) * diff --git a/test/c/lock_order_check.c b/test/c/lock_order_check.c index 4ec0eb8185..a3ecb2adf5 100644 --- a/test/c/lock_order_check.c +++ b/test/c/lock_order_check.c @@ -51,7 +51,9 @@ #include #include -#include "db.h" +#include "db_config.h" +#include "db_int.h" +#include "dbinc/mp.h" #define HOME_PREFIX "LOCK_ORDER_CHECK" #define NSEED 400 @@ -62,6 +64,7 @@ */ #define SUBJECT_TIMEOUT 60 + static int ck(int ret, const char *what, int line) { @@ -158,6 +161,60 @@ 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); +} + +static void +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)); +} + int main(int argc, char **argv) { @@ -169,7 +226,10 @@ 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) + two_env(); + else + workload((u_int32_t)atoi(argv[2])); printf("ARM_COMPLETED\n"); return (0); } @@ -266,6 +326,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); } From 0030fde3d6fb8401046d91fc5b605bd7bea441c4 Mon Sep 17 00:00:00 2001 From: Greg Burd Date: Wed, 4 Nov 2026 14:37:57 -0500 Subject: [PATCH 2/3] test/c: include db_config.h first in lock_order_check.c s_chk_inclconfig requires db_config.h before any other include in a file that includes it. Found by the region-signature CI job, whose first step runs dist/s_validate. (cherry picked from commit 0c749aba4115c2441cdacb102de7d170f34c331a) --- test/c/lock_order_check.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/c/lock_order_check.c b/test/c/lock_order_check.c index a3ecb2adf5..4244e68b0a 100644 --- a/test/c/lock_order_check.c +++ b/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 #include #include @@ -51,7 +53,6 @@ #include #include -#include "db_config.h" #include "db_int.h" #include "dbinc/mp.h" From 8bec6e80b5c421e1d018810f9a962ad694a0c515 Mon Sep 17 00:00:00 2001 From: Greg Burd Date: Wed, 4 Nov 2026 15:38:06 -0500 Subject: [PATCH 3/3] test/c: lock_order_check's two-env arm returns int MUTEX_LOCK returns DB_RUNRECOVERY from its caller when the environment is panicked, so it compiles only inside a function that returns int. gcc accepted the void function; clang on FreeBSD rejects it with -Wreturn-mismatch. (cherry picked from commit 6f11b17ff0416d227e26ced1c3162289066ad64b) --- test/c/lock_order_check.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/test/c/lock_order_check.c b/test/c/lock_order_check.c index 4244e68b0a..38ba6c9f0d 100644 --- a/test/c/lock_order_check.c +++ b/test/c/lock_order_check.c @@ -192,7 +192,8 @@ two_env_open(const char *tag) return (dbenv); } -static void +/* int, not void: MUTEX_LOCK returns DB_RUNRECOVERY from its caller on panic. */ +static int two_env(void) { DB_ENV *a, *b; @@ -214,6 +215,7 @@ two_env(void) MUTEX_UNLOCK(a->env, ma); CK(b->close(b, 0)); CK(a->close(a, 0)); + return (0); } int @@ -227,9 +229,10 @@ main(int argc, char **argv) * the partition count. */ if (argc == 3 && strcmp(argv[1], "--arm") == 0) { - if (strcmp(argv[2], "2env") == 0) - two_env(); - else + 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);