From 4555115a5ceff77d09f4c3e42dae897414c9c749 Mon Sep 17 00:00:00 2001 From: libdb-agent Date: Mon, 2 Nov 2026 18:53:44 -0500 Subject: [PATCH 1/2] lock: SSI must not drop its own SIREAD marker when write-locking the page Fixes a silent write-skew miss under DB_TXN_SERIALIZABLE: on a multi-level B-tree, two transactions whose read and write records sit on different leaves could both commit a non-serializable history, with no error returned. Root cause: __lock_get_internal's SIREAD scan had a branch that, when a writer write-locked an object on which it already held its OWN SIREAD marker, REMOVED that marker ("upgrading my own read to a write"). Conflict detection is at PAGE granularity, so holding a WRITE lock on a page does NOT mean the transaction wrote the record it had read there: a point update of key X write-locks X's leaf, and the B-tree write descent can also write-lock a different leaf that carries the transaction's SIREAD from an earlier read of key Y. Dropping that marker erased the record of "this transaction read key Y", so a later writer of Y's page by another transaction found no marker and never recorded the rw-antidependency -- both transactions committed a write skew. Fix: keep the transaction's own SIREAD marker and merely skip it for self-conflict purposes (a transaction never forms an rw-edge with itself). The marker persists exactly like any committed reader's and is reclaimed by the normal GC (__lock_sicommit at commit, __lock_siclean_obj once its snapshot is no longer visible), which also balances si_ref -- so the issue-#137 accounting the old eager drop maintained is preserved by the ordinary path, not lost. FAIL-before / PASS-after gate: test/isolation/ssi_multiread_skew (0/40 skews at every tree depth and timing after this change; 20-40/40 before). No format/ABI change. Backport candidate for lts/2026.10.7. Workstream S1. (cherry picked from commit cced5c481a3cbc89e5e3ce135de82bdd22f17ea4) --- src/lock/lock.c | 57 +++++++++++++++++++++++-------------------------- 1 file changed, 27 insertions(+), 30 deletions(-) diff --git a/src/lock/lock.c b/src/lock/lock.c index 983626bc8a..e4adaa2aa8 100644 --- a/src/lock/lock.c +++ b/src/lock/lock.c @@ -1255,38 +1255,35 @@ again: if (obj == NULL) { if (lock_mode == DB_LOCK_WRITE && sh_off == sireadlp->holder) { /* - * Upgrading our own SIREAD to WRITE: drop the - * SIREAD marker to avoid self-conflicts. + * Our own SIREAD marker on this object. A + * transaction never forms an rw-antidependency + * with itself, so skip it for conflict purposes -- + * but DO NOT remove it. Conflict detection is at + * PAGE granularity: write-locking this page does + * NOT mean we are writing the same record we read. + * T may have READ key k1 on this page (leaving the + * marker) and now be WRITE-locking the page only to + * update a DIFFERENT key k2 that happens to share + * the leaf -- or merely because the B-tree write + * descent couples a write lock across this leaf. + * Dropping the marker here erased "T read this + * page", so a later writer of the page by ANOTHER + * transaction found no marker and the rw-edge + * T --rw--> W was never recorded -- a write skew + * across two leaves of a multi-level B-tree then + * committed silently (the SSI cross-page miss). * - * Account for the detail reference the marker - * held, exactly as the other two removal sites - * (__lock_sicommit, __lock_siclean_obj) do and - * with the same td_off guard the grant used. - * Without it si_ref stays permanently above the - * true marker count, so the owning detail (and - * the locker deferring on it) can never be - * reclaimed: a snapshot txn that reads then - * writes the same key leaks a detail, a locker - * and their mutex slots on every iteration until - * txn_begin returns ENOMEM. - * - * This is the reader's OWN marker (sh_off == - * holder), so the detail is its live, running - * transaction: si_ref cannot reach zero here and - * no reclaim can trigger underneath us. + * Keeping the marker is correct and does not leak: + * it persists on the object exactly like any other + * committed reader's marker and is reclaimed by the + * normal GC (__lock_sicommit detaches it at commit; + * __lock_siclean_obj frees it once its snapshot is + * no longer visible to any active reader), which is + * also what balances si_ref -- so the #137 accounting + * the old eager drop maintained is preserved by the + * ordinary path, not lost. */ - SH_TAILQ_REMOVE(&sh_obj->sireaders, - sireadlp, links, __db_lock); - if (atomic_read_relaxed(®ion->nsireaders) > 0) - (void)atomic_dec(env, - ®ion->nsireaders); - if (sh_locker->td_off != INVALID_ROFF) - (void)atomic_dec(env, - &LOCKER_TD(env, sh_locker)->si_ref); - if ((ret = __lock_freelock(lt, sireadlp, - LOCK_HOLDER(env, sireadlp), - DB_LOCK_UNLINK | DB_LOCK_FREE)) != 0) - goto err; + continue; } else if (lock_mode == DB_LOCK_WRITE && sh_off != sireadlp->holder && (LOCK_OWNER(env, sireadlp)->status == TXN_RUNNING || From b83310f6b4b78beb95d3fe3408b583c50e0eb49d Mon Sep 17 00:00:00 2001 From: libdb-agent Date: Mon, 2 Nov 2026 19:21:12 -0500 Subject: [PATCH 2/2] [2026.10.7] test/ssi: cross-leaf write-skew regression gate Add test/isolation/ssi_multiread_skew.c (the deterministic FAIL-before/ PASS-after gate for the cross-leaf SSI write-skew fix) and wire it into the isolation tier's SSI gates and test/MANIFEST. It must PASS at every B-tree depth and both commit timings. Companion to the cherry-picked engine fix on this branch. --- test/MANIFEST | 5 + test/isolation/run.sh | 21 +- test/isolation/ssi_multiread_skew.c | 329 ++++++++++++++++++++++++++++ 3 files changed, 354 insertions(+), 1 deletion(-) create mode 100755 test/isolation/ssi_multiread_skew.c diff --git a/test/MANIFEST b/test/MANIFEST index caad5399bf..38f220a7e1 100644 --- a/test/MANIFEST +++ b/test/MANIFEST @@ -95,6 +95,11 @@ isolation ssi_gc_pressure@serializable-default isolation ssi_gc_pressure@serializable-1 isolation ssi_crash_pivot-default optional isolation ssi_crash_pivot-1 optional +# ssi_multiread_skew is the regression gate for the cross-leaf write-skew fix +# (SSI keeps its own SIREAD marker instead of dropping it on a same-page write +# lock). It FAILed on every pre-fix build and must now PASS. +isolation ssi_multiread_skew-default +isolation ssi_multiread_skew-1 # The two SSI mechanism gates. ssi_gc_pressure runs at both levels (the # snapshot pass is the anti-vacuity control). ssi_crash_pivot needs a # DIAGNOSTIC build for its in-commit crash hook, so it is optional -- but the diff --git a/test/isolation/run.sh b/test/isolation/run.sh index 1fcd706676..1458990fba 100755 --- a/test/isolation/run.sh +++ b/test/isolation/run.sh @@ -92,7 +92,8 @@ ISO_SSI_GATES=${ISO_SSI_GATES:-1} ISO_PARTS=${ISO_PARTS:-"default 1"} mkdir -p "$OUT" -for t in test_iso_anomaly test_ssi_gc_pressure test_ssi_crash_pivot; do +for t in test_iso_anomaly test_ssi_gc_pressure test_ssi_crash_pivot \ + ssi_multiread_skew; do # shellcheck disable=SC2086 $CC $CFLAGS "$HERE/$t.c" "$LIBDBA" $LDLIBS -o "$OUT/$t" echo "built $OUT/$t" @@ -152,6 +153,24 @@ run_ssi_gates() { # "nobody invoked it" must not look the same to the gate. hi_emit ssi_crash_pivot skip fi + + # Regression gate for the cross-leaf write-skew fix (SSI keeps its + # own SIREAD marker instead of dropping it on a same-page write + # lock). ssi_multiread_skew FAILed on every pre-fix build and must + # now PASS at every tree depth and timing. + echo "--- SSI gate: multi-level cross-leaf write skew prevented"\ + "(parts=$PARTS_LABEL)" + mkdir -p "mrskew-$PARTS_LABEL" + if ( cd "mrskew-$PARTS_LABEL" && SMR_ATTEMPTS=40 SMR_EXTRA=0 \ + SMR_FILLER=400 SMR_SEQUENTIAL=1 \ + timeout "$ISO_TIMEOUT" ../ssi_multiread_skew ); then + hi_emit "ssi_multiread_skew-$PARTS_LABEL" pass + else + echo " FAIL: SSI admitted the multi-level write skew --"\ + "the cross-leaf detection fix has regressed." + hi_emit "ssi_multiread_skew-$PARTS_LABEL" fail + _rc=1 + fi fi return $_rc } diff --git a/test/isolation/ssi_multiread_skew.c b/test/isolation/ssi_multiread_skew.c new file mode 100755 index 0000000000..e23a9c10a7 --- /dev/null +++ b/test/isolation/ssi_multiread_skew.c @@ -0,0 +1,329 @@ +/*- + * Copyright (c) 2026 libdb contributors. All rights reserved. + * + * SPDX-License-Identifier: Sleepycat + * + * See the file LICENSE for redistribution information. + */ + +/*- + * ssi_multiread_skew.c -- INDEPENDENT reproducer probe. + * + * The Adya cycle checker (test_ssi_cycle) reported, under DB_TXN_SERIALIZABLE + * at high concurrency, a 2-transaction write skew where EACH transaction has a + * MULTI-KEY read set, not the single read of the canonical ssi write_skew gate. + * This program reproduces that exact shape deterministically, with NO cycle + * checker, NO shared buffers, NO version model -- just two threads, barriers, + * and a direct check of the committed outcome. It answers one question: can + * two serializable transactions, each reading several keys (one of which is the + * key the other writes) and writing one key the other reads, BOTH commit? + * + * SHAPE (keys on DIFFERENT pages, so no page write-write collision): + * initial: X=0, Y=0 (plus filler to spread the tree) + * T1: read Y (and some other keys), then write X=1 + * T2: read X (and some other keys), then write Y=1 + * Both take their snapshot before either writes. Serial orders give + * (X,Y) in {(1,0),(0,1)} ... never (1,1). (1,1) == both committed == a + * write skew that SSI must prevent. + * + * The ONLY difference from the known-good ssi write_skew gate is that each + * txn also reads EXTRA keys before its decisive read. If (1,1) appears here + * but not in the single-read gate, the extra reads are the trigger. + * + * Env/knobs: SMR_ATTEMPTS (default 200), SMR_EXTRA (extra reads per txn, + * default 4), SMR_PAGESIZE (default 512), SMR_FILLER (default 400). + * + * Exit: 0 = SSI prevented the skew in every attempt (no (1,1)); 1 = at least + * one attempt committed both (a write skew under DB_TXN_SERIALIZABLE); 2 = + * harness error. + */ +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "db.h" +#include "iso_knobs.h" + +static DB_ENV *env; +static DB *db; +static char home[512]; +static int smr_extra = 4, smr_pagesize = 512, smr_filler = 400; +static int smr_sequential = 0; /* T1 fully commits before T2 writes */ +static int nkeys; /* total keys loaded (filler + X + Y) */ +static int kx, ky; /* the two decisive keys (far apart) */ + +static pthread_barrier_t bar; +static atomic_int t1_committed, t2_committed; + +static void +die(const char *what, int rc) +{ + fprintf(stderr, "harness error: %s: %s (%d)\n", what, db_strerror(rc), rc); + exit(2); +} + +static void +rmtree(const char *dir) +{ + char cmd[600]; + (void)snprintf(cmd, sizeof(cmd), + "find '%s' -mindepth 1 -delete 2>/dev/null", dir); + (void)system(cmd); +} + +static int +rd(DB_TXN *txn, int key, int *out) +{ + DBT k, d; + char kb[16]; + int v, rc; + + (void)snprintf(kb, sizeof(kb), "k%08d", key); + memset(&k, 0, sizeof(k)); + memset(&d, 0, sizeof(d)); + k.data = kb; k.size = (u_int32_t)strlen(kb); + d.data = &v; d.ulen = sizeof(v); d.flags = DB_DBT_USERMEM; + if ((rc = db->get(db, txn, &k, &d, 0)) != 0) + return (rc); + if (out) *out = v; + return (0); +} + +static int +wr(DB_TXN *txn, int key, int val) +{ + DBT k, d; + char kb[16]; + + (void)snprintf(kb, sizeof(kb), "k%08d", key); + memset(&k, 0, sizeof(k)); + memset(&d, 0, sizeof(d)); + k.data = kb; k.size = (u_int32_t)strlen(kb); + d.data = &val; d.size = sizeof(val); + return (db->put(db, txn, &k, &d, 0)); +} + +static void +open_env(u_int32_t create) +{ + int rc; + + if ((rc = db_env_create(&env, 0)) != 0) die("db_env_create", rc); + (void)iso_set_partitions(env); + if ((rc = env->set_lk_detect(env, DB_LOCK_DEFAULT)) != 0) + die("set_lk_detect", rc); + if ((rc = env->set_timeout(env, 2000000, DB_SET_LOCK_TIMEOUT)) != 0) + die("set_timeout", rc); + if ((rc = env->set_cachesize(env, 0, 16 * 1024 * 1024, 1)) != 0) + die("set_cachesize", rc); + if ((rc = env->open(env, home, create | DB_INIT_LOCK | DB_INIT_LOG | + DB_INIT_MPOOL | DB_INIT_TXN | DB_THREAD, 0600)) != 0) + die("env->open", rc); + if ((rc = db_create(&db, env, 0)) != 0) die("db_create", rc); + if (create && (rc = db->set_pagesize(db, (u_int32_t)smr_pagesize)) != 0) + die("set_pagesize", rc); + if ((rc = db->open(db, NULL, "skew.db", NULL, DB_BTREE, + create | DB_MULTIVERSION | DB_AUTO_COMMIT | DB_THREAD, 0600)) != 0) + die("db->open", rc); +} + +static void +close_env(void) +{ + int rc; + if ((rc = db->close(db, 0)) != 0) die("db->close", rc); + if ((rc = env->close(env, 0)) != 0) die("env->close", rc); + env = NULL; db = NULL; +} + +static void +sync_pt(void) +{ + int rc = pthread_barrier_wait(&bar); + if (rc != 0 && rc != PTHREAD_BARRIER_SERIAL_THREAD) + die("barrier", rc); +} + +/* T2: read X (+ extra), write Y. */ +static void * +t2_thread(void *arg) +{ + DB_TXN *txn; + int e, junk, rc, x; + + (void)arg; + if ((rc = env->txn_begin(env, NULL, &txn, DB_TXN_SERIALIZABLE)) != 0) + die("t2 begin", rc); + /* Extra reads first (spread across the key space). */ + for (e = 0; e < smr_extra; e++) + if ((rc = rd(txn, (e * 7 + 3) % nkeys, &junk)) != 0 && + rc != DB_LOCK_DEADLOCK && rc != DB_SNAPSHOT_CONFLICT && + rc != DB_SNAPSHOT_UNSAFE && rc != DB_LOCK_NOTGRANTED) + die("t2 extra read", rc); + rc = rd(txn, kx, &x); /* the decisive read */ + sync_pt(); /* both snapshots taken + read */ + sync_pt(); /* T1 write done */ + if (rc == 0) + rc = wr(txn, ky, 1); + if (rc == 0) { + rc = txn->commit(txn, 0); + if (rc == 0) atomic_store(&t2_committed, 1); + } else + (void)txn->abort(txn); + if (rc != 0 && rc != DB_LOCK_DEADLOCK && rc != DB_SNAPSHOT_CONFLICT && + rc != DB_SNAPSHOT_UNSAFE && rc != DB_LOCK_NOTGRANTED) + die("t2 outcome", rc); + return (NULL); +} + +/* Returns 1 if this attempt committed BOTH (a write skew). */ +static int +one_attempt(void) +{ + DB_TXN *txn; + pthread_t t2; + int e, junk, rc, y, xv, yv; + + rmtree(home); + open_env(DB_CREATE); + /* Load filler + the two decisive keys, all 0. */ + for (e = 0; e < nkeys; e++) + if ((rc = wr(NULL, e, 0)) != 0) + die("load", rc); + { + static int reported = 0; + if (!reported) { + DB_BTREE_STAT *bst; + reported = 1; + if (db->stat(db, NULL, &bst, 0) == 0) { + printf(" tree: %lu leaf pages, %lu levels, " + "%lu ndata\n", + (u_long)bst->bt_leaf_pg, + (u_long)bst->bt_levels, + (u_long)bst->bt_ndata); + free(bst); + } + } + } + + atomic_store(&t1_committed, 0); + atomic_store(&t2_committed, 0); + if ((rc = pthread_barrier_init(&bar, NULL, 2)) != 0) + die("barrier_init", rc); + if ((rc = pthread_create(&t2, NULL, t2_thread, NULL)) != 0) + die("pthread_create", rc); + + /* T1: read Y (+ extra), write X. */ + if ((rc = env->txn_begin(env, NULL, &txn, DB_TXN_SERIALIZABLE)) != 0) + die("t1 begin", rc); + for (e = 0; e < smr_extra; e++) + if ((rc = rd(txn, (e * 5 + 11) % nkeys, &junk)) != 0 && + rc != DB_LOCK_DEADLOCK && rc != DB_SNAPSHOT_CONFLICT && + rc != DB_SNAPSHOT_UNSAFE && rc != DB_LOCK_NOTGRANTED) + die("t1 extra read", rc); + rc = rd(txn, ky, &y); + sync_pt(); /* both snapshots taken + read */ + if (rc == 0) + rc = wr(txn, kx, 1); + if (smr_sequential) { + /* + * Unambiguous "late" timing: T1 COMMITS before T2 writes, so no + * commit-window race (#136) is possible. A skew here is purely a + * conflict-DETECTION gap: T2's write must still find T1's SIREAD + * marker on key Y and abort. + */ + if (rc == 0) { + rc = txn->commit(txn, 0); + if (rc == 0) atomic_store(&t1_committed, 1); + } else + (void)txn->abort(txn); + sync_pt(); /* T1 fully committed -> T2 writes */ + } else { + sync_pt(); /* T1 write done -> T2 may write */ + if (rc == 0) { + rc = txn->commit(txn, 0); + if (rc == 0) atomic_store(&t1_committed, 1); + } else + (void)txn->abort(txn); + } + if (rc != 0 && rc != DB_LOCK_DEADLOCK && rc != DB_SNAPSHOT_CONFLICT && + rc != DB_SNAPSHOT_UNSAFE && rc != DB_LOCK_NOTGRANTED) + die("t1 outcome", rc); + + (void)pthread_join(t2, NULL); + (void)pthread_barrier_destroy(&bar); + + /* Read the durable state back after a reopen. */ + close_env(); + open_env(0); + if ((rc = rd(NULL, kx, &xv)) != 0 || (rc = rd(NULL, ky, &yv)) != 0) + die("read back", rc); + close_env(); + + /* A write skew is (X,Y)=(1,1) with BOTH committed. */ + return (atomic_load(&t1_committed) && atomic_load(&t2_committed) && + xv == 1 && yv == 1); +} + +int +main(void) +{ + int a, attempts, skews = 0; + const char *s; + + if ((s = getenv("SMR_EXTRA")) != NULL) smr_extra = atoi(s); + if ((s = getenv("SMR_SEQUENTIAL")) != NULL) smr_sequential = atoi(s); + if ((s = getenv("SMR_PAGESIZE")) != NULL) smr_pagesize = atoi(s); + if ((s = getenv("SMR_FILLER")) != NULL) smr_filler = atoi(s); + attempts = (s = getenv("SMR_ATTEMPTS")) != NULL ? atoi(s) : 200; + + setvbuf(stdout, NULL, _IOLBF, 0); + iso_watchdog("ssi_multiread_skew", 240); + + /* + * Keys: filler [0..smr_filler), then X and Y. By default Y is at the far + * end (different leaf). SMR_KYOFF overrides Y's key so a caller can place + * X and Y on the SAME leaf (small offset) to probe same-page behaviour: + * under page-level MVCC a read of X and a write of a *different* key Y on + * the same page is still a cross write-skew on {X,Y} if each txn reads one + * and writes the other, and SSI must still prevent it. + */ + kx = 0; + { + const char *o = getenv("SMR_KYOFF"); + ky = (o != NULL && *o != '\0') ? atoi(o) : smr_filler + 1; + } + nkeys = (ky > smr_filler ? ky : smr_filler) + 1; + + (void)snprintf(home, sizeof(home), "SMRDIR"); + (void)mkdir(home, 0755); + + printf("%s\n", db_version(NULL, NULL, NULL)); + printf("ssi_multiread_skew: attempts=%d extra_reads=%d pagesize=%d " + "filler=%d (X=key%d Y=key%d)\n", + attempts, smr_extra, smr_pagesize, smr_filler, kx, ky); + + for (a = 0; a < attempts; a++) + if (one_attempt()) { + skews++; + printf(" attempt %d: WRITE SKEW -- both committed, " + "(X,Y)=(1,1) under DB_TXN_SERIALIZABLE\n", a); + } + + rmtree(home); + if (skews != 0) { + printf("FAIL: %d/%d attempts committed a write skew under " + "DB_TXN_SERIALIZABLE (SSI admitted a non-serializable " + "history)\n", skews, attempts); + return (1); + } + printf("PASS: 0/%d attempts -- SSI prevented the multi-read write " + "skew in every attempt\n", attempts); + return (0); +}