From acdaeeab5c7b2c9543936c48fd6d8d4ae5ebab3d Mon Sep 17 00:00:00 2001 From: Leo Yuriev Date: Wed, 15 Feb 2017 19:50:23 +0300 Subject: [PATCH 1/7] mdbx: notes about free/reuse cursors. --- lmdb.h | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/lmdb.h b/lmdb.h index b6a55f11..0fded84e 100644 --- a/lmdb.h +++ b/lmdb.h @@ -1039,8 +1039,16 @@ size_t mdb_txn_id(MDB_txn *txn); * * The transaction handle is freed. It and its cursors must not be used * again after this call, except with #mdb_cursor_renew(). - * @note Earlier documentation incorrectly said all cursors would be freed. + * + * @note MDBX-mode: + * A cursor must be closed explicitly always, before + * or after its transaction ends. It can be reused with + * #mdb_cursor_renew() before finally closing it. + * + * @note LMDB-compatible mode: + * Earlier documentation incorrectly said all cursors would be freed. * Only write-transactions free cursors. + * * @param[in] txn A transaction handle returned by #mdb_txn_begin() * @return A non-zero error value on failure and 0 on success. Some possible * errors are: @@ -1057,8 +1065,16 @@ int mdb_txn_commit(MDB_txn *txn); * * The transaction handle is freed. It and its cursors must not be used * again after this call, except with #mdb_cursor_renew(). - * @note Earlier documentation incorrectly said all cursors would be freed. + * + * @note MDBX-mode: + * A cursor must be closed explicitly always, before + * or after its transaction ends. It can be reused with + * #mdb_cursor_renew() before finally closing it. + * + * @note LMDB-compatible mode: + * Earlier documentation incorrectly said all cursors would be freed. * Only write-transactions free cursors. + * * @param[in] txn A transaction handle returned by #mdb_txn_begin() */ int mdb_txn_abort(MDB_txn *txn); From cf8ef06ebceba9c510d15c8f9b17ddccc3838631 Mon Sep 17 00:00:00 2001 From: Leo Yuriev Date: Wed, 15 Feb 2017 20:04:20 +0300 Subject: [PATCH 2/7] mdbx: 'unlikely' for DB_STALE. --- mdb.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/mdb.c b/mdb.c index ce088c72..8edd5e5b 100644 --- a/mdb.c +++ b/mdb.c @@ -5763,7 +5763,7 @@ mdb_page_search(MDB_cursor *mc, MDB_val *key, int flags) return MDB_BAD_TXN; } else { /* Make sure we're using an up-to-date root */ - if (*mc->mc_dbflag & DB_STALE) { + if (unlikely(*mc->mc_dbflag & DB_STALE)) { MDB_cursor mc2; if (unlikely(TXN_DBI_CHANGED(mc->mc_txn, mc->mc_dbi))) return MDB_BAD_DBI; @@ -7826,15 +7826,14 @@ mdb_cursor_init(MDB_cursor *mc, MDB_txn *txn, MDB_dbi dbi, MDB_xcursor *mx) mc->mc_pg[0] = 0; mc->mc_flags = 0; mc->mc_ki[0] = 0; + mc->mc_xcursor = NULL; if (txn->mt_dbs[dbi].md_flags & MDB_DUPSORT) { mdb_tassert(txn, mx != NULL); mx->mx_cursor.mc_signature = MDBX_MC_SIGNATURE; mc->mc_xcursor = mx; mdb_xcursor_init0(mc); - } else { - mc->mc_xcursor = NULL; } - if (*mc->mc_dbflag & DB_STALE) { + if (unlikely(*mc->mc_dbflag & DB_STALE)) { mdb_page_search(mc, NULL, MDB_PS_ROOTONLY); } } @@ -10226,7 +10225,7 @@ mdbx_stat(MDB_txn *txn, MDB_dbi dbi, MDBX_stat *arg, size_t bytes) if (unlikely(txn->mt_flags & MDB_TXN_BLOCKED)) return MDB_BAD_TXN; - if (txn->mt_dbflags[dbi] & DB_STALE) { + if (unlikely(txn->mt_dbflags[dbi] & DB_STALE)) { MDB_cursor mc; MDB_xcursor mx; /* Stale, must read the DB's root. cursor_init does it for us. */ From d47d8c25d8e2af129a67217559a21c107db2cdeb Mon Sep 17 00:00:00 2001 From: Leo Yuriev Date: Wed, 15 Feb 2017 20:05:26 +0300 Subject: [PATCH 3/7] mdbx: drop unused 'flags' from mdb_env_walk(). --- mdbx.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/mdbx.c b/mdbx.c index 71fd224b..3599d14b 100644 --- a/mdbx.c +++ b/mdbx.c @@ -172,7 +172,7 @@ typedef struct mdb_walk_ctx { /** Depth-first tree traversal. */ static int __cold -mdb_env_walk(mdb_walk_ctx_t *ctx, const char* dbi, pgno_t pg, int flags, int deep) +mdb_env_walk(mdb_walk_ctx_t *ctx, const char* dbi, pgno_t pg, int deep) { MDB_page *mp; int rc, i, nkeys; @@ -238,7 +238,7 @@ mdb_env_walk(mdb_walk_ctx_t *ctx, const char* dbi, pgno_t pg, int flags, int dee payload_size += NODESIZE + node->mn_ksize; if (IS_BRANCH(mp)) { - rc = mdb_env_walk(ctx, dbi, NODEPGNO(node), flags, deep); + rc = mdb_env_walk(ctx, dbi, NODEPGNO(node), deep); if (rc) return rc; continue; @@ -286,7 +286,7 @@ mdb_env_walk(mdb_walk_ctx_t *ctx, const char* dbi, pgno_t pg, int flags, int dee name[namelen] = 0; } rc = mdb_env_walk(ctx, (name && name[0]) ? name : dbi, - db->md_root, node->mn_flags & F_DUPDATA, deep + 1); + db->md_root, deep + 1); if (rc) return rc; } @@ -314,9 +314,9 @@ mdbx_env_pgwalk(MDB_txn *txn, MDBX_pgvisitor_func* visitor, void* user) rc = visitor(0, 2, user, "lmdb", "meta", 2, sizeof(MDB_meta)*2, PAGEHDRSZ*2, (txn->mt_env->me_psize - sizeof(MDB_meta) - PAGEHDRSZ) *2); if (! rc) - rc = mdb_env_walk(&ctx, "free", txn->mt_dbs[FREE_DBI].md_root, 0, 0); + rc = mdb_env_walk(&ctx, "free", txn->mt_dbs[FREE_DBI].md_root, 0); if (! rc) - rc = mdb_env_walk(&ctx, "main", txn->mt_dbs[MAIN_DBI].md_root, 0, 0); + rc = mdb_env_walk(&ctx, "main", txn->mt_dbs[MAIN_DBI].md_root, 0); if (! rc) rc = visitor(P_INVALID, 0, user, NULL, NULL, 0, 0, 0, 0); return rc; From 2b924524ecd58e7973f8df45df1c1d10f41526dc Mon Sep 17 00:00:00 2001 From: Leo Yuriev Date: Wed, 15 Feb 2017 20:16:07 +0300 Subject: [PATCH 4/7] mdbx: initial mdbx_cursor_on_ first/last(). --- mdbx.c | 41 +++++++++++++++++++++++++++++++++++++++++ mdbx.h | 13 +++++++++++-- 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/mdbx.c b/mdbx.c index 3599d14b..d32394a5 100644 --- a/mdbx.c +++ b/mdbx.c @@ -354,6 +354,47 @@ size_t mdbx_canary_get(MDB_txn *txn, mdbx_canary* canary) return txn->mt_txnid; } +int mdbx_cursor_on_first(MDB_cursor *mc) +{ + if (unlikely(mc == NULL)) + return EINVAL; + + if (unlikely(mc->mc_signature != MDBX_MC_SIGNATURE)) + return MDB_VERSION_MISMATCH; + + if (!(mc->mc_flags & C_INITIALIZED)) + return MDBX_RESULT_FALSE; + + unsigned i; + for(i = 0; i < mc->mc_snum; ++i) { + if (mc->mc_ki[i]) + return MDBX_RESULT_FALSE; + } + + return MDBX_RESULT_TRUE; +} + +int mdbx_cursor_on_last(MDB_cursor *mc) +{ + if (unlikely(mc == NULL)) + return EINVAL; + + if (unlikely(mc->mc_signature != MDBX_MC_SIGNATURE)) + return MDB_VERSION_MISMATCH; + + if (!(mc->mc_flags & C_INITIALIZED)) + return MDBX_RESULT_FALSE; + + unsigned i; + for(i = 0; i < mc->mc_snum; ++i) { + unsigned nkeys = NUMKEYS(mc->mc_pg[i]); + if (mc->mc_ki[i] != nkeys - 1) + return MDBX_RESULT_FALSE; + } + + return MDBX_RESULT_TRUE; +} + int mdbx_cursor_eof(MDB_cursor *mc) { if (unlikely(mc == NULL)) diff --git a/mdbx.h b/mdbx.h index 6a266395..515e819e 100644 --- a/mdbx.h +++ b/mdbx.h @@ -219,10 +219,19 @@ typedef struct mdbx_canary { int mdbx_canary_put(MDB_txn *txn, const mdbx_canary* canary); size_t mdbx_canary_get(MDB_txn *txn, mdbx_canary* canary); -/* Returns 1 when no more data available or cursor not positioned, - * 0 otherwise or less that zero in error case. */ +/* Returns: + * - MDBX_RESULT_TRUE when no more data available + * or cursor not positioned; + * - MDBX_RESULT_FALSE when data available; + * - Otherwise the error code. */ int mdbx_cursor_eof(MDB_cursor *mc); +/* Returns: MDBX_RESULT_TRUE, MDBX_RESULT_FALSE or Error code. */ +int mdbx_cursor_on_first(MDB_cursor *mc); + +/* Returns: MDBX_RESULT_TRUE, MDBX_RESULT_FALSE or Error code. */ +int mdbx_cursor_on_last(MDB_cursor *mc); + #define MDBX_EMULTIVAL (MDB_LAST_ERRCODE - 42) #define MDBX_RESULT_FALSE MDB_SUCCESS #define MDBX_RESULT_TRUE (-1) From f75fa27fe60837e88a634111508dc555f3eeafaf Mon Sep 17 00:00:00 2001 From: Leo Yuriev Date: Wed, 15 Feb 2017 20:54:58 +0300 Subject: [PATCH 5/7] mdbx: don't close DBI-handles from R/O txn_abort(). This is related to: - https://github.com/ReOpen/ReOpenLDAP/issues/119 (its6794 regression test may fail) - https://github.com/ReOpen/ReOpenLDAP/issues/92 (rare test060-mt-hot failures) --- mdb.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/mdb.c b/mdb.c index 8edd5e5b..cf4a3e2d 100644 --- a/mdb.c +++ b/mdb.c @@ -3256,6 +3256,12 @@ mdb_txn_abort(MDB_txn *txn) if(unlikely(txn->mt_signature != MDBX_MT_SIGNATURE)) return MDB_VERSION_MISMATCH; +#if MDBX_MODE_ENABLED + if (F_ISSET(txn->mt_flags, MDB_TXN_RDONLY)) + /* LY: don't close DBI-handles in MDBX mode */ + return mdb_txn_end(txn, MDB_END_UPDATE|MDB_END_SLOT|MDB_END_FREE); +#endif /* MDBX_MODE_ENABLED */ + if (txn->mt_child) mdb_txn_abort(txn->mt_child); From 41576e553cbc68737be14220a4042a36d2dc8842 Mon Sep 17 00:00:00 2001 From: Leo Yuriev Date: Thu, 16 Feb 2017 14:07:38 +0300 Subject: [PATCH 6/7] mdbx: fix cursor-untrack bug. Fix segfault possibility, during closing the cursor after a write-txn. --- mdb.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/mdb.c b/mdb.c index cf4a3e2d..00597d92 100644 --- a/mdb.c +++ b/mdb.c @@ -2700,8 +2700,9 @@ mdb_cursors_eot(MDB_txn *txn, unsigned merge) for (i = txn->mt_numdbs; --i >= 0; ) { for (mc = cursors[i]; mc; mc = next) { - mdb_ensure(NULL, mc->mc_signature == MDBX_MC_SIGNATURE - || mc->mc_signature == MDBX_MC_WAIT4EOT); + unsigned stage = mc->mc_signature; + mdb_ensure(NULL, stage == MDBX_MC_SIGNATURE + || stage == MDBX_MC_WAIT4EOT); next = mc->mc_next; if ((bk = mc->mc_backup) != NULL) { if (merge) { @@ -2714,10 +2715,8 @@ mdb_cursors_eot(MDB_txn *txn, unsigned merge) if ((mx = mc->mc_xcursor) != NULL) mx->mx_cursor.mc_txn = bk->mc_txn; } else { - /* Abort nested txn, but save current cursor's stage */ - unsigned stage = mc->mc_signature; + /* Abort nested txn */ *mc = *bk; - mc->mc_signature = stage; if ((mx = mc->mc_xcursor) != NULL) *mx = *(MDB_xcursor *)(bk+1); } @@ -2725,11 +2724,12 @@ mdb_cursors_eot(MDB_txn *txn, unsigned merge) bk->mc_signature = 0; free(bk); } - if (mc->mc_signature == MDBX_MC_WAIT4EOT) { + if (stage == MDBX_MC_WAIT4EOT) { mc->mc_signature = 0; free(mc); } else { mc->mc_signature = MDBX_MC_READY4CLOSE; + mc->mc_flags = 0 /* reset C_UNTRACK */; } #else mc = bk; From 17080256510e23b6162db0cf2ca09e43d0dfac42 Mon Sep 17 00:00:00 2001 From: Leo Yuriev Date: Thu, 16 Feb 2017 14:15:47 +0300 Subject: [PATCH 7/7] mdbx: don't close/lost DBI-handles on ro-txn renew/reset. More for d0383e5aeeb29fc07b568a0d4b5287d539be5d0d Also this is related to: - https://github.com/ReOpen/ReOpenLDAP/issues/119 (its6794 regression test may fail) - https://github.com/ReOpen/ReOpenLDAP/issues/92 (rare test060-mt-hot failures) --- mdb.c | 7 ++++++- mdbx.c | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/mdb.c b/mdb.c index 00597d92..06ec817b 100644 --- a/mdb.c +++ b/mdb.c @@ -3244,7 +3244,12 @@ mdb_txn_reset(MDB_txn *txn) if (unlikely(!(txn->mt_flags & MDB_TXN_RDONLY))) return EINVAL; +#if MDBX_MODE_ENABLED + /* LY: don't close DBI-handles in MDBX mode */ + return mdb_txn_end(txn, MDB_END_RESET|MDB_END_UPDATE); +#else return mdb_txn_end(txn, MDB_END_RESET); +#endif /* MDBX_MODE_ENABLED */ } int @@ -3259,7 +3264,7 @@ mdb_txn_abort(MDB_txn *txn) #if MDBX_MODE_ENABLED if (F_ISSET(txn->mt_flags, MDB_TXN_RDONLY)) /* LY: don't close DBI-handles in MDBX mode */ - return mdb_txn_end(txn, MDB_END_UPDATE|MDB_END_SLOT|MDB_END_FREE); + return mdb_txn_end(txn, MDB_END_ABORT|MDB_END_UPDATE|MDB_END_SLOT|MDB_END_FREE); #endif /* MDBX_MODE_ENABLED */ if (txn->mt_child) diff --git a/mdbx.c b/mdbx.c index d32394a5..42ad6242 100644 --- a/mdbx.c +++ b/mdbx.c @@ -369,7 +369,7 @@ int mdbx_cursor_on_first(MDB_cursor *mc) for(i = 0; i < mc->mc_snum; ++i) { if (mc->mc_ki[i]) return MDBX_RESULT_FALSE; - } + } return MDBX_RESULT_TRUE; }