From: Vladimir Davydov <vdavydov.dev@gmail.com> To: kostja@tarantool.org Cc: tarantool-patches@freelists.org Subject: [PATCH] txn: don't unref stmt tuples before rollback triggers are run Date: Thu, 18 Jul 2019 18:45:56 +0300 [thread overview] Message-ID: <b1a2686ee02ba2c99fed958fb128edfe7f55d40b.1563463932.git.vdavydov.dev@gmail.com> (raw) 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
next reply other threads:[~2019-07-18 15:45 UTC|newest] Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-07-18 15:45 Vladimir Davydov [this message] 2019-07-19 17:40 ` Vladimir Davydov 2019-07-19 19:28 ` [tarantool-patches] " Vladislav Shpilevoy 2019-07-19 19:36 ` Vladislav Shpilevoy 2019-07-24 12:30 ` [tarantool-patches] " Георгий Кириченко 2019-07-24 12:55 ` Vladimir Davydov
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=b1a2686ee02ba2c99fed958fb128edfe7f55d40b.1563463932.git.vdavydov.dev@gmail.com \ --to=vdavydov.dev@gmail.com \ --cc=kostja@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='Re: [PATCH] txn: don'\''t unref stmt tuples before rollback triggers are run' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox