tools/libxl: Only continue stream operations if the stream is still in progress

Part of the callback contract with check_all_finished() is that each
running parallel task shall call it exactly once.

Previously, it was possible for stream_continue() or
write_toolstack_record() to fail and call into check_all_finished().  As
the save helpers callback has fired, it no longer counts as in use,
which causes check_all_finished() to fire the stream callback.  Then,
unwinding the stack back and calling check_all_finished() a second time
results in the same conditions being observed, and the stream callback
being fired a second time.

To avoid this, check_all_finished() is called before any other actions
which continue the stream functionality, and the stream is only
continued if it has not been torn down.  This guarantees not to continue
stream operations if the stream does not owe a callback to
check_all_finished().

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

diff --git a/tools/libxl/libxl_stream_read.c b/tools/libxl/libxl_stream_read.c
index d54789ca41d829affa9ec5862fcc6becd02f9527..fd3675cc591c277822ddfda38934758031497f24 100644 (file)
--- a/tools/libxl/libxl_stream_read.c
+++ b/tools/libxl/libxl_stream_read.c
@@ -738,14 +738,22 @@ void libxl__xc_domain_restore_done(libxl__egc *egc, void *dcs_void,
         goto err;
     }
 
-    /*
-     * Libxc has indicated that it is done with the stream.  Resume reading
-     * libxl records from it.
-     */
-    stream_continue(egc, stream);
-
  err:
     check_all_finished(egc, stream, rc);
+
+    /*
+     * This function is the callback associated with the save helper
+     * task, not the stream task.  We do not know whether the stream is
+     * alive, and check_all_finished() may have torn it down around us.
+     * If the stream is not still alive, we must not continue any work.
+     */
+    if (libxl__stream_read_inuse(stream)) {
+        /*
+         * Libxc has indicated that it is done with the stream.  Resume reading
+         * libxl records from it.
+         */
+        stream_continue(egc, stream);
+    }
 }
 
 static void conversion_done(libxl__egc *egc,

diff --git a/tools/libxl/libxl_stream_write.c b/tools/libxl/libxl_stream_write.c
index 676ad0ad29829ee8f71235a7738ca7788f26874c..9e9c9986f1bcc8e165546ba3eff9525dff31718e 100644 (file)
--- a/tools/libxl/libxl_stream_write.c
+++ b/tools/libxl/libxl_stream_write.c
@@ -275,10 +275,17 @@ void libxl__xc_domain_save_done(libxl__egc *egc, void *dss_void,
         goto err;
     }
 
-    write_toolstack_record(egc, stream);
-
  err:
     check_all_finished(egc, stream, rc);
+
+    /*
+     * This function is the callback associated with the save helper
+     * task, not the stream task.  We do not know whether the stream is
+     * alive, and check_all_finished() may have torn it down around us.
+     * If the stream is not still alive, we must not continue any work.
+     */
+    if (libxl__stream_write_inuse(stream))
+        write_toolstack_record(egc, stream);
 }
 
 static void write_toolstack_record(libxl__egc *egc,