diff --git a/changelogs/unreleased/gh-6057-qsync-confirm-async-no-wal.md b/changelogs/unreleased/gh-6057-qsync-confirm-async-no-wal.md
new file mode 100644
index 0000000000000000000000000000000000000000..1005389d87e7667334e1f1b9e41c2ff593ce84a6
--- /dev/null
+++ b/changelogs/unreleased/gh-6057-qsync-confirm-async-no-wal.md
@@ -0,0 +1,5 @@
+## bugfix/replication
+
+* Fixed a possible crash when a synchronous transaction was followed by an
+ asynchronous transaction right when its confirmation was being written
+ (gh-6057).
diff --git a/src/box/txn.c b/src/box/txn.c
index 1d42c9113031da36bf914f4e51c6e363a661fc2a..966dfafdfd6725ec50f11c9c11a44e49d5e470b9 100644
--- a/src/box/txn.c
+++ b/src/box/txn.c
@@ -879,9 +879,14 @@ txn_commit(struct txn *txn)
req = txn_journal_entry_new(txn);
if (req == NULL)
goto rollback;
-
- bool is_sync = txn_has_flag(txn, TXN_WAIT_SYNC);
- if (is_sync) {
+ /*
+ * Do not cache the flag value in a variable. The flag might be deleted
+ * during WAL write. This can happen for async transactions created
+ * during CONFIRM write, whose all blocking sync transactions get
+ * confirmed. Then they turn the async transaction into just a plain
+ * txn not waiting for anything.
+ */
+ if (txn_has_flag(txn, TXN_WAIT_SYNC)) {
/*
* Remote rows, if any, come before local rows, so
* check for originating instance id here.
@@ -900,13 +905,13 @@ txn_commit(struct txn *txn)
fiber_set_txn(fiber(), NULL);
if (journal_write(req) != 0 || req->res < 0) {
- if (is_sync)
+ if (txn_has_flag(txn, TXN_WAIT_SYNC))
txn_limbo_abort(&txn_limbo, limbo_entry);
diag_set(ClientError, ER_WAL_IO);
diag_log();
goto rollback;
}
- if (is_sync) {
+ if (txn_has_flag(txn, TXN_WAIT_SYNC)) {
if (txn_has_flag(txn, TXN_WAIT_ACK)) {
int64_t lsn = req->rows[req->n_rows - 1]->lsn;
/*
diff --git a/src/box/txn_limbo.c b/src/box/txn_limbo.c
index f287369a28753ff1820d79f42610e69d22b1ae0d..dae6d2df4aa68504a0a61df9da57486f2197228a 100644
--- a/src/box/txn_limbo.c
+++ b/src/box/txn_limbo.c
@@ -389,6 +389,33 @@ txn_limbo_read_confirm(struct txn_limbo *limbo, int64_t lsn)
*/
if (e->lsn == -1)
break;
+ } else if (e->txn->signature < 0) {
+ /*
+ * A transaction might be covered by the CONFIRM even if
+ * it is not written to WAL yet when it is an async
+ * transaction. It could be created just when the
+ * CONFIRM was being written to WAL.
+ */
+ assert(e->txn->status == TXN_PREPARED);
+ /*
+ * Let it complete normally as a plain transaction. It
+ * is important to remove the limbo entry, because the
+ * async transaction might be committed in a
+ * non-blocking way and won't ever wait explicitly for
+ * its completion. Therefore, won't be able to remove
+ * the limbo entry on its own. This happens for txns
+ * created in the applier.
+ */
+ txn_clear_flags(e->txn, TXN_WAIT_SYNC);
+ txn_limbo_remove(limbo, e);
+ /*
+ * The limbo entry now should not be used by the owner
+ * transaction since it just became a plain one. Nullify
+ * the txn to get a crash on any usage attempt instead
+ * of potential undefined behaviour.
+ */
+ e->txn = NULL;
+ continue;
}
e->is_commit = true;
txn_limbo_remove(limbo, e);
diff --git a/test/replication/gh-6057-qsync-confirm-async-no-wal.result b/test/replication/gh-6057-qsync-confirm-async-no-wal.result
new file mode 100644
index 0000000000000000000000000000000000000000..23c77729b70b498b67b13603212c32b2bc64660d
--- /dev/null
+++ b/test/replication/gh-6057-qsync-confirm-async-no-wal.result
@@ -0,0 +1,163 @@
+-- test-run result file version 2
+test_run = require('test_run').new()
+ | ---
+ | ...
+fiber = require('fiber')
+ | ---
+ | ...
+
+--
+-- gh-6057: while CONFIRM is being written, there might appear async
+-- transactions not having an LSN/signature yet. When CONFIRM is done, it covers
+-- these transactions too since they don't need to wait for an ACK, but it
+-- should not try to complete them. Because their WAL write is not done and
+-- it might even fail later. It should simply turn them into plain transactions
+-- not depending on any synchronous ones.
+--
+
+old_synchro_quorum = box.cfg.replication_synchro_quorum
+ | ---
+ | ...
+old_synchro_timeout = box.cfg.replication_synchro_timeout
+ | ---
+ | ...
+box.cfg{ \
+ replication_synchro_quorum = 1, \
+ replication_synchro_timeout = 1000000, \
+}
+ | ---
+ | ...
+s = box.schema.create_space('test', {is_sync = true})
+ | ---
+ | ...
+_ = s:create_index('pk')
+ | ---
+ | ...
+s2 = box.schema.create_space('test2')
+ | ---
+ | ...
+_ = s2:create_index('pk')
+ | ---
+ | ...
+
+errinj = box.error.injection
+ | ---
+ | ...
+
+function create_hanging_async_after_confirm(sync_key, async_key1, async_key2) \
+-- Let the transaction itself to WAL, but CONFIRM will be blocked. \
+ errinj.set('ERRINJ_WAL_DELAY_COUNTDOWN', 1) \
+ local lsn = box.info.lsn \
+ local f = fiber.create(function() s:replace{sync_key} end) \
+ test_run:wait_cond(function() return box.info.lsn == lsn + 1 end) \
+-- Wait for the CONFIRM block. WAL thread is in busy loop now. \
+ test_run:wait_cond(function() return errinj.get('ERRINJ_WAL_DELAY') end) \
+ \
+-- Create 2 async transactions to ensure multiple of them are handled fine. \
+-- But return only fiber of the second one. It is enough because if it is \
+-- finished, the first one is too. \
+ fiber.new(function() s2:replace{async_key1} end) \
+ local f2 = fiber.new(function() s2:replace{async_key2} end) \
+ fiber.yield() \
+-- When WAL thread would finish CONFIRM, it should be blocked on the async \
+-- transaction so as it wouldn't be completed when CONFIRM is processed. \
+ errinj.set('ERRINJ_WAL_DELAY_COUNTDOWN', 0) \
+-- Let CONFIRM go. \
+ errinj.set('ERRINJ_WAL_DELAY', false) \
+-- And WAL thread should be blocked on the async txn. \
+ test_run:wait_cond(function() return errinj.get('ERRINJ_WAL_DELAY') end) \
+-- Wait CONFIRM finish. \
+ test_run:wait_cond(function() return f:status() == 'dead' end) \
+ return f2 \
+end
+ | ---
+ | ...
+
+async_f = create_hanging_async_after_confirm(1, 2, 3)
+ | ---
+ | ...
+-- Let the async transaction finish its WAL write.
+errinj.set('ERRINJ_WAL_DELAY', false)
+ | ---
+ | - ok
+ | ...
+-- It should see that even though it is in the limbo, it does not have any
+-- synchronous transactions to wait for and can complete right away.
+test_run:wait_cond(function() return async_f:status() == 'dead' end)
+ | ---
+ | - true
+ | ...
+
+assert(s:get({1}) ~= nil)
+ | ---
+ | - true
+ | ...
+assert(s2:get({2}) ~= nil)
+ | ---
+ | - true
+ | ...
+assert(s2:get({3}) ~= nil)
+ | ---
+ | - true
+ | ...
+
+--
+-- Try all the same, but the async transaction fails its WAL write.
+--
+async_f = create_hanging_async_after_confirm(4, 5, 6)
+ | ---
+ | ...
+-- The async transaction will fail to go to WAL when WAL thread is unblocked.
+errinj.set('ERRINJ_WAL_ROTATE', true)
+ | ---
+ | - ok
+ | ...
+errinj.set('ERRINJ_WAL_DELAY', false)
+ | ---
+ | - ok
+ | ...
+test_run:wait_cond(function() return async_f:status() == 'dead' end)
+ | ---
+ | - true
+ | ...
+errinj.set('ERRINJ_WAL_ROTATE', false)
+ | ---
+ | - ok
+ | ...
+
+assert(s:get({4}) ~= nil)
+ | ---
+ | - true
+ | ...
+assert(s2:get({5}) == nil)
+ | ---
+ | - true
+ | ...
+assert(s2:get({6}) == nil)
+ | ---
+ | - true
+ | ...
+
+-- Ensure nothing is broken, works fine.
+s:replace{7}
+ | ---
+ | - [7]
+ | ...
+s2:replace{8}
+ | ---
+ | - [8]
+ | ...
+
+s:drop()
+ | ---
+ | ...
+s2:drop()
+ | ---
+ | ...
+
+box.cfg{ \
+ replication_synchro_quorum = old_synchro_quorum, \
+ replication_synchro_timeout = old_synchro_timeout, \
+}
+ | ---
+ | ...
diff --git a/test/replication/gh-6057-qsync-confirm-async-no-wal.test.lua b/test/replication/gh-6057-qsync-confirm-async-no-wal.test.lua
new file mode 100644
index 0000000000000000000000000000000000000000..a11ddc042229c3f087708c365a083c430dcd4cea
--- /dev/null
+++ b/test/replication/gh-6057-qsync-confirm-async-no-wal.test.lua
@@ -0,0 +1,88 @@
+test_run = require('test_run').new()
+fiber = require('fiber')
+
+--
+-- gh-6057: while CONFIRM is being written, there might appear async
+-- transactions not having an LSN/signature yet. When CONFIRM is done, it covers
+-- these transactions too since they don't need to wait for an ACK, but it
+-- should not try to complete them. Because their WAL write is not done and
+-- it might even fail later. It should simply turn them into plain transactions
+-- not depending on any synchronous ones.
+--
+
+old_synchro_quorum = box.cfg.replication_synchro_quorum
+old_synchro_timeout = box.cfg.replication_synchro_timeout
+box.cfg{ \
+ replication_synchro_quorum = 1, \
+ replication_synchro_timeout = 1000000, \
+}
+s = box.schema.create_space('test', {is_sync = true})
+_ = s:create_index('pk')
+s2 = box.schema.create_space('test2')
+_ = s2:create_index('pk')
+
+errinj = box.error.injection
+
+function create_hanging_async_after_confirm(sync_key, async_key1, async_key2) \
+-- Let the transaction itself to WAL, but CONFIRM will be blocked. \
+ errinj.set('ERRINJ_WAL_DELAY_COUNTDOWN', 1) \
+ local lsn = box.info.lsn \
+ local f = fiber.create(function() s:replace{sync_key} end) \
+ test_run:wait_cond(function() return box.info.lsn == lsn + 1 end) \
+-- Wait for the CONFIRM block. WAL thread is in busy loop now. \
+ test_run:wait_cond(function() return errinj.get('ERRINJ_WAL_DELAY') end) \
+ \
+-- Create 2 async transactions to ensure multiple of them are handled fine. \
+-- But return only fiber of the second one. It is enough because if it is \
+-- finished, the first one is too. \
+ fiber.new(function() s2:replace{async_key1} end) \
+ local f2 = fiber.new(function() s2:replace{async_key2} end) \
+ fiber.yield() \
+-- When WAL thread would finish CONFIRM, it should be blocked on the async \
+-- transaction so as it wouldn't be completed when CONFIRM is processed. \
+ errinj.set('ERRINJ_WAL_DELAY_COUNTDOWN', 0) \
+-- Let CONFIRM go. \
+ errinj.set('ERRINJ_WAL_DELAY', false) \
+-- And WAL thread should be blocked on the async txn. \
+ test_run:wait_cond(function() return errinj.get('ERRINJ_WAL_DELAY') end) \
+-- Wait CONFIRM finish. \
+ test_run:wait_cond(function() return f:status() == 'dead' end) \
+ return f2 \
+end
+
+async_f = create_hanging_async_after_confirm(1, 2, 3)
+-- Let the async transaction finish its WAL write.
+errinj.set('ERRINJ_WAL_DELAY', false)
+-- It should see that even though it is in the limbo, it does not have any
+-- synchronous transactions to wait for and can complete right away.
+test_run:wait_cond(function() return async_f:status() == 'dead' end)
+
+assert(s:get({1}) ~= nil)
+assert(s2:get({2}) ~= nil)
+assert(s2:get({3}) ~= nil)
+
+--
+-- Try all the same, but the async transaction fails its WAL write.
+--
+async_f = create_hanging_async_after_confirm(4, 5, 6)
+-- The async transaction will fail to go to WAL when WAL thread is unblocked.
+errinj.set('ERRINJ_WAL_ROTATE', true)
+errinj.set('ERRINJ_WAL_DELAY', false)
+test_run:wait_cond(function() return async_f:status() == 'dead' end)
+errinj.set('ERRINJ_WAL_ROTATE', false)
+
+assert(s:get({4}) ~= nil)
+assert(s2:get({5}) == nil)
+assert(s2:get({6}) == nil)
+
+-- Ensure nothing is broken, works fine.
+s:replace{7}
+s2:replace{8}
+
+s:drop()
+s2:drop()
+
+box.cfg{ \
+ replication_synchro_quorum = old_synchro_quorum, \
+ replication_synchro_timeout = old_synchro_timeout, \
+}
diff --git a/test/replication/suite.cfg b/test/replication/suite.cfg
index dfe4be9aec2ea3ecf0692f3df5d4f03b4cab73fc..4b7aa9baaf9eddda804e98693b37b7763627f710 100644
--- a/test/replication/suite.cfg
+++ b/test/replication/suite.cfg
@@ -46,6 +46,7 @@
"gh-5536-wal-limit.test.lua": {},
"gh-5566-final-join-synchro.test.lua": {},
"gh-6032-promote-wal-write.test.lua": {},
+ "gh-6057-qsync-confirm-async-no-wal.test.lua": {},
"*": {
"memtx": {"engine": "memtx"},
"vinyl": {"engine": "vinyl"}
diff --git a/test/replication/suite.ini b/test/replication/suite.ini
index 2625c5eea8848c2090a1f472bbcfdbfa730a705e..80e968d56ec38772fe0a3387a8f9453729d4be4f 100644
--- a/test/replication/suite.ini
+++ b/test/replication/suite.ini
@@ -3,7 +3,7 @@ core = tarantool
script = master.lua
description = tarantool/box, replication
disabled = consistent.test.lua
-release_disabled = catch.test.lua errinj.test.lua gc.test.lua gc_no_space.test.lua before_replace.test.lua qsync_advanced.test.lua qsync_errinj.test.lua quorum.test.lua recover_missing_xlog.test.lua sync.test.lua long_row_timeout.test.lua gh-4739-vclock-assert.test.lua gh-4730-applier-rollback.test.lua gh-5140-qsync-casc-rollback.test.lua gh-5144-qsync-dup-confirm.test.lua gh-5167-qsync-rollback-snap.test.lua gh-5506-election-on-off.test.lua gh-5536-wal-limit.test.lua hang_on_synchro_fail.test.lua anon_register_gap.test.lua gh-5213-qsync-applier-order.test.lua gh-5213-qsync-applier-order-3.test.lua gh-6032-promote-wal-write.test.lua
+release_disabled = catch.test.lua errinj.test.lua gc.test.lua gc_no_space.test.lua before_replace.test.lua qsync_advanced.test.lua qsync_errinj.test.lua quorum.test.lua recover_missing_xlog.test.lua sync.test.lua long_row_timeout.test.lua gh-4739-vclock-assert.test.lua gh-4730-applier-rollback.test.lua gh-5140-qsync-casc-rollback.test.lua gh-5144-qsync-dup-confirm.test.lua gh-5167-qsync-rollback-snap.test.lua gh-5506-election-on-off.test.lua gh-5536-wal-limit.test.lua hang_on_synchro_fail.test.lua anon_register_gap.test.lua gh-5213-qsync-applier-order.test.lua gh-5213-qsync-applier-order-3.test.lua gh-6032-promote-wal-write.test.lua gh-6057-qsync-confirm-async-no-wal.test.lua
config = suite.cfg
lua_libs = lua/fast_replica.lua lua/rlimit.lua
use_unix_sockets = True