[PATCH] Fix deadlock in handling EOF in LXC monitor

["Daniel P. Berrange" <berrange@xxxxxxxxxx>]

From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>

Depending on the scenario in which LXC containers exit, it is
possible for the EOF callback of the LXC monitor to deadlock
the driver. When the driver calls virLXCMonitorClose, there
is really no need for the EOF callback to be invoked in this
case, since the caller can easily handle events itself.

In changing this, the monitor needs to take a deep copy of
the callback list, not merely a reference.

Also adds debug statements in various places to aid
troubleshooting

Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
---
 src/lxc/lxc_monitor.c | 32 ++++++++++++++++++++++----------
 src/lxc/lxc_process.c |  6 +++++-
 2 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/src/lxc/lxc_monitor.c b/src/lxc/lxc_monitor.c
index 772c613..3e00751 100644
--- a/src/lxc/lxc_monitor.c
+++ b/src/lxc/lxc_monitor.c
@@ -40,7 +40,7 @@ struct _virLXCMonitor {
     virMutex lock;
 
     virDomainObjPtr vm;
-    virLXCMonitorCallbacksPtr cb;
+    virLXCMonitorCallbacks cb;
 
     virNetClientPtr client;
     virNetClientProgramPtr program;
@@ -83,8 +83,8 @@ virLXCMonitorHandleEventExit(virNetClientProgramPtr prog ATTRIBUTE_UNUSED,
     virLXCProtocolExitEventMsg *msg = evdata;
 
     VIR_DEBUG("Event exit %d", msg->status);
-    if (mon->cb->exitNotify)
-        mon->cb->exitNotify(mon, msg->status, mon->vm);
+    if (mon->cb.exitNotify)
+        mon->cb.exitNotify(mon, msg->status, mon->vm);
 }
 
 
@@ -96,20 +96,25 @@ static void virLXCMonitorEOFNotify(virNetClientPtr client ATTRIBUTE_UNUSED,
     virLXCMonitorCallbackEOFNotify eofNotify;
     virDomainObjPtr vm;
 
-    VIR_DEBUG("EOF notify");
+    VIR_DEBUG("EOF notify mon=%p", mon);
     virLXCMonitorLock(mon);
-    eofNotify = mon->cb->eofNotify;
+    eofNotify = mon->cb.eofNotify;
     vm = mon->vm;
     virLXCMonitorUnlock(mon);
 
-    eofNotify(mon, vm);
+    if (eofNotify) {
+        VIR_DEBUG("EOF callback mon=%p vm=%p", mon, vm);
+        eofNotify(mon, vm);
+    } else {
+        VIR_DEBUG("No EOF callback");
+    }
 }
 
 
 static void virLXCMonitorCloseFreeCallback(void *opaque)
 {
     virLXCMonitorPtr mon = opaque;
-    virObjectUnref(mon);;
+    virObjectUnref(mon);
 }
 
 
@@ -155,7 +160,7 @@ virLXCMonitorPtr virLXCMonitorNew(virDomainObjPtr vm,
         goto error;
 
     mon->vm = vm;
-    mon->cb = cb;
+    memcpy(&mon->cb, cb, sizeof(mon->cb));
 
     virObjectRef(mon);
     virNetClientSetCloseCallback(mon->client, virLXCMonitorEOFNotify, mon,
@@ -179,8 +184,8 @@ static void virLXCMonitorDispose(void *opaque)
     virLXCMonitorPtr mon = opaque;
 
     VIR_DEBUG("mon=%p", mon);
-    if (mon->cb && mon->cb->destroy)
-        (mon->cb->destroy)(mon, mon->vm);
+    if (mon->cb.destroy)
+        (mon->cb.destroy)(mon, mon->vm);
     virMutexDestroy(&mon->lock);
     virObjectUnref(mon->program);
 }
@@ -188,7 +193,14 @@ static void virLXCMonitorDispose(void *opaque)
 
 void virLXCMonitorClose(virLXCMonitorPtr mon)
 {
+    VIR_DEBUG("mon=%p", mon);
     if (mon->client) {
+        /* When manually closing the monitor, we don't
+         * want to have callbacks back into us, since
+         * the caller is not re-entrant safe
+         */
+        VIR_DEBUG("Clear EOF callback mon=%p", mon);
+        mon->cb.eofNotify = NULL;
         virNetClientClose(mon->client);
         virObjectUnref(mon->client);
         mon->client = NULL;
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index b9cff85..079bc3a 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -169,6 +169,8 @@ virLXCProcessReboot(virLXCDriverPtr driver,
     int ret = -1;
     virDomainDefPtr savedDef;
 
+    VIR_DEBUG("Faking reboot");
+
     if (conn) {
         virConnectRef(conn);
         autodestroy = true;
@@ -555,13 +557,15 @@ cleanup:
 
 
 extern virLXCDriverPtr lxc_driver;
-static void virLXCProcessMonitorEOFNotify(virLXCMonitorPtr mon ATTRIBUTE_UNUSED,
+static void virLXCProcessMonitorEOFNotify(virLXCMonitorPtr mon,
                                           virDomainObjPtr vm)
 {
     virLXCDriverPtr driver = lxc_driver;
     virDomainEventPtr event = NULL;
     virLXCDomainObjPrivatePtr priv;
 
+    VIR_DEBUG("mon=%p vm=%p", mon, vm);
+
     lxcDriverLock(driver);
     virDomainObjLock(vm);
     lxcDriverUnlock(driver);
-- 
1.7.11.2

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list