[PATCH 05/12] alter: allocate triggers before the point of no return
Vladimir Davydov
vdavydov.dev at gmail.com
Tue Apr 10 14:54:35 MSK 2018
On Tue, Apr 10, 2018 at 10:57:42AM +0300, Vladimir Davydov wrote:
> On Mon, Apr 09, 2018 at 11:36:02PM +0300, Konstantin Osipov wrote:
> > * Vladimir Davydov <vdavydov.dev at gmail.com> [18/04/09 10:33]:
> > > Trigger allocation, as any other memory allocation, is allowed to fail.
> > > If this happens in alter_space_do, the space will be left in an
> > > inconsistent state. Let's move trigger allocation to the beginning of
> > > alter_space_do and add a comment denoting the point of no return.
> >
> > Previously we would reference the allocated trigger immediately
> > in txn_on_commit() /txn_on_rollback(), the changed code leaks
> > memory in case of any exception between allocation and "point of no
> > return".
> >
> > Please add guards.
>
> We don't need guards for the triggers, because they are allocated on
> region - see txn_alter_trigger_new().
Added a comment, as discussed:
(not on the branch, don't push)
>From d0467518750a6d7aaaf0eb80842ecd6ccc6b2219 Mon Sep 17 00:00:00 2001
From: Vladimir Davydov <vdavydov.dev at gmail.com>
Date: Wed, 4 Apr 2018 18:48:13 +0300
Subject: [PATCH] alter: allocate triggers before the point of no return
Trigger allocation, as any other memory allocation, is allowed to fail.
If this happens in alter_space_do, the space will be left in an
inconsistent state. Let's move trigger allocation to the beginning of
alter_space_do and add a comment denoting the point of no return.
diff --git a/src/box/alter.cc b/src/box/alter.cc
index 96143b95..c4141145 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -803,6 +803,15 @@ alter_space_rollback(struct trigger *trigger, void * /* event */)
static void
alter_space_do(struct txn *txn, struct alter_space *alter)
{
+ /*
+ * Prepare triggers while we may fail. Note, we don't have to
+ * free them in case of failure, because they are allocated on
+ * the region.
+ */
+ struct trigger *on_commit, *on_rollback;
+ on_commit = txn_alter_trigger_new(alter_space_commit, alter);
+ on_rollback = txn_alter_trigger_new(alter_space_rollback, alter);
+
/* Create a definition of the new space. */
space_dump_def(alter->old_space, &alter->key_list);
class AlterSpaceOp *op;
@@ -853,6 +862,11 @@ alter_space_do(struct txn *txn, struct alter_space *alter)
throw;
}
+ /*
+ * This function must not throw exceptions or yield after
+ * this point.
+ */
+
/* Rebuild index maps once for all indexes. */
space_fill_index_map(alter->old_space);
space_fill_index_map(alter->new_space);
@@ -873,11 +887,7 @@ alter_space_do(struct txn *txn, struct alter_space *alter)
* finish or rollback the DDL depending on the results of
* writing to WAL.
*/
- struct trigger *on_commit =
- txn_alter_trigger_new(alter_space_commit, alter);
txn_on_commit(txn, on_commit);
- struct trigger *on_rollback =
- txn_alter_trigger_new(alter_space_rollback, alter);
txn_on_rollback(txn, on_rollback);
}
More information about the Tarantool-patches
mailing list