From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: [PATCH v2 for-4.6 2/3] tools/libxl: Do not fire the stream callback multiple times Date: Mon, 27 Jul 2015 17:47:26 +0100 Message-ID: <1438015647-25377-3-git-send-email-andrew.cooper3@citrix.com> References: <1438015647-25377-1-git-send-email-andrew.cooper3@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1438015647-25377-1-git-send-email-andrew.cooper3@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Xen-devel Cc: Andrew Cooper , Ian Jackson , Ian Campbell , Wei Liu List-Id: xen-devel@lists.xenproject.org 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 --- CC: Ian Campbell CC: Ian Jackson CC: Wei Liu --- 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(+) diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h index f2466dc..e2bf327 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -3010,6 +3010,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. */ @@ -3351,6 +3352,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 4458aec..65100f4 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. */ -- 1.7.10.4