Re: [PATCH 05/12] alter: allocate triggers before the point of no return

[Konstantin Osipov <kostja@tarantool.org>]

* Vladimir Davydov <vdavydov.dev@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.

> ---
>  src/box/alter.cc | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/src/box/alter.cc b/src/box/alter.cc
> index 36310f1c..9d0c4c23 100644
> --- a/src/box/alter.cc
> +++ b/src/box/alter.cc
> @@ -803,6 +803,11 @@ 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. */
> +	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 +858,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 +883,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);
>  }
>  
> -- 
> 2.11.0
> 

-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov