From 342d56977fee61abc56c75d817551d920f0e8eb0 Mon Sep 17 00:00:00 2001 From: Hallvard Furuseth Date: Sun, 19 Jul 2015 21:31:25 +0200 Subject: [PATCH] lmdb: Catch most uses of finished/parent txns. * Add MDB_TXN_FINISHED, MDB_TXN_HAS_CHILD, MDB_TXN_BLOCKED. * Clear mt_numdbs in writers, for TXN_DBI_EXIST() to catch. We already do in readers. Change-Id: I4f714c0789188dfab4ce22b7d9d3a75d8b62ef6b --- lmdb.h | 2 +- mdb.c | 52 +++++++++++++++++++++++++++++++--------------------- 2 files changed, 32 insertions(+), 22 deletions(-) diff --git a/lmdb.h b/lmdb.h index 64cd7934..1d912f1c 100644 --- a/lmdb.h +++ b/lmdb.h @@ -423,7 +423,7 @@ typedef enum MDB_cursor_op { #define MDB_INCOMPATIBLE (-30784) /** Invalid reuse of reader locktable slot */ #define MDB_BAD_RSLOT (-30783) - /** Transaction cannot recover - it must be aborted */ + /** Transaction must abort, has a child, or is invalid */ #define MDB_BAD_TXN (-30782) /** Unsupported size of key/DB name/data, or wrong DUPFIXED size */ #define MDB_BAD_VALSIZE (-30781) diff --git a/mdb.c b/mdb.c index aa52ec2b..cee98c96 100644 --- a/mdb.c +++ b/mdb.c @@ -868,7 +868,8 @@ typedef struct MDB_dbx { */ struct MDB_txn { MDB_txn *mt_parent; /**< parent of a nested txn */ - MDB_txn *mt_child; /**< nested txn under this txn */ + /** Nested txn under this txn, set together with flag #MDB_TXN_HAS_CHILD */ + MDB_txn *mt_child; pgno_t mt_next_pgno; /**< next unallocated page */ /** The ID of this transaction. IDs are integers incrementing from 1. * Only committed write transactions increment the ID. If a transaction @@ -918,8 +919,9 @@ struct MDB_txn { MDB_cursor **mt_cursors; /** Array of flags for each DB */ unsigned char *mt_dbflags; - /** Number of DB records in use. This number only ever increments; - * we don't decrement it when individual DB handles are closed. + /** Number of DB records in use, or 0 when the txn is finished. + * This number only ever increments until the txn finishes; we + * don't decrement it when individual DB handles are closed. */ MDB_dbi mt_numdbs; @@ -934,9 +936,13 @@ struct MDB_txn { #define MDB_TXN_RDONLY MDB_RDONLY /**< read-only transaction */ /* internal txn flags */ #define MDB_TXN_WRITEMAP MDB_WRITEMAP /**< copy of #MDB_env flag in writers */ +#define MDB_TXN_FINISHED 0x01 /**< txn is finished or never began */ #define MDB_TXN_ERROR 0x02 /**< txn is unusable after an error */ #define MDB_TXN_DIRTY 0x04 /**< must write, even if dirty list is empty */ #define MDB_TXN_SPILLS 0x08 /**< txn or a parent has spilled pages */ +#define MDB_TXN_HAS_CHILD 0x10 /**< txn has an #MDB_txn.%mt_child */ + /** most operations on the txn are currently illegal */ +#define MDB_TXN_BLOCKED (MDB_TXN_FINISHED|MDB_TXN_ERROR|MDB_TXN_HAS_CHILD) /** @} */ unsigned mt_flags; /**< @ref mdb_txn */ /** #dirty_list room: Array size - \#dirty pages visible to this txn. @@ -1215,7 +1221,7 @@ static char *const mdb_errstr[] = { "MDB_MAP_RESIZED: Database contents grew beyond environment mapsize", "MDB_INCOMPATIBLE: Operation and DB incompatible, or DB flags changed", "MDB_BAD_RSLOT: Invalid reuse of reader locktable slot", - "MDB_BAD_TXN: Transaction cannot recover - it must be aborted", + "MDB_BAD_TXN: Transaction must abort, has a child, or is invalid", "MDB_BAD_VALSIZE: Unsupported size of key/DB name/data, or wrong DUPFIXED size", "MDB_BAD_DBI: The specified DBI handle was closed/changed unexpectedly", }; @@ -2893,9 +2899,7 @@ mdb_txn_begin(MDB_env *env, MDB_txn *parent, unsigned flags, MDB_txn **ret) if (parent) { /* Nested transactions: Max 1 child, write txns only, no writemap */ flags |= parent->mt_flags; - if (parent->mt_child || - (flags & (MDB_RDONLY|MDB_WRITEMAP|MDB_TXN_ERROR))) - { + if (flags & (MDB_RDONLY|MDB_WRITEMAP|MDB_TXN_BLOCKED)) { return (parent->mt_flags & MDB_TXN_RDONLY) ? EINVAL : MDB_BAD_TXN; } /* Child txns save MDB_pgstate and use own copy of cursors */ @@ -2937,6 +2941,7 @@ mdb_txn_begin(MDB_env *env, MDB_txn *parent, unsigned flags, MDB_txn **ret) txn->mt_u.dirty_list[0].mid = 0; txn->mt_spill_pgs = NULL; txn->mt_next_pgno = parent->mt_next_pgno; + parent->mt_flags |= MDB_TXN_HAS_CHILD; parent->mt_child = txn; txn->mt_parent = parent; txn->mt_numdbs = parent->mt_numdbs; @@ -3075,9 +3080,10 @@ mdb_txn_end(MDB_txn *txn, unsigned mode) } /* else txn owns the slot until it does MDB_END_SLOT */ } mdb_coherent_barrier(); - txn->mt_numdbs = 0; /* close nothing if called again */ + txn->mt_numdbs = 0; /* prevent further DBI activity */ + txn->mt_flags |= MDB_TXN_FINISHED; txn->mt_dbxs = NULL; /* mark txn as reset */ - } else { + } else if (!F_ISSET(txn->mt_flags, MDB_TXN_FINISHED)) { pgno_t *pghead = env->me_pghead; if (!(mode & MDB_END_UPDATE)) /* !(already closed cursors) */ @@ -3093,6 +3099,8 @@ mdb_txn_end(MDB_txn *txn, unsigned mode) txn->mt_lifo_reclaimed = NULL; } } + txn->mt_numdbs = 0; + txn->mt_flags = MDB_TXN_FINISHED; if (!txn->mt_parent) { mdb_midl_shrink(&txn->mt_free_pgs); @@ -3108,6 +3116,7 @@ mdb_txn_end(MDB_txn *txn, unsigned mode) mdb_mutex_unlock(env, MDB_MUTEX(env, w)); } else { txn->mt_parent->mt_child = NULL; + txn->mt_parent->mt_flags &= ~MDB_TXN_HAS_CHILD; env->me_pgstate = ((MDB_ntxn *)txn)->mnt_pgstate; mdb_midl_free(txn->mt_free_pgs); mdb_midl_free(txn->mt_spill_pgs); @@ -3605,7 +3614,7 @@ mdb_txn_commit(MDB_txn *txn) goto done; } - if (F_ISSET(txn->mt_flags, MDB_TXN_ERROR)) { + if (unlikely(txn->mt_flags & (MDB_TXN_FINISHED|MDB_TXN_ERROR))) { mdb_debug("error flag is set, can't commit"); if (txn->mt_parent) txn->mt_parent->mt_flags |= MDB_TXN_ERROR; @@ -4683,6 +4692,7 @@ mdb_env_open(MDB_env *env, const char *path, unsigned flags, mode_t mode) txn->mt_dbflags = (unsigned char *)(txn->mt_dbiseqs + env->me_maxdbs); txn->mt_env = env; txn->mt_dbxs = env->me_dbxs; + txn->mt_flags = MDB_TXN_FINISHED; env->me_txn0 = txn; } else { rc = ENOMEM; @@ -5220,7 +5230,7 @@ mdb_page_search(MDB_cursor *mc, MDB_val *key, int flags) /* Make sure the txn is still viable, then find the root from * the txn's db table and set it as the root of the cursor's stack. */ - if (F_ISSET(mc->mc_txn->mt_flags, MDB_TXN_ERROR)) { + if (unlikely(mc->mc_txn->mt_flags & MDB_TXN_BLOCKED)) { mdb_debug("transaction has failed, must abort"); return MDB_BAD_TXN; } else { @@ -5408,7 +5418,7 @@ mdb_get(MDB_txn *txn, MDB_dbi dbi, if (!key || !data || !TXN_DBI_EXIST(txn, dbi, DB_USRVALID)) return EINVAL; - if (txn->mt_flags & MDB_TXN_ERROR) + if (txn->mt_flags & MDB_TXN_BLOCKED) return MDB_BAD_TXN; mdb_cursor_init(&mc, txn, dbi, &mx); @@ -5930,7 +5940,7 @@ mdb_cursor_get(MDB_cursor *mc, MDB_val *key, MDB_val *data, if (mc == NULL) return EINVAL; - if (mc->mc_txn->mt_flags & MDB_TXN_ERROR) + if (mc->mc_txn->mt_flags & MDB_TXN_BLOCKED) return MDB_BAD_TXN; switch (op) { @@ -6160,7 +6170,7 @@ mdb_cursor_put(MDB_cursor *mc, MDB_val *key, MDB_val *data, nospill = flags & MDB_NOSPILL; flags &= ~MDB_NOSPILL; - if (mc->mc_txn->mt_flags & (MDB_TXN_RDONLY|MDB_TXN_ERROR)) + if (mc->mc_txn->mt_flags & (MDB_TXN_RDONLY|MDB_TXN_BLOCKED)) return (mc->mc_txn->mt_flags & MDB_TXN_RDONLY) ? EACCES : MDB_BAD_TXN; if (key->mv_size-1 >= ENV_MAXKEY(env)) @@ -6651,7 +6661,7 @@ mdb_cursor_del(MDB_cursor *mc, unsigned flags) MDB_page *mp; int rc; - if (mc->mc_txn->mt_flags & (MDB_TXN_RDONLY|MDB_TXN_ERROR)) + if (mc->mc_txn->mt_flags & (MDB_TXN_RDONLY|MDB_TXN_BLOCKED)) return (mc->mc_txn->mt_flags & MDB_TXN_RDONLY) ? EACCES : MDB_BAD_TXN; if (!(mc->mc_flags & C_INITIALIZED)) @@ -7181,7 +7191,7 @@ mdb_cursor_open(MDB_txn *txn, MDB_dbi dbi, MDB_cursor **ret) if (!ret || !TXN_DBI_EXIST(txn, dbi, DB_VALID)) return EINVAL; - if (txn->mt_flags & MDB_TXN_ERROR) + if (txn->mt_flags & MDB_TXN_BLOCKED) return MDB_BAD_TXN; /* Allow read access to the freelist */ @@ -7216,7 +7226,7 @@ mdb_cursor_renew(MDB_txn *txn, MDB_cursor *mc) if ((mc->mc_flags & C_UNTRACK) || txn->mt_cursors) return EINVAL; - if (txn->mt_flags & MDB_TXN_ERROR) + if (txn->mt_flags & MDB_TXN_BLOCKED) return MDB_BAD_TXN; mdb_cursor_init(mc, txn, mc->mc_dbi, mc->mc_xcursor); @@ -7235,7 +7245,7 @@ mdb_cursor_count(MDB_cursor *mc, size_t *countp) if (mc->mc_xcursor == NULL) return MDB_INCOMPATIBLE; - if (mc->mc_txn->mt_flags & MDB_TXN_ERROR) + if (mc->mc_txn->mt_flags & MDB_TXN_BLOCKED) return MDB_BAD_TXN; if (!(mc->mc_flags & C_INITIALIZED)) @@ -7960,7 +7970,7 @@ mdb_del(MDB_txn *txn, MDB_dbi dbi, if (!key || !TXN_DBI_EXIST(txn, dbi, DB_USRVALID)) return EINVAL; - if (txn->mt_flags & (MDB_TXN_RDONLY|MDB_TXN_ERROR)) + if (txn->mt_flags & (MDB_TXN_RDONLY|MDB_TXN_BLOCKED)) return (txn->mt_flags & MDB_TXN_RDONLY) ? EACCES : MDB_BAD_TXN; if (!F_ISSET(txn->mt_dbs[dbi].md_flags, MDB_DUPSORT)) { @@ -9133,7 +9143,7 @@ int mdb_dbi_open(MDB_txn *txn, const char *name, unsigned flags, MDB_dbi *dbi) if (flags & ~VALID_FLAGS) return EINVAL; - if (txn->mt_flags & MDB_TXN_ERROR) + if (txn->mt_flags & MDB_TXN_BLOCKED) return MDB_BAD_TXN; /* main DB? */ @@ -9231,7 +9241,7 @@ mdb_stat(MDB_txn *txn, MDB_dbi dbi, MDB_stat *arg) if (!arg || !TXN_DBI_EXIST(txn, dbi, DB_VALID)) return EINVAL; - if (txn->mt_flags & MDB_TXN_ERROR) + if (txn->mt_flags & MDB_TXN_BLOCKED) return MDB_BAD_TXN; if (txn->mt_dbflags[dbi] & DB_STALE) {