[libvirt] [PATCH] virsh: Fix possible deadlock when virsh is about to exit
Not only was ctl->quit accessed without a mutex but unfortunately, virEventAddTimeout only interrupts the poll when event loop is running so the hack needs to add a timeout that will make next poll return immediately without blocking. --- tools/virsh.c | 39 +++++++++++++++++++++++++++++++++------ 1 files changed, 33 insertions(+), 6 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 618b0c1..723ec65 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -251,6 +251,7 @@ typedef struct __vshControl { bool useSnapshotOld; /* cannot use virDomainSnapshotGetParent or virDomainSnapshotNumChildren */ virThread eventLoop; + virMutex lock; bool eventLoopStarted; bool quit; } __vshControl; @@ -16796,10 +16797,17 @@ vshEventLoop(void *opaque) { vshControl *ctl = opaque; - while (!ctl->quit) { - if (virEventRunDefaultImpl() < 0) { + while (1) { + bool quit; + virMutexLock(&ctl->lock); + quit = ctl->quit; + virMutexUnlock(&ctl->lock); + + if (quit) + break; + + if (virEventRunDefaultImpl() < 0) virshReportError(ctl); - } } } @@ -17235,13 +17243,18 @@ vshReadline (vshControl *ctl, const char *prompt) #endif /* !USE_READLINE */ +static void +vshDeinitTimer(int timer ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) +{ + /* nothing to be done here */ +} + /* * Deinitialize virsh */ static bool vshDeinit(vshControl *ctl) { - ctl->quit = true; vshReadlineDeinit(ctl); vshCloseLogFile(ctl); VIR_FREE(ctl->name); @@ -17254,15 +17267,24 @@ vshDeinit(vshControl *ctl) virResetLastError(); if (ctl->eventLoopStarted) { + int timer; + + virMutexLock(&ctl->lock); + ctl->quit = true; /* HACK: Add a dummy timeout to break event loop */ - int timer = virEventAddTimeout(-1, NULL, NULL, NULL); + timer = virEventAddTimeout(0, vshDeinitTimer, NULL, NULL); + virMutexUnlock(&ctl->lock); + + virThreadJoin(&ctl->eventLoop); + if (timer != -1) virEventRemoveTimeout(timer); - virThreadJoin(&ctl->eventLoop); ctl->eventLoopStarted = false; } + virMutexDestroy(&ctl->lock); + return true; } @@ -17543,6 +17565,11 @@ main(int argc, char **argv) return EXIT_FAILURE; } + if (virMutexInit(&ctl->lock) < 0) { + vshError(ctl, "%s", _("Failed to initialize mutex")); + return EXIT_FAILURE; + } + if (virInitialize() < 0) { vshError(ctl, "%s", _("Failed to initialize libvirt")); return EXIT_FAILURE; -- 1.7.8.rc4
On 11/30/2011 12:42 PM, Jiri Denemark wrote:
Not only was ctl->quit accessed without a mutex but unfortunately, virEventAddTimeout only interrupts the poll when event loop is running so the hack needs to add a timeout that will make next poll return immediately without blocking. --- tools/virsh.c | 39 +++++++++++++++++++++++++++++++++------ 1 files changed, 33 insertions(+), 6 deletions(-)
ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Wed, Nov 30, 2011 at 02:59:56PM -0700, Eric Blake wrote:
On 11/30/2011 12:42 PM, Jiri Denemark wrote:
Not only was ctl->quit accessed without a mutex but unfortunately, virEventAddTimeout only interrupts the poll when event loop is running so the hack needs to add a timeout that will make next poll return immediately without blocking. --- tools/virsh.c | 39 +++++++++++++++++++++++++++++++++------ 1 files changed, 33 insertions(+), 6 deletions(-)
ACK.
ACK too, and pushed ! thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
Dňa 30.11.2011 20:42, Jiri Denemark wrote / napísal(a):
Not only was ctl->quit accessed without a mutex but unfortunately, virEventAddTimeout only interrupts the poll when event loop is running so the hack needs to add a timeout that will make next poll return immediately without blocking. --- tools/virsh.c | 39 +++++++++++++++++++++++++++++++++------ 1 files changed, 33 insertions(+), 6 deletions(-)
This patch fixes it for me. ACK. Without this patch there was approximately a 1 in 5 chance that one of the tests would hang. With this patch, the test ran for 5 hours in a loop without hanging. Peter
participants (4)
-
Daniel Veillard -
Eric Blake -
Jiri Denemark -
Peter Krempa