diff --git a/ChangeLog.md b/ChangeLog.md index 1622a622..1ddb5d30 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -32,6 +32,8 @@ Fixes: - Fixed 4-byte aligned access to 64-bit integers. - Fixed missing cleanup (null assigned) in the C++ commit/abort (https://github.com/erthink/libmdbx/pull/143). + - Fixed `mdbx_realloc()` for case of nullptr and `MDBX_AVOID_CRT=ON` for Windows. + - Fixed the possibility to use invalid and renewed (closed & re-opened, dropped & re-created) DBI-handles (https://github.com/erthink/libmdbx/issues/146). ## v0.9.2 scheduled at 2020-11-27 diff --git a/src/core.c b/src/core.c index 2c064b16..dff3ac56 100644 --- a/src/core.c +++ b/src/core.c @@ -6881,13 +6881,50 @@ int mdbx_txn_flags(const MDBX_txn *txn) { return txn->mt_flags; } +/* Check for misused dbi handles */ +#define TXN_DBI_CHANGED(txn, dbi) \ + ((txn)->mt_dbiseqs[dbi] != (txn)->mt_env->me_dbiseqs[dbi]) + +static void dbi_import_locked(MDBX_txn *txn) { + MDBX_env *const env = txn->mt_env; + const unsigned n = env->me_numdbs; + for (unsigned i = CORE_DBS; i < n; ++i) { + if (i >= txn->mt_numdbs) { + txn->mt_dbistate[i] = 0; + if (!(txn->mt_flags & MDBX_TXN_RDONLY)) + txn->tw.cursors[i] = NULL; + } + if ((env->me_dbflags[i] & DB_VALID) && + !(txn->mt_dbistate[i] & DBI_USRVALID)) { + txn->mt_dbiseqs[i] = env->me_dbiseqs[i]; + txn->mt_dbs[i].md_flags = env->me_dbflags[i] & DB_PERSISTENT_FLAGS; + txn->mt_dbistate[i] = DBI_VALID | DBI_USRVALID | DBI_STALE; + mdbx_tassert(txn, txn->mt_dbxs[i].md_cmp != NULL); + } + } + txn->mt_numdbs = n; +} + +/* Import DBI which opened after txn started into context */ +static __cold bool dbi_import(MDBX_txn *txn, MDBX_dbi dbi) { + if (dbi < CORE_DBS || dbi >= txn->mt_env->me_numdbs) + return false; + + mdbx_ensure(txn->mt_env, mdbx_fastmutex_acquire(&txn->mt_env->me_dbi_lock) == + MDBX_SUCCESS); + dbi_import_locked(txn); + mdbx_ensure(txn->mt_env, mdbx_fastmutex_release(&txn->mt_env->me_dbi_lock) == + MDBX_SUCCESS); + return txn->mt_dbistate[dbi] & DBI_USRVALID; +} + /* Export or close DBI handles opened in this txn. */ -static void mdbx_dbis_update(MDBX_txn *txn, int keep) { +static void dbi_update(MDBX_txn *txn, int keep) { mdbx_tassert(txn, !txn->mt_parent && txn == txn->mt_env->me_txn0); MDBX_dbi n = txn->mt_numdbs; if (n) { bool locked = false; - MDBX_env *env = txn->mt_env; + MDBX_env *const env = txn->mt_env; for (unsigned i = n; --i >= CORE_DBS;) { if (likely((txn->mt_dbistate[i] & DBI_CREAT) == 0)) @@ -6897,11 +6934,10 @@ static void mdbx_dbis_update(MDBX_txn *txn, int keep) { mdbx_fastmutex_acquire(&env->me_dbi_lock) == MDBX_SUCCESS); locked = true; } + if (env->me_numdbs <= i || txn->mt_dbiseqs[i] != env->me_dbiseqs[i]) + continue /* dbi explicitly closed and/or then re-opened by other txn */; if (keep) { env->me_dbflags[i] = txn->mt_dbs[i].md_flags | DB_VALID; - mdbx_compiler_barrier(); - if (env->me_numdbs <= i) - env->me_numdbs = i + 1; } else { char *ptr = env->me_dbxs[i].md_name.iov_base; if (ptr) { @@ -6915,6 +6951,20 @@ static void mdbx_dbis_update(MDBX_txn *txn, int keep) { } } + n = env->me_numdbs; + if (n > CORE_DBS && unlikely(!(env->me_dbflags[n - 1] & DB_VALID))) { + if (!locked) { + mdbx_ensure(env, + mdbx_fastmutex_acquire(&env->me_dbi_lock) == MDBX_SUCCESS); + locked = true; + } + + n = env->me_numdbs; + while (n > CORE_DBS && !(env->me_dbflags[n - 1] & DB_VALID)) + --n; + env->me_numdbs = n; + } + if (unlikely(locked)) mdbx_ensure(env, mdbx_fastmutex_release(&env->me_dbi_lock) == MDBX_SUCCESS); @@ -6996,7 +7046,7 @@ static int mdbx_txn_end(MDBX_txn *txn, unsigned mode) { if (txn == env->me_txn0) { mdbx_assert(env, txn->mt_parent == NULL); /* Export or close DBI handles created in this txn */ - mdbx_dbis_update(txn, mode & MDBX_END_UPDATE); + dbi_update(txn, mode & MDBX_END_UPDATE); mdbx_pnl_shrink(&txn->tw.retired_pages); mdbx_pnl_shrink(&txn->tw.reclaimed_pglist); /* The writer mutex was locked in mdbx_txn_begin. */ @@ -8096,42 +8146,13 @@ __hot static int mdbx_page_flush(MDBX_txn *txn, const size_t keep) { return MDBX_SUCCESS; } -/* Check for misused dbi handles */ -#define TXN_DBI_CHANGED(txn, dbi) \ - ((txn)->mt_dbiseqs[dbi] != (txn)->mt_env->me_dbiseqs[dbi]) - -/* Import DBI which opened after txn started into context */ -static __cold bool mdbx_txn_import_dbi(MDBX_txn *txn, MDBX_dbi dbi) { - MDBX_env *env = txn->mt_env; - if (dbi < CORE_DBS || dbi >= env->me_numdbs) - return false; - - mdbx_ensure(env, mdbx_fastmutex_acquire(&env->me_dbi_lock) == MDBX_SUCCESS); - const unsigned snap_numdbs = env->me_numdbs; - mdbx_compiler_barrier(); - for (unsigned i = CORE_DBS; i < snap_numdbs; ++i) { - if (i >= txn->mt_numdbs) - txn->mt_dbistate[i] = 0; - if (!(txn->mt_dbistate[i] & DBI_USRVALID) && - (env->me_dbflags[i] & DB_VALID)) { - txn->mt_dbs[i].md_flags = env->me_dbflags[i] & DB_PERSISTENT_FLAGS; - txn->mt_dbistate[i] = DBI_VALID | DBI_USRVALID | DBI_STALE; - mdbx_tassert(txn, txn->mt_dbxs[i].md_cmp != NULL); - } - } - txn->mt_numdbs = snap_numdbs; - - mdbx_ensure(env, mdbx_fastmutex_release(&env->me_dbi_lock) == MDBX_SUCCESS); - return txn->mt_dbistate[dbi] & DBI_USRVALID; -} - /* Check txn and dbi arguments to a function */ static __always_inline bool mdbx_txn_dbi_exists(MDBX_txn *txn, MDBX_dbi dbi, unsigned validity) { if (likely(dbi < txn->mt_numdbs && (txn->mt_dbistate[dbi] & validity))) return true; - return mdbx_txn_import_dbi(txn, dbi); + return dbi_import(txn, dbi); } int mdbx_txn_commit(MDBX_txn *txn) { return __inline_mdbx_txn_commit(txn); } @@ -12712,6 +12733,9 @@ int mdbx_cursor_put(MDBX_cursor *mc, const MDBX_val *key, MDBX_val *data, if (unlikely(rc != MDBX_SUCCESS)) return rc; + if (unlikely(TXN_DBI_CHANGED(mc->mc_txn, mc->mc_dbi))) + return MDBX_BAD_DBI; + mdbx_cassert(mc, cursor_is_tracked(mc)); env = mc->mc_txn->mt_env; @@ -13511,6 +13535,9 @@ int mdbx_cursor_del(MDBX_cursor *mc, MDBX_put_flags_t flags) { if (unlikely(rc != MDBX_SUCCESS)) return rc; + if (unlikely(TXN_DBI_CHANGED(mc->mc_txn, mc->mc_dbi))) + return MDBX_BAD_DBI; + if (unlikely(!(mc->mc_flags & C_INITIALIZED))) return MDBX_ENODATA; @@ -14147,6 +14174,9 @@ static __inline int mdbx_couple_init(MDBX_cursor_couple *couple, /* Initialize a cursor for a given transaction and database. */ static int mdbx_cursor_init(MDBX_cursor *mc, MDBX_txn *txn, MDBX_dbi dbi) { STATIC_ASSERT(offsetof(MDBX_cursor_couple, outer) == 0); + if (unlikely(TXN_DBI_CHANGED(txn, dbi))) + return MDBX_BAD_DBI; + return mdbx_couple_init(container_of(mc, MDBX_cursor_couple, outer), dbi, txn, &txn->mt_dbs[dbi], &txn->mt_dbxs[dbi], &txn->mt_dbistate[dbi]); @@ -14318,16 +14348,19 @@ again: } void mdbx_cursor_close(MDBX_cursor *mc) { - if (mc) { + if (likely(mc)) { mdbx_ensure(NULL, mc->mc_signature == MDBX_MC_LIVE || mc->mc_signature == MDBX_MC_READY4CLOSE); + MDBX_txn *const txn = mc->mc_txn; + MDBX_env *const env = txn ? txn->mt_env : NULL; if (!mc->mc_backup) { + mc->mc_txn = NULL; /* Remove from txn, if tracked. * A read-only txn (!C_UNTRACK) may have been freed already, * so do not peek inside it. Only write txns track cursors. */ if (mc->mc_flags & C_UNTRACK) { - mdbx_cassert(mc, !(mc->mc_txn->mt_flags & MDBX_TXN_RDONLY)); - MDBX_cursor **prev = &mc->mc_txn->tw.cursors[mc->mc_dbi]; + mdbx_ensure(env, check_txn_rw(txn, 0) == MDBX_SUCCESS); + MDBX_cursor **prev = &txn->tw.cursors[mc->mc_dbi]; while (*prev && *prev != mc) prev = &(*prev)->mc_next; mdbx_cassert(mc, *prev == mc); @@ -14339,6 +14372,7 @@ void mdbx_cursor_close(MDBX_cursor *mc) { } else { /* Cursor closed before nested txn ends */ mdbx_cassert(mc, mc->mc_signature == MDBX_MC_LIVE); + mdbx_ensure(env, check_txn_rw(txn, 0) == MDBX_SUCCESS); mc->mc_signature = MDBX_MC_WAIT4EOT; } } @@ -17577,18 +17611,8 @@ static int dbi_open(MDBX_txn *txn, const char *table_name, unsigned user_flags, goto early_bailout; } - if (txn->mt_numdbs < env->me_numdbs) { - /* Import handles from env */ - for (unsigned i = txn->mt_numdbs; i < env->me_numdbs; ++i) { - txn->mt_dbistate[i] = 0; - if (env->me_dbflags[i] & DB_VALID) { - txn->mt_dbs[i].md_flags = env->me_dbflags[i] & DB_PERSISTENT_FLAGS; - txn->mt_dbistate[i] = DBI_VALID | DBI_USRVALID | DBI_STALE; - mdbx_tassert(txn, txn->mt_dbxs[i].md_cmp != NULL); - } - } - txn->mt_numdbs = env->me_numdbs; - } + /* Import handles from env */ + dbi_import_locked(txn); /* Rescan after mutex acquisition & import handles */ for (slot = scan = txn->mt_numdbs; --scan >= CORE_DBS;) { @@ -17648,18 +17672,16 @@ static int dbi_open(MDBX_txn *txn, const char *table_name, unsigned user_flags, txn->mt_dbistate[slot] = (uint8_t)dbiflags; txn->mt_dbxs[slot].md_name.iov_base = namedup; txn->mt_dbxs[slot].md_name.iov_len = len; - if ((txn->mt_flags & MDBX_TXN_RDONLY) == 0) - txn->tw.cursors[slot] = NULL; - txn->mt_numdbs += (slot == txn->mt_numdbs); - if ((dbiflags & DBI_CREAT) == 0) { + txn->mt_dbiseqs[slot] = ++env->me_dbiseqs[slot]; + if (!(dbiflags & DBI_CREAT)) env->me_dbflags[slot] = txn->mt_dbs[slot].md_flags | DB_VALID; + if (txn->mt_numdbs == slot) { mdbx_compiler_barrier(); - if (env->me_numdbs <= slot) - env->me_numdbs = slot + 1; - } else { - env->me_dbiseqs[slot]++; + txn->mt_numdbs = env->me_numdbs = slot + 1; + if (!(txn->mt_flags & MDBX_TXN_RDONLY)) + txn->tw.cursors[slot] = NULL; } - txn->mt_dbiseqs[slot] = env->me_dbiseqs[slot]; + mdbx_assert(env, env->me_numdbs > slot); *dbi = slot; } @@ -17717,10 +17739,15 @@ static int mdbx_dbi_close_locked(MDBX_env *env, MDBX_dbi dbi) { return MDBX_BAD_DBI; env->me_dbflags[dbi] = 0; + env->me_dbiseqs[dbi]++; env->me_dbxs[dbi].md_name.iov_len = 0; mdbx_compiler_barrier(); env->me_dbxs[dbi].md_name.iov_base = NULL; mdbx_free(ptr); + + if (env->me_numdbs == dbi + 1) + env->me_numdbs = dbi; + return MDBX_SUCCESS; } @@ -17734,7 +17761,9 @@ int mdbx_dbi_close(MDBX_env *env, MDBX_dbi dbi) { rc = mdbx_fastmutex_acquire(&env->me_dbi_lock); if (likely(rc == MDBX_SUCCESS)) { - rc = mdbx_dbi_close_locked(env, dbi); + rc = (dbi < env->me_maxdbs && (env->me_dbflags[dbi] & DB_VALID)) + ? mdbx_dbi_close_locked(env, dbi) + : MDBX_BAD_DBI; mdbx_ensure(env, mdbx_fastmutex_release(&env->me_dbi_lock) == MDBX_SUCCESS); } return rc; @@ -17900,7 +17929,6 @@ int mdbx_drop(MDBX_txn *txn, MDBX_dbi dbi, bool del) { txn->mt_flags |= MDBX_TXN_ERROR; goto bailout; } - env->me_dbiseqs[dbi]++; mdbx_dbi_close_locked(env, dbi); mdbx_ensure(env, mdbx_fastmutex_release(&env->me_dbi_lock) == MDBX_SUCCESS); diff --git a/test/jitter.cc b/test/jitter.cc index 71d58699..36066e76 100644 --- a/test/jitter.cc +++ b/test/jitter.cc @@ -14,6 +14,14 @@ #include "test.h" +void testcase_jitter::check_dbi_error(int expect, const char *stage) { + MDBX_stat stat; + int err = mdbx_dbi_stat(txn_guard.get(), dbi, &stat, sizeof(stat)); + if (err != expect) + failure("unexpected result for %s dbi-handle: expect %d, got %d", stage, + expect, err); +} + bool testcase_jitter::run() { int err; size_t upper_limit = config.params.size_upper; @@ -24,6 +32,47 @@ bool testcase_jitter::run() { jitter_delay(); db_open(); + if (!dbi && !mode_readonly()) { + // create table + txn_begin(false); + dbi = db_table_open(true); + check_dbi_error(MDBX_SUCCESS, "created-uncommitted"); + // note: here and below the 4-byte length keys and value are used + // to be compatible with any Db-flags given from command line. + MDBX_val key = {(void *)"k000", 4}, value = {(void *)"v001", 4}; + err = mdbx_put(txn_guard.get(), dbi, &key, &value, MDBX_UPSERT); + if (err != MDBX_SUCCESS) + failure_perror("jitter.put-1", err); + txn_end(false); + + // drop & re-create table, but abort txn + txn_begin(false); + check_dbi_error(MDBX_SUCCESS, "created-committed"); + err = mdbx_drop(txn_guard.get(), dbi, true); + if (unlikely(err != MDBX_SUCCESS)) + failure_perror("mdbx_drop(delete=true)", err); + check_dbi_error(MDBX_BAD_DBI, "dropped-uncommitted"); + dbi = db_table_open(true); + check_dbi_error(MDBX_SUCCESS, "recreated-uncommitted"); + txn_end(true); + + // check after aborted txn + txn_begin(false); + value = {(void *)"v002", 4}; + err = mdbx_put(txn_guard.get(), dbi, &key, &value, MDBX_UPSERT); + if (err != MDBX_BAD_DBI) + failure_perror("jitter.put-2", err); + check_dbi_error(MDBX_BAD_DBI, "dropped-recreated-aborted"); + // restore DBI + dbi = db_table_open(false); + check_dbi_error(MDBX_SUCCESS, "dropped-recreated-aborted+reopened"); + value = {(void *)"v003", 4}; + err = mdbx_put(txn_guard.get(), dbi, &key, &value, MDBX_UPSERT); + if (err != MDBX_SUCCESS) + failure_perror("jitter.put-3", err); + txn_end(false); + } + if (upper_limit < 1) { MDBX_envinfo info; err = mdbx_env_info_ex(db_guard.get(), txn_guard.get(), &info, diff --git a/test/test.h b/test/test.h index bcc33209..bd349178 100644 --- a/test/test.h +++ b/test/test.h @@ -287,6 +287,9 @@ public: }; class testcase_jitter : public testcase { +protected: + void check_dbi_error(int expect, const char *stage); + public: testcase_jitter(const actor_config &config, const mdbx_pid_t pid) : testcase(config, pid) {}