fix(lmdb): never call mdb_dbi_open from the read path

lmdb.h: "This function must not be called from multiple concurrent
transactions in the same process. A transaction that uses this function must
finish (either commit or abort) before any other transaction in the process
may use this function."

try_open_for_read called it on every read of a collection this process had not
yet written, from every gRPC thread at once, inside overlapping read
transactions. The v2.8.0 comment there described the cost as "one
mdb_dbi_open per read" as if it were merely slow. It is not slow, it is
unsafe: MDB_dbi indexes the env's shared handle table, and churning that
table concurrently silently rebinds handles that committed writes had cached.

Live on 2.8.0-3: 41 refusals in 45 minutes, every one "caller asked for
'image_hashes' but the handle addresses 'nsfw_images'". The v2.4.4 identity
sentinel caught all 41, so nothing was corrupted, but each refusal bumped
mirror drift and the Find gate then sent every read in the process to
MemoryStore for the rest of its life. That silently disabled query.projection
(MemoryStore ignores it) and served reads from a bounded evicting cache
instead of the full dataset.

prime_dbi_cache() now opens a handle for every existing sub-db in one
committed write transaction. The CONSTRUCTOR calls it, not the call site: a
store whose cache was never primed reports every existing-but-unwritten
collection as empty, and "remember to prime" is not an enforcement mechanism.
The read path serves only from cache; a miss means no such collection, which
is correct because priming covers what exists and cacheCommittedDbi covers
what is created later. Only write transactions now call mdb_dbi_open, and
LMDB permits one at a time - the contract is satisfied structurally.

Logging stays out of document_store_lmdb.cpp since test targets do not link
spdlog; primed_count()/prime_error() expose the outcome and
ProjectStoreRegistry reports it.

Live after upgrade: primed 47 + 19 handles, memoryMatchCount=0, projection
back to 166 bytes from 4273, filtered query 408ms against a 2463ms baseline -
the v2.8.0 scan work was never reaching LMDB on this box.

Test limitation stated honestly in the test itself: the concurrency test does
not reproduce the race (reverting leaves it green but for the primed_count
assertion). It pins the invariant and the mechanism; the evidence is the
documented contract plus the production log.
test_existing_collection_readable_without_writing_first guards the risk this
fix introduces.

ctest 19/19.

VERSION

@@ -1 +1 @@
-2.8.0
+2.8.1

docs/ROADMAP.md

@@ -1,6 +1,6 @@
 # Smartbotic Database - Status and Roadmap
 
-**Current version: 2.8.0** (see `VERSION`). Last reviewed: 2026-08-09.
+**Current version: 2.8.1** (see `VERSION`). Last reviewed: 2026-08-09.
 
 This is the single authoritative statement of what exists and what does not.
 If any other document in this repository disagrees with this one, this one is

service/src/storage/document_store_lmdb.cpp

@@ -287,7 +287,24 @@ void apply_projection_inplace(nlohmann::json& doc,
 // LmdbDocumentStore implementation
 // -------------------------------------------------------------------------
 
-LmdbDocumentStore::LmdbDocumentStore(LmdbEnv& env) noexcept : env_(env) {}
+LmdbDocumentStore::LmdbDocumentStore(LmdbEnv& env) : env_(env) {
+    // v2.8.1 — prime here, not at the call site. The read path now serves
+    // exclusively from dbi_cache_, so a store whose cache was never primed
+    // reports every existing-but-unwritten collection as EMPTY. That is a
+    // silent-wrong-data failure, and "the caller must remember to prime"
+    // is not an enforcement mechanism - so the constructor does it.
+    // Outcome is recorded rather than logged: this translation unit is linked
+    // into test targets that do not pull in spdlog, and keeping it
+    // logging-free preserves that. ProjectStoreRegistry reports it.
+    try {
+        primed_count_ = prime_dbi_cache();
+    } catch (const std::exception& e) {
+        // Advisory: a read-only or genuinely broken env lands here. Refusing to
+        // construct would take the whole project offline over something that
+        // degrades to "collections read as empty until first written".
+        prime_error_ = e.what();
+    }
+}
 
 unsigned int LmdbDocumentStore::open_for_write(WriteTxn& wtxn,
                                                 std::string_view collection) {
@@ -352,27 +369,96 @@ LmdbDocumentStore::try_open_for_read(ReadTxn& rtxn,
         auto it = dbi_cache_.find(key);
         if (it != dbi_cache_.end()) return it->second;
     }
-    MDB_dbi raw_dbi = 0;
-    std::string name = to_cstr(collection);
-    int rc = mdb_dbi_open(rtxn.raw(), name.c_str(), 0, &raw_dbi);
-    if (rc == MDB_NOTFOUND) return std::nullopt;
-    if (rc != MDB_SUCCESS) throw_mdb(rc, "dbi_open (collection read)");
-
-    // Deliberately NOT cached. LMDB keeps a handle private to the opening
-    // transaction until that transaction COMMITS; if it aborts, the handle is
-    // closed. ReadTxn always aborts (ReadTxn::~ReadTxn), so caching this
-    // handle would hand out a dangling MDB_dbi to every later caller, and
-    // every mdb_put / mdb_cursor_open using it fails with EINVAL - for the
-    // life of the process, for that one collection.
+    // v2.8.1 — the read path no longer calls mdb_dbi_open AT ALL.
     //
-    // In production this hit whichever collection happened to be READ before
-    // it was WRITTEN after a restart: peers were fine because open_for_write
-    // commits, which promotes the handle into the env's shared table.
+    // A cache miss means the sub-db does not exist, because prime_dbi_cache()
+    // opened every sub-db present at env-open time and open_for_write's callers
+    // cache any created since (after their commit). So the cache is complete by
+    // construction, and a miss is a definitive "no such collection".
     //
-    // The handle IS valid inside `rtxn`, which is all the caller needs. The
-    // cost is one mdb_dbi_open per read on collections this process has never
-    // written; once a write happens, open_for_write caches it permanently.
-    return raw_dbi;
+    // Why this had to change - lmdb.h states the contract plainly:
+    //
+    //   "This function must not be called from multiple concurrent
+    //    transactions in the same process. A transaction that uses this
+    //    function must finish (either commit or abort) before any other
+    //    transaction in the process may use this function."
+    //
+    // The previous revision called mdb_dbi_open here on every read of a
+    // collection this process had not yet written, from many concurrent gRPC
+    // threads, inside overlapping read transactions. That is a direct violation,
+    // and the consequence is not merely slow: MDB_dbi is an index into the env's
+    // shared handle table, so churning that table can silently rebind a handle
+    // that a committed write had already cached.
+    //
+    // Observed live on 2.8.0-3: 41 refusals in 45 minutes, every one of them
+    // "caller asked for 'image_hashes' but the handle addresses 'nsfw_images'".
+    // The v2.4.4 identity sentinel caught each attempt, so nothing was corrupted
+    // - but each failure bumped mirror drift, and the Find gate
+    // (mirrorHealthy() && mirrorDriftCount() == 0) then sent EVERY read in the
+    // process to MemoryStore for the rest of its life. That silently disabled
+    // query.projection too, which MemoryStore ignores entirely.
+    //
+    // With this change only write transactions ever call mdb_dbi_open, and LMDB
+    // permits exactly one write transaction at a time - so the contract is
+    // satisfied by construction rather than by convention.
+    return std::nullopt;
+}
+
+size_t LmdbDocumentStore::prime_dbi_cache() {
+    // Open a handle for every sub-db that already exists, in ONE write
+    // transaction, and cache them only after it commits. After this returns,
+    // reads never need mdb_dbi_open - see try_open_for_read.
+    std::vector<std::string> names;
+    std::unordered_map<std::string, unsigned int> opened;
+
+    WriteTxn wtxn(env_);
+
+    // Enumerate first, in a scope that closes the cursor before any handle is
+    // opened and well before the commit.
+    {
+        MDB_dbi main_dbi = 0;
+        // name == nullptr addresses the unnamed root sub-db, which lists the
+        // named ones. This never allocates a slot in the shared table (it
+        // resolves to LMDB's MAIN_DBI), so it cannot cause the rebinding this
+        // function exists to prevent.
+        int rc = mdb_dbi_open(wtxn.raw(), nullptr, 0, &main_dbi);
+        if (rc == MDB_NOTFOUND) return 0;          // brand-new env, nothing to do
+        if (rc != MDB_SUCCESS) throw_mdb(rc, "dbi_open (main for prime)");
+
+        MDB_cursor* cursor = nullptr;
+        mdb_check(mdb_cursor_open(wtxn.raw(), main_dbi, &cursor),
+                  "cursor_open (main for prime)");
+        struct CursorGuard {
+            MDB_cursor* c;
+            ~CursorGuard() { if (c) mdb_cursor_close(c); }
+        } guard{cursor};
+
+        MDB_val k{0, nullptr};
+        MDB_val v{0, nullptr};
+        int crc = mdb_cursor_get(cursor, &k, &v, MDB_FIRST);
+        while (crc == MDB_SUCCESS) {
+            names.emplace_back(to_sv(k));
+            crc = mdb_cursor_get(cursor, &k, &v, MDB_NEXT);
+        }
+    }
+
+    for (const auto& n : names) {
+        MDB_dbi dbi = 0;
+        int rc = mdb_dbi_open(wtxn.raw(), n.c_str(), 0, &dbi);
+        if (rc == MDB_NOTFOUND) continue;          // vanished under us; ignore
+        if (rc != MDB_SUCCESS) throw_mdb(rc, "dbi_open (prime)");
+        opened[n] = dbi;
+    }
+
+    // Commit BEFORE caching. Until this succeeds every handle above is private
+    // to wtxn and would be closed by an abort - the v2.8.0 lesson.
+    wtxn.commit();
+
+    {
+        std::lock_guard<std::mutex> lock(cache_mutex_);
+        for (const auto& [n, d] : opened) dbi_cache_[n] = d;
+    }
+    return opened.size();
 }
 
 void LmdbDocumentStore::put(std::string_view collection,

service/src/storage/document_store_lmdb.hpp

@@ -27,9 +27,30 @@ class LmdbEnv;
 
 class LmdbDocumentStore : public DocumentStore {
 public:
-    explicit LmdbDocumentStore(LmdbEnv& env) noexcept;
+    explicit LmdbDocumentStore(LmdbEnv& env);
     ~LmdbDocumentStore() override = default;
 
+    // v2.8.1 — open a handle for every sub-db that already exists, in one
+    // committed write transaction, and cache them. Returns how many were opened.
+    //
+    // The CONSTRUCTOR already calls this; it is public only for tests and for
+    // re-priming. It is what makes the read path safe: lmdb.h forbids calling
+    // mdb_dbi_open from concurrent transactions, and the read path used to do
+    // exactly that on every read of a not-yet-written collection. Churning the
+    // shared handle table silently rebound handles that committed writes had
+    // cached, which took LMDB reads offline process-wide (see the long comment
+    // on try_open_for_read).
+    //
+    // After priming, only write transactions call mdb_dbi_open, and LMDB allows
+    // just one of those at a time. Idempotent: mdb_dbi_open returns the existing
+    // handle when a sub-db is already open.
+    size_t prime_dbi_cache();
+
+    // Outcome of the constructor's priming pass, for the owner to report.
+    // prime_error() is empty on success.
+    size_t primed_count() const noexcept { return primed_count_; }
+    const std::string& prime_error() const noexcept { return prime_error_; }
+
     LmdbDocumentStore(const LmdbDocumentStore&) = delete;
     LmdbDocumentStore& operator=(const LmdbDocumentStore&) = delete;
     LmdbDocumentStore(LmdbDocumentStore&&) = delete;
@@ -74,6 +95,8 @@ private:
     // cache mutex; subsequent puts re-create the sub-db on demand.
     LmdbEnv& env_;
     std::mutex cache_mutex_;
+    size_t primed_count_ = 0;
+    std::string prime_error_;
     std::unordered_map<std::string, unsigned int> dbi_cache_;
 
     // Resolve a collection name to an MDB_dbi handle.

service/src/storage/project_store.cpp

@@ -53,7 +53,19 @@ ProjectStoreRegistry::open_one(const std::string& name) {
     auto store = std::make_unique<ProjectStore>();
     store->name = name;
     store->env = std::make_unique<LmdbEnv>(opts);
-    store->doc_store = std::make_unique<LmdbDocumentStore>(*store->env);
+    // The LmdbDocumentStore constructor primes every sub-db handle (see
+    // prime_dbi_cache) - reads depend on that cache being complete.
+    auto lmdb_store = std::make_unique<LmdbDocumentStore>(*store->env);
+    if (!lmdb_store->prime_error().empty()) {
+        spdlog::error("[project_store] project '{}': failed to prime sub-db "
+                      "handles: {} - existing collections may read as empty "
+                      "until first written",
+                      name, lmdb_store->prime_error());
+    } else {
+        spdlog::info("[project_store] project '{}': primed {} sub-db handle(s)",
+                     name, lmdb_store->primed_count());
+    }
+    store->doc_store = std::move(lmdb_store);
     return store;
 }
 

tests/test_subdb_identity.cpp

@@ -16,6 +16,7 @@
 #include <iostream>
 #include <cstdio>
 #include <algorithm>
+#include <thread>
 #include <set>
 #include <string>
 #include <unistd.h>
@@ -543,6 +544,157 @@ void test_filtered_scan_operator_matrix() {
     }
 }
 
+
+// v2.8.1 — concurrent reads must not rebind a cached handle.
+//
+// lmdb.h: "This function [mdb_dbi_open] must not be called from multiple
+// concurrent transactions in the same process. A transaction that uses this
+// function must finish (either commit or abort) before any other transaction in
+// the process may use this function."
+//
+// The old read path violated that on every read of a collection this process had
+// not yet written, from every gRPC thread at once. MDB_dbi is an index into the
+// env's shared handle table, so churning it silently REBOUND handles that
+// committed writes had already cached. Live on 2.8.0-3 that produced 41 refusals
+// in 45 minutes, all "caller asked for 'image_hashes' but the handle addresses
+// 'nsfw_images'" - and because each refusal bumped mirror drift, every read in
+// the process fell back to MemoryStore permanently.
+//
+// The fix primes all handles in one committed write txn at construction, so only
+// write txns ever call mdb_dbi_open and LMDB serialises those itself.
+//
+// HONEST LIMITATION: this test does NOT reproduce the race. Reverting the fix
+// leaves it green apart from the primed_count assertion - the torn slot-table
+// update needs an interleaving of concurrent mdb_dbi_open calls that 8 threads
+// over 10 collections does not reliably hit. What the test does pin is the
+// invariant the race violates (no read returns another collection's document, no
+// write is refused by the sentinel) plus the mechanism that removes the race:
+// handles are opened up front, so the read path has no mdb_dbi_open left to call.
+// The deterministic evidence is the documented contract in lmdb.h and the
+// production log.
+void test_concurrent_reads_do_not_rebind_cached_handles() {
+    const std::string path = make_tmpdir("dbi-concurrent");
+    const LmdbEnvOpts opts{path, 64ULL << 20, 256, 126, false};
+    struct Cleanup {
+        const std::string& p;
+        ~Cleanup() { std::error_code ec; fs::remove_all(p, ec); }
+    } cleanup{path};
+
+    // Enough collections that slot churn has somewhere to go.
+    const std::vector<std::string> colls = {
+        "alpha", "beta", "gamma", "delta", "epsilon",
+        "zeta", "eta", "theta", "iota", "kappa"};
+
+    {
+        LmdbEnv env(opts);
+        LmdbDocumentStore writer(env);
+        for (const auto& c : colls) {
+            Document d;
+            d.id = "seed";
+            d.collection = c;
+            d.set_data(nlohmann::json{{"who", c}});
+            writer.put(c, "seed", d);
+        }
+    }
+
+    // Restart: fresh env, so the shared handle table starts empty and the store
+    // must prime it.
+    LmdbEnv env2(opts);
+    LmdbDocumentStore store(env2);
+    check(store.prime_error().empty(), "priming succeeded on reopen");
+    check(store.primed_count() >= colls.size(),
+          "priming opened a handle for every existing sub-db");
+
+    // Hammer reads from many threads. Every one of these used to call
+    // mdb_dbi_open inside its own read txn.
+    std::atomic<int> read_failures{0};
+    std::atomic<int> wrong_data{0};
+    {
+        std::vector<std::thread> threads;
+        for (int t = 0; t < 8; ++t) {
+            threads.emplace_back([&, t]() {
+                for (int i = 0; i < 40; ++i) {
+                    const auto& c = colls[(t + i) % colls.size()];
+                    try {
+                        auto got = store.get(c, "seed");
+                        if (!got) { ++read_failures; continue; }
+                        // A rebound handle reads a STRANGER's sub-db, so the
+                        // document that comes back belongs to another collection.
+                        if (got->data().value("who", std::string{}) != c) ++wrong_data;
+                    } catch (const std::exception&) {
+                        ++read_failures;
+                    }
+                }
+            });
+        }
+        for (auto& th : threads) th.join();
+    }
+    check(read_failures.load() == 0, "concurrent reads all succeeded");
+    check(wrong_data.load() == 0,
+          "no read returned another collection's document - a rebound handle "
+          "addresses whichever sub-db now occupies its slot");
+
+    // Now write to every collection through the cached handles. This is where
+    // the live failure surfaced: the sentinel refused the write.
+    int write_failures = 0;
+    for (const auto& c : colls) {
+        try {
+            Document d;
+            d.id = "after";
+            d.collection = c;
+            d.set_data(nlohmann::json{{"who", c}});
+            store.put(c, "after", d);
+        } catch (const std::exception&) {
+            ++write_failures;
+        }
+    }
+    check(write_failures == 0,
+          "writes through primed handles are not refused by the identity "
+          "sentinel - the refusal is what production saw");
+
+    for (const auto& c : colls) {
+        check(store.count(c) == 2, ("both documents readable in " + c).c_str());
+    }
+}
+
+// A collection that exists on disk but has NOT been written by this process must
+// still be readable. The read path no longer opens handles on demand, so if
+// priming missed anything a read would report the collection as EMPTY - a
+// silent-wrong-data failure worse than the bug being fixed.
+void test_existing_collection_readable_without_writing_first() {
+    const std::string path = make_tmpdir("dbi-prime-read");
+    const LmdbEnvOpts opts{path, 64ULL << 20, 256, 126, false};
+    struct Cleanup {
+        const std::string& p;
+        ~Cleanup() { std::error_code ec; fs::remove_all(p, ec); }
+    } cleanup{path};
+
+    {
+        LmdbEnv env(opts);
+        LmdbDocumentStore writer(env);
+        Document d;
+        d.id = "only";
+        d.collection = "archive";
+        d.set_data(nlohmann::json{{"kept", true}});
+        writer.put("archive", "only", d);
+    }
+
+    LmdbEnv env2(opts);
+    LmdbDocumentStore reader(env2);
+
+    // Read-only access, never a write on this collection in this process.
+    auto got = reader.get("archive", "only");
+    check(got.has_value(), "a never-written-here collection is still readable");
+    check(reader.count("archive") == 1, "count sees it");
+    smartbotic::database::Query q; q.limit = 10;
+    check(reader.scan("archive", q).documents.size() == 1, "scan sees it");
+
+    // And a collection that genuinely does not exist still reads as absent.
+    check(!reader.get("nosuch", "x").has_value(),
+          "a missing collection is still absent, not an error");
+    check(reader.count("nosuch") == 0, "and counts zero");
+}
+
 }  // namespace
 
 int main() {
@@ -557,6 +709,8 @@ int main() {
     test_scan_fast_path_matches_general_path();
     test_aborted_write_does_not_poison_the_collection();
     test_filtered_scan_operator_matrix();
+    test_concurrent_reads_do_not_rebind_cached_handles();
+    test_existing_collection_readable_without_writing_first();
 
     std::cout << "passed: " << g_pass << ", failed: " << g_fail << "\n";
     return g_fail == 0 ? 0 : 1;