Re: [Tarantool-patches] [PATCH v3 3/3] vinyl: clean-up write iterator if vy_task_write_run() fails

[Nikita Pettik <korablev@tarantool.org>]

On 06 May 13:37, Aleksandr Lyapunov wrote:
> It's very strange, as I see start() method already makes its own cleanup
> in case of failure and does not require 'stop' call in this case.
> If it does not do it correctly - it must be fixed (or removed completely,
> but it seems to be very serious change).

Turns out start() doesn't provide proper clean-up: in case
vy_write_iterator_add_src() fails, only last source is cleaned up,
meanwhile the rest will be destroyed in vy_write_iterator_stop().
I've moved clean-up part from stop() method right to start(). Here's diff:

diff --git a/src/box/vy_scheduler.c b/src/box/vy_scheduler.c
index 387f58723..9dba93d34 100644
--- a/src/box/vy_scheduler.c
+++ b/src/box/vy_scheduler.c
@@ -1065,10 +1065,8 @@ vy_task_write_run(struct vy_task *task, bool no_compression)
                                 no_compression) != 0)
                goto fail;
 
-       if (wi->iface->start(wi) != 0) {
-               wi->iface->stop(wi);
+       if (wi->iface->start(wi) != 0)
                goto fail_abort_writer;
-       }
        int rc;
        int loops = 0;
        struct tuple *stmt = NULL;
diff --git a/src/box/vy_write_iterator.c b/src/box/vy_write_iterator.c
index 21c18d3dc..33ad5ed51 100644
--- a/src/box/vy_write_iterator.c
+++ b/src/box/vy_write_iterator.c
@@ -401,18 +401,23 @@ vy_write_iterator_start(struct vy_stmt_stream *vstream)
        struct vy_write_src *src, *tmp;
        rlist_foreach_entry_safe(src, &stream->src_list, in_src_list, tmp) {
                if (vy_write_iterator_add_src(stream, src) != 0)
-                       return -1;
+                       goto fail;
 #ifndef NDEBUG
                struct errinj *inj =
                        errinj(ERRINJ_VY_WRITE_ITERATOR_START_FAIL, ERRINJ_BOOL);
                if (inj != NULL && inj->bparam) {
                        inj->bparam = false;
                        diag_set(OutOfMemory, 666, "malloc", "struct vy_stmt");
-                       return -1;
+                       goto fail;
                }
 #endif
        }
        return 0;
+fail:
+       /* Clean-up all previously added sources. */
+       rlist_foreach_entry_safe(src, &stream->src_list, in_src_list, tmp)
+               vy_write_iterator_delete_src(stream, src);
+       return -1;
 }