[PATCH] txn: don't unref stmt tuples before rollback triggers are run
Vladimir Davydov
vdavydov.dev at gmail.com
Thu Jul 18 18:45:56 MSK 2019
Rollback triggers expect to see the database state after reverting all
changes done by the transaction. At the same time they may use tuples
referenced by reverted statements.
As a result, using txn_rollback_to_svp(), which both reverts changes and
releases tuples, on yield in a DDL transaction may result in a crash.
The crash can only be reproduced using SQL, because Lua implicitly
references all tuples anyway.
Thanks to @Gerold103 for reporting this issue and carrying out
preliminary investigation.
Closes #4365
---
https://github.com/tarantool/tarantool/issues/4365
https://github.com/tarantool/tarantool/commits/dv/gh-4365-fix-tx-ddl-crash-on-yield
src/box/txn.c | 22 +++++++++++++++++++++-
test/box/transaction.result | 9 +++++++++
test/box/transaction.test.lua | 6 ++++++
3 files changed, 36 insertions(+), 1 deletion(-)
diff --git a/src/box/txn.c b/src/box/txn.c
index 73eaee73..6d844af2 100644
--- a/src/box/txn.c
+++ b/src/box/txn.c
@@ -761,10 +761,30 @@ txn_on_yield(struct trigger *trigger, void *event)
(void) event;
struct txn *txn = in_txn();
assert(txn != NULL && !txn->can_yield);
- txn_rollback_to_svp(txn, NULL);
+ /*
+ * Rollback triggers expect to see the database state
+ * after reverting all changes done by the transaction.
+ * At the same time they may use tuples referenced by
+ * reverted statements. So we rollback statements before
+ * running the triggers, but postpone tuple release until
+ * after the triggers have been invoked.
+ */
+ struct txn_stmt *stmt;
+ stailq_foreach_entry(stmt, &txn->stmts, next) {
+ if (stmt->space != NULL)
+ engine_rollback_statement(txn->engine, txn, stmt);
+ stmt->space = NULL;
+ stmt->row = NULL;
+ }
if (txn->has_triggers) {
txn_run_triggers(txn, &txn->on_rollback);
txn->has_triggers = false;
}
+ stailq_foreach_entry(stmt, &txn->stmts, next)
+ txn_stmt_unref_tuples(stmt);
+ stailq_create(&txn->stmts);
+ txn->n_new_rows = 0;
+ txn->n_local_rows = 0;
+ txn->n_applier_rows = 0;
txn->is_aborted_by_yield = true;
}
diff --git a/test/box/transaction.result b/test/box/transaction.result
index 9a48f2f7..771ad96e 100644
--- a/test/box/transaction.result
+++ b/test/box/transaction.result
@@ -778,3 +778,12 @@ box.begin() drop() box.rollback()
box.begin() drop() box.commit()
---
...
+--
+-- gh-4365: DDL revered by yield triggers crash.
+--
+box.begin() box.execute([[CREATE TABLE test(id INTEGER PRIMARY KEY AUTOINCREMENT)]]) fiber.yield()
+---
+...
+box.rollback()
+---
+...
diff --git a/test/box/transaction.test.lua b/test/box/transaction.test.lua
index 0792cc3c..d8a6e60a 100644
--- a/test/box/transaction.test.lua
+++ b/test/box/transaction.test.lua
@@ -415,3 +415,9 @@ box.begin() create() box.rollback()
box.begin() create() box.commit()
box.begin() drop() box.rollback()
box.begin() drop() box.commit()
+
+--
+-- gh-4365: DDL revered by yield triggers crash.
+--
+box.begin() box.execute([[CREATE TABLE test(id INTEGER PRIMARY KEY AUTOINCREMENT)]]) fiber.yield()
+box.rollback()
--
2.11.0
More information about the Tarantool-patches
mailing list