From 753b2270fdfc8df36a23c82e2e2707c7357de955 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=D0=9B=D0=B5=D0=BE=D0=BD=D0=B8=D0=B4=20=D0=AE=D1=80=D1=8C?=
=?UTF-8?q?=D0=B5=D0=B2=20=28Leonid=20Yuriev=29?= <leo@yuriev.ru>
Date: Thu, 20 Mar 2025 01:14:34 +0300
Subject: [PATCH] =?UTF-8?q?mdbx:=20=D0=B4=D0=BE=D0=B1=D0=B0=D0=B2=D0=BB?=
=?UTF-8?q?=D0=B5=D0=BD=D0=B8=D0=B5=20`mdbx=5Fcursor=5Fclose2()`=20=D0=B2?=
=?UTF-8?q?=20API=20(backport).?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
---
mdbx.h | 30 +++++++++++++++-
mdbx.h++ | 5 ++-
src/api-cursor.c | 91 ++++++++++++++++++++++++++++++------------------
src/cursor.c | 4 +--
src/mdbx.c++ | 9 -----
5 files changed, 93 insertions(+), 46 deletions(-)
diff --git a/mdbx.h b/mdbx.h
index 559c664b..e8e5748b 100644
--- a/mdbx.h
+++ b/mdbx.h
@@ -5210,12 +5210,17 @@ LIBMDBX_API int mdbx_cursor_reset(MDBX_cursor *cursor);
* \retval MDBX_EINVAL An invalid parameter was specified. */
LIBMDBX_API int mdbx_cursor_open(MDBX_txn *txn, MDBX_dbi dbi, MDBX_cursor **cursor);
-/** \brief Close a cursor handle.
+/** \brief Closes a cursor handle without returning error code.
* \ingroup c_cursors
*
* The cursor handle will be freed and must not be used again after this call,
* but its transaction may still be live.
*
+ * This function returns `void` but panic in case of error. Use \ref mdbx_cursor_close2()
+ * if you need to receive an error code instead of an app crash.
+ *
+ * \see mdbx_cursor_close2
+ *
* \note In contrast to LMDB, the MDBX required that any opened cursors can be
* reused and must be freed explicitly, regardless ones was opened in a
* read-only or write transaction. The REASON for this is eliminates ambiguity
@@ -5226,6 +5231,29 @@ LIBMDBX_API int mdbx_cursor_open(MDBX_txn *txn, MDBX_dbi dbi, MDBX_cursor **curs
* or \ref mdbx_cursor_create(). */
LIBMDBX_API void mdbx_cursor_close(MDBX_cursor *cursor);
+/** \brief Closes a cursor handle with returning error code.
+ * \ingroup c_cursors
+ *
+ * The cursor handle will be freed and must not be used again after this call,
+ * but its transaction may still be live.
+ *
+ * \see mdbx_cursor_close
+ *
+ * \note In contrast to LMDB, the MDBX required that any opened cursors can be
+ * reused and must be freed explicitly, regardless ones was opened in a
+ * read-only or write transaction. The REASON for this is eliminates ambiguity
+ * which helps to avoid errors such as: use-after-free, double-free, i.e.
+ * memory corruption and segfaults.
+ *
+ * \param [in] cursor A cursor handle returned by \ref mdbx_cursor_open()
+ * or \ref mdbx_cursor_create().
+ * \returns A non-zero error value on failure and 0 on success,
+ * some possible errors are:
+ * \retval MDBX_THREAD_MISMATCH Given transaction is not owned
+ * by current thread.
+ * \retval MDBX_EINVAL An invalid parameter was specified. */
+LIBMDBX_API int mdbx_cursor_close2(MDBX_cursor *cursor);
+
/** \brief Unbind or closes all cursors of a given transaction.
* \ingroup c_cursors
*
diff --git a/mdbx.h++ b/mdbx.h++
index e993f17f..3b6e6239 100644
--- a/mdbx.h++
+++ b/mdbx.h++
@@ -4545,7 +4545,10 @@ public:
}
/// \brief Explicitly closes the cursor.
- void close();
+ inline void close() {
+ error::success_or_throw(::mdbx_cursor_close2(handle_));
+ handle_ = nullptr;
+ }
cursor_managed(cursor_managed &&) = default;
cursor_managed &operator=(cursor_managed &&other) noexcept {
diff --git a/src/api-cursor.c b/src/api-cursor.c
index cfe935f1..d86bc34e 100644
--- a/src/api-cursor.c
+++ b/src/api-cursor.c
@@ -93,6 +93,10 @@ int mdbx_cursor_unbind(MDBX_cursor *mc) {
/* TODO: реализовать при переходе на двусвязный список курсоров */
return LOG_IFERR(MDBX_EINVAL);
+ int rc = check_txn(mc->txn, MDBX_TXN_FINISHED | MDBX_TXN_HAS_CHILD);
+ if (unlikely(rc != MDBX_SUCCESS))
+ return LOG_IFERR(rc);
+
if (unlikely(!mc->txn || mc->txn->signature != txn_signature)) {
ERROR("Wrong cursor's transaction %p 0x%x", __Wpedantic_format_voidptr(mc->txn), mc->txn ? mc->txn->signature : 0);
return LOG_IFERR(MDBX_PROBLEM);
@@ -137,42 +141,63 @@ int mdbx_cursor_open(MDBX_txn *txn, MDBX_dbi dbi, MDBX_cursor **ret) {
return MDBX_SUCCESS;
}
-void mdbx_cursor_close(MDBX_cursor *mc) {
- if (likely(mc)) {
- ENSURE(nullptr, mc->signature == cur_signature_live || mc->signature == cur_signature_ready4dispose);
- MDBX_txn *const txn = mc->txn;
- if (!mc->backup) {
- mc->txn = nullptr;
- /* Unlink from txn, if tracked. */
- if (mc->next != mc) {
- ENSURE(txn->env, check_txn(txn, 0) == MDBX_SUCCESS);
- const size_t dbi = (kvx_t *)mc->clc - txn->env->kvs;
- tASSERT(txn, dbi < txn->n_dbi);
- if (dbi < txn->n_dbi) {
- MDBX_cursor **prev = &txn->cursors[dbi];
- while (*prev) {
- ENSURE(txn->env, (*prev)->signature == cur_signature_live || (*prev)->signature == cur_signature_wait4eot);
- if (*prev == mc)
- break;
- prev = &(*prev)->next;
- }
- tASSERT(txn, *prev == mc);
- *prev = mc->next;
- }
- mc->next = mc;
- }
- mc->signature = 0;
- osal_free(mc);
- } else {
- /* Cursor closed before nested txn ends */
- tASSERT(txn, mc->signature == cur_signature_live);
- ENSURE(txn->env, check_txn_rw(txn, 0) == MDBX_SUCCESS);
- be_poor(mc);
- mc->signature = cur_signature_wait4eot;
- }
+void mdbx_cursor_close(MDBX_cursor *cursor) {
+ if (likely(cursor)) {
+ int err = mdbx_cursor_close2(cursor);
+ if (unlikely(err != MDBX_SUCCESS))
+ mdbx_panic("%s:%d error %d (%s) while closing cursor", __func__, __LINE__, err, mdbx_liberr2str(err));
}
}
+int mdbx_cursor_close2(MDBX_cursor *mc) {
+ if (unlikely(!mc))
+ return LOG_IFERR(MDBX_EINVAL);
+
+ if (mc->signature == cur_signature_ready4dispose) {
+ if (unlikely(mc->txn || mc->backup))
+ return LOG_IFERR(MDBX_PANIC);
+ cursor_drown((cursor_couple_t *)mc);
+ mc->signature = 0;
+ osal_free(mc);
+ return MDBX_SUCCESS;
+ }
+
+ if (unlikely(mc->signature != cur_signature_live))
+ return LOG_IFERR(MDBX_EBADSIGN);
+
+ MDBX_txn *const txn = mc->txn;
+ int rc = check_txn(txn, MDBX_TXN_FINISHED);
+ if (unlikely(rc != MDBX_SUCCESS))
+ return LOG_IFERR(rc);
+
+ if (mc->backup) {
+ /* Cursor closed before nested txn ends */
+ cursor_reset((cursor_couple_t *)mc);
+ mc->signature = cur_signature_wait4eot;
+ return MDBX_SUCCESS;
+ }
+
+ if (mc->next != mc) {
+ const size_t dbi = cursor_dbi(mc);
+ cASSERT(mc, dbi < mc->txn->n_dbi);
+ cASSERT(mc, &mc->txn->env->kvs[dbi].clc == mc->clc);
+ if (likely(dbi < txn->n_dbi)) {
+ MDBX_cursor **prev = &txn->cursors[dbi];
+ while (/* *prev && */ *prev != mc) {
+ ENSURE(txn->env, (*prev)->signature == cur_signature_live || (*prev)->signature == cur_signature_wait4eot);
+ prev = &(*prev)->next;
+ }
+ tASSERT(txn, *prev == mc);
+ *prev = mc->next;
+ }
+ mc->next = mc;
+ }
+ cursor_drown((cursor_couple_t *)mc);
+ mc->signature = 0;
+ osal_free(mc);
+ return MDBX_SUCCESS;
+}
+
int mdbx_cursor_copy(const MDBX_cursor *src, MDBX_cursor *dest) {
if (unlikely(!src))
return LOG_IFERR(MDBX_EINVAL);
diff --git a/src/cursor.c b/src/cursor.c
index 73da6a8b..bb6f4e61 100644
--- a/src/cursor.c
+++ b/src/cursor.c
@@ -252,9 +252,9 @@ MDBX_cursor *cursor_eot(MDBX_cursor *mc, MDBX_txn *txn, const bool merge) {
osal_free(bk);
} else {
ENSURE(mc->txn->env, stage == cur_signature_live);
- cursor_drown((cursor_couple_t *)mc);
- mc->next = mc;
mc->signature = cur_signature_ready4dispose /* Cursor may be reused */;
+ mc->next = mc;
+ cursor_drown((cursor_couple_t *)mc);
}
return next;
}
diff --git a/src/mdbx.c++ b/src/mdbx.c++
index d891b654..63f56190 100644
--- a/src/mdbx.c++
+++ b/src/mdbx.c++
@@ -1590,15 +1590,6 @@ __cold bool txn::rename_map(const ::std::string &old_name, const ::std::string &
//------------------------------------------------------------------------------
-void cursor_managed::close() {
- if (MDBX_UNLIKELY(!handle_))
- MDBX_CXX20_UNLIKELY error::throw_exception(MDBX_EINVAL);
- ::mdbx_cursor_close(handle_);
- handle_ = nullptr;
-}
-
-//------------------------------------------------------------------------------
-
__cold ::std::ostream &operator<<(::std::ostream &out, const slice &it) {
out << "{";
if (!it.is_valid())