[Xen-changelog] [xen master] tools/libxl: Do not fire the stream callback multiple times

[patchbot@xxxxxxx]

commit 44313ab77f3e3c5b566ea4f23b0e32bfd5eafa29
Author:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
AuthorDate: Mon Jul 27 17:47:26 2015 +0100
Commit:     Ian Campbell <ian.campbell@xxxxxxxxxx>
CommitDate: Tue Jul 28 14:02:18 2015 +0100

    tools/libxl: Do not fire the stream callback multiple times
    
    Avoid stacking of check_all_finished() via synchronous teardown of
    tasks.  If the _abort() functions call back synchronously,
    stream->completion_callback() ends up getting called twice, as first
    and last check_all_finished() frames observe each task being finished.
    
    Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    Acked-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
---
 tools/libxl/libxl_internal.h     |    2 ++
 tools/libxl/libxl_stream_read.c  |   21 +++++++++++++++++++++
 tools/libxl/libxl_stream_write.c |   21 +++++++++++++++++++++
 3 files changed, 44 insertions(+), 0 deletions(-)

diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index b7619a9..911de2d 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -3009,6 +3009,7 @@ struct libxl__stream_write_state {
     int rc;
     bool running;
     bool in_checkpoint;
+    bool sync_teardown;  /* Only used to coordinate shutdown on error path. */
     libxl__save_helper_state shs;
 
     /* Main stream-writing data. */
@@ -3350,6 +3351,7 @@ struct libxl__stream_read_state {
     int rc;
     bool running;
     bool in_checkpoint;
+    bool sync_teardown; /* Only used to coordinate shutdown on error path. */
     libxl__save_helper_state shs;
     libxl__conversion_helper_state chs;
 
diff --git a/tools/libxl/libxl_stream_read.c b/tools/libxl/libxl_stream_read.c
index e2dad35..d54789c 100644
--- a/tools/libxl/libxl_stream_read.c
+++ b/tools/libxl/libxl_stream_read.c
@@ -176,6 +176,7 @@ void libxl__stream_read_init(libxl__stream_read_state 
*stream)
     stream->rc = 0;
     stream->running = false;
     stream->in_checkpoint = false;
+    stream->sync_teardown = false;
     libxl__save_helper_init(&stream->shs);
     libxl__conversion_helper_init(&stream->chs);
     FILLZERO(stream->dc);
@@ -760,13 +761,33 @@ static void check_all_finished(libxl__egc *egc,
 {
     STATE_AO_GC(stream->ao);
 
+    /*
+     * In the case of a failure, the _abort()'s below might cancel
+     * synchronously on top of us, or asynchronously at a later point.
+     *
+     * We must avoid the situation where all _abort() cancel
+     * synchronously and the completion_callback() gets called twice;
+     * once by the first error and once by the final stacked abort(),
+     * both of whom will find that all of the tasks have stopped.
+     *
+     * To avoid this problem, any stacked re-entry into this function is
+     * ineligible to fire the completion callback.  The outermost
+     * instance will take care of completing, once the stack has
+     * unwound.
+     */
+    if (stream->sync_teardown)
+        return;
+
     if (!stream->rc && rc) {
         /* First reported failure. Tear everything down. */
         stream->rc = rc;
+        stream->sync_teardown = true;
 
         libxl__stream_read_abort(egc, stream, rc);
         libxl__save_helper_abort(egc, &stream->shs);
         libxl__conversion_helper_abort(egc, &stream->chs, rc);
+
+        stream->sync_teardown = false;
     }
 
     /* Don't fire the callback until all our parallel tasks have stopped. */
diff --git a/tools/libxl/libxl_stream_write.c b/tools/libxl/libxl_stream_write.c
index ec46105..676ad0a 100644
--- a/tools/libxl/libxl_stream_write.c
+++ b/tools/libxl/libxl_stream_write.c
@@ -158,6 +158,7 @@ void libxl__stream_write_init(libxl__stream_write_state 
*stream)
     stream->rc = 0;
     stream->running = false;
     stream->in_checkpoint = false;
+    stream->sync_teardown = false;
     FILLZERO(stream->dc);
     stream->record_done_callback = NULL;
     FILLZERO(stream->emu_dc);
@@ -524,12 +525,32 @@ static void check_all_finished(libxl__egc *egc,
 {
     STATE_AO_GC(stream->ao);
 
+    /*
+     * In the case of a failure, the _abort()'s below might cancel
+     * synchronously on top of us, or asynchronously at a later point.
+     *
+     * We must avoid the situation where all _abort() cancel
+     * synchronously and the completion_callback() gets called twice;
+     * once by the first error and once by the final stacked abort(),
+     * both of whom will find that all of the tasks have stopped.
+     *
+     * To avoid this problem, any stacked re-entry into this function is
+     * ineligible to fire the completion callback.  The outermost
+     * instance will take care of completing, once the stack has
+     * unwound.
+     */
+    if (stream->sync_teardown)
+        return;
+
     if (!stream->rc && rc) {
         /* First reported failure. Tear everything down. */
         stream->rc = rc;
+        stream->sync_teardown = true;
 
         libxl__stream_write_abort(egc, stream, rc);
         libxl__save_helper_abort(egc, &stream->shs);
+
+        stream->sync_teardown = false;
     }
 
     /* Don't fire the callback until all our parallel tasks have stopped. */
--
generated by git-patchbot for /home/xen/git/xen.git#master

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog