diff --git a/changelogs/unreleased/gh-10146-mvcc-ddl-crashes-and-isolation-violation.md b/changelogs/unreleased/gh-10146-mvcc-ddl-crashes-and-isolation-violation.md
new file mode 100644
index 0000000000000000000000000000000000000000..dc0aa0b2520d8616df5aa7e64b21bedebe252d9f
--- /dev/null
+++ b/changelogs/unreleased/gh-10146-mvcc-ddl-crashes-and-isolation-violation.md
@@ -0,0 +1,4 @@
+## bugfix/memtx
+
+* Fixed several bugs when DDL with MVCC enabled could lead to a crash
+ or violate isolation of other transactions (gh-10146).
diff --git a/src/box/memtx_tx.c b/src/box/memtx_tx.c
index 97b0ee2088ca822fe2fe170be8322019db28241d..8c29c9432c049458afbebbf646221a84d64dd515 100644
--- a/src/box/memtx_tx.c
+++ b/src/box/memtx_tx.c
@@ -1202,53 +1202,6 @@ memtx_tx_story_link_top(struct memtx_story *new_top,
rlist_splice(&new_link->read_gaps, &old_link->read_gaps);
}
-static void
-memtx_tx_story_unlink_top_on_space_delete(struct memtx_story *story,
- uint32_t idx)
-{
- struct memtx_story_link *link = &story->link[idx];
- struct memtx_story *old_story = link->older_story;
- if (old_story != NULL)
- memtx_tx_story_unlink(story, old_story, idx);
-}
-
-/**
- * Unlink a @a story from history chain in @a index in both directions.
- * Handles case when the story is not on top of the history: simply remove from
- * list.
- */
-static void
-memtx_tx_story_unlink_both_common(struct memtx_story *story,
- uint32_t idx)
-{
- assert(idx < story->index_count);
- struct memtx_story_link *link = &story->link[idx];
- struct memtx_story *newer_story = link->newer_story;
- struct memtx_story *older_story = link->older_story;
- memtx_tx_story_unlink(newer_story, story, idx);
- memtx_tx_story_unlink(story, older_story, idx);
- memtx_tx_story_link(newer_story, older_story, idx);
-}
-
-/**
- * Unlink a @a story from history chain in @a index in both directions.
- * If the story was in the top of history chain - unlink from top. Otherwise,
- * see description of `memtx_tx_story_unlink_both_common`.
- * Since the space is being deleted, we need to simply unlink the story.
- */
-static void
-memtx_tx_story_unlink_both_on_space_delete(struct memtx_story *story,
- uint32_t idx)
-{
- assert(idx < story->index_count);
- struct memtx_story_link *link = &story->link[idx];
- if (link->newer_story == NULL) {
- memtx_tx_story_unlink_top_on_space_delete(story, idx);
- } else {
- memtx_tx_story_unlink_both_common(story, idx);
- }
-}
-
/**
* Change the order of stories in history chain.
*/
@@ -2076,28 +2029,6 @@ memtx_tx_history_add_stmt(struct txn_stmt *stmt, struct tuple *old_tuple,
result);
}
-/*
- * Relink those who delete this story and make them delete older story.
- */
-static void
-memtx_tx_history_remove_story_del_stmts(struct memtx_story *story)
-{
- struct memtx_story *old_story = story->link[0].older_story;
- while (story->del_stmt) {
- struct txn_stmt *del_stmt = story->del_stmt;
-
- /* Unlink from old list in any case. */
- story->del_stmt = del_stmt->next_in_del_list;
- del_stmt->next_in_del_list = NULL;
- del_stmt->del_story = NULL;
-
- /* Link to old story's list. */
- if (old_story != NULL)
- memtx_tx_story_link_deleted_by(old_story,
- del_stmt);
- }
-}
-
/*
* Abort with conflict all transactions that have read @a story.
*/
@@ -2332,44 +2263,6 @@ memtx_tx_history_rollback_stmt(struct txn_stmt *stmt)
assert(stmt->add_story == NULL && stmt->del_story == NULL);
}
-/*
- * Completely remove statement that adds a story.
- */
-static void
-memtx_tx_history_remove_added_story(struct txn_stmt *stmt)
-{
- assert(stmt->add_story != NULL);
- assert(stmt->add_story->tuple == stmt->rollback_info.new_tuple);
- struct memtx_story *story = stmt->add_story;
- memtx_tx_history_remove_story_del_stmts(story);
- for (uint32_t i = 0; i < story->index_count; i++)
- memtx_tx_story_unlink_both_on_space_delete(story, i);
- memtx_tx_story_unlink_added_by(story, stmt);
-}
-
-/*
- * Completely remove statement that deletes a story.
- */
-static void
-memtx_tx_history_remove_deleted_story(struct txn_stmt *stmt)
-{
- memtx_tx_story_unlink_deleted_by(stmt->del_story, stmt);
-}
-
-/*
- * Completely (as opposed to rollback) remove statement from history.
- */
-static void
-memtx_tx_history_remove_stmt(struct txn_stmt *stmt)
-{
- if (stmt->add_story != NULL)
- memtx_tx_history_remove_added_story(stmt);
- if (stmt->del_story != NULL)
- memtx_tx_history_remove_deleted_story(stmt);
- if (stmt->txn->status == TXN_ABORTED)
- stmt->engine_savepoint = NULL;
-}
-
/**
* Abort or send to read view readers of @a story, except the transaction
* @a writer that is actually deletes the story.
@@ -2944,9 +2837,10 @@ memtx_tx_invalidate_space(struct space *space, struct txn *active_txn)
struct memtx_story,
in_space_stories);
if (story->add_stmt != NULL)
- memtx_tx_history_remove_stmt(story->add_stmt);
+ memtx_tx_story_unlink_added_by(story, story->add_stmt);
while (story->del_stmt != NULL)
- memtx_tx_history_remove_stmt(story->del_stmt);
+ memtx_tx_story_unlink_deleted_by(story,
+ story->del_stmt);
memtx_tx_story_full_unlink_on_space_delete(story);
for (uint32_t i = 0; i < story->index_count; i++) {
struct rlist *read_gaps = &story->link[i].read_gaps;
@@ -2972,6 +2866,23 @@ memtx_tx_invalidate_space(struct space *space, struct txn *active_txn)
}
memtx_tx_story_delete(story);
}
+
+ /*
+ * Phase three: remove savepoints from all affected statements so that
+ * they won't be rolled back because we already did it. Moreover, they
+ * could access the old space that is going to be deleted leading to
+ * use-after-free.
+ */
+ struct txn *txn;
+ rlist_foreach_entry(txn, &txm.all_txs, in_all_txs) {
+ if (txn->status != TXN_ABORTED || txn->psn != 0)
+ continue;
+ struct txn_stmt *stmt;
+ stailq_foreach_entry(stmt, &txn->stmts, next) {
+ if (stmt->space == space)
+ stmt->engine_savepoint = NULL;
+ }
+ }
}
/**
diff --git a/test/box-luatest/mvcc_ddl_test.lua b/test/box-luatest/mvcc_ddl_test.lua
index 0279172c55172e170abbc70c839d4f7fbacd956a..7f0381adfa5af6fa391ee738abdb424dca0ff8da 100644
--- a/test/box-luatest/mvcc_ddl_test.lua
+++ b/test/box-luatest/mvcc_ddl_test.lua
@@ -472,3 +472,25 @@ g.test_ddl_handles_prepared = function(cg)
end
end)
end
+
+-- The test covers the problem when already deleted space could be accessed
+-- by rollback of a deletion when another concurrent deletion was already
+-- prepared.
+-- Since the access is read-only and doesn't break anything, the test actually
+-- covers the problem only when ASAN is enabled.
+g.test_ddl_use_after_free_on_rollback_of_deletion = function(cg)
+ cg.server:exec(function()
+ local txn_proxy = require('test.box.lua.txn_proxy')
+ box.schema.space.create('test')
+ box.space.test:create_index('pk')
+ box.space.test:replace{1}
+
+ local tx = txn_proxy.new()
+ tx:begin()
+ tx('box.space.test:delete(1)')
+
+ box.space.test:delete(1)
+ box.space.test:create_index('sk')
+ tx:rollback()
+ end)
+end