Skip to content
Snippets Groups Projects
Commit 73b97984 authored by Cyrill Gorcunov's avatar Cyrill Gorcunov Committed by Kirill Yukhin
Browse files

applier: prevent nil dereference on applier rollback


Currently when transaction rollback happens we just drop an existing
error setting ClientError to the replicaset.applier.diag. This action
leaves current fiber with diag=nil, which in turn leads to sigsegv once
diag_raise() called right after applier_apply_tx():

 | applier_f
 |   try {
 |   applier_subscribe
 |     applier_apply_tx
 |       // error happens
 |       txn_rollback
 |         diag_set(ClientError, ER_WAL_IO)
 |         diag_move(&fiber()->diag, &replicaset.applier.diag)
 |         // fiber->diag = nil
 |       applier_on_rollback
 |         diag_add_error(&applier->diag, diag_last_error(&replicaset.applier.diag)
 |         fiber_cancel(applier->reader);
 |     diag_raise() -> NULL dereference
 |   } catch { ... }

Thus:
 - use diag_set_error() instead of diag_move() to not drop error
   from a current fiber() preventing a nil dereference;
 - put fixme mark into the code: we need to rework it in a
   more sense way.

Fixes #4730

Acked-by: default avatarSerge Petrenko <sergepetrenko@tarantool.org>
Signed-off-by: default avatarCyrill Gorcunov <gorcunov@gmail.com>
parent 069901c8
No related branches found
No related tags found
No related merge requests found
......@@ -692,9 +692,22 @@ static int
applier_txn_rollback_cb(struct trigger *trigger, void *event)
{
(void) trigger;
/* Setup shared applier diagnostic area. */
/*
* Setup shared applier diagnostic area.
*
* FIXME: We should consider redesign this
* moment and instead of carrying one shared
* diag use per-applier diag instead all the time
* (which actually already present in the structure).
*
* But remember that transactions are asynchronous
* and rollback may happen a way latter after it
* passed to the journal engine.
*/
diag_set(ClientError, ER_WAL_IO);
diag_move(&fiber()->diag, &replicaset.applier.diag);
diag_set_error(&replicaset.applier.diag,
diag_last_error(diag_get()));
/* Broadcast the rollback event across all appliers. */
trigger_run(&replicaset.applier.on_rollback, event);
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment