Fix log locking problem when using fork() in the library

Ad pointed out by Dan Berrange:
So if some thread in libvirtd is currently executing a logging call,
while another thread calls virExec(), that other thread no longer
exists in the child, but its lock is never released. So when the
child then does virLogReset() it deadlocks.

The only way I see to address this, is for the parent process to call
virLogLock(), immediately before fork(), and then virLogUnlock()
afterwards in both parent & child. This will ensure that no other
thread
can be holding the lock across fork().

* src/util/logging.[ch] src/libvirt_private.syms: export virLogLock() and
  virLogUnlock()
* src/util/util.c: lock just before forking and unlock just after - in
  both parent and child.

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index e5e8860a6a3121705e0903d08b08ffefbba3869a..7573af31ed7289df96ed8be8cebc68a18f688a9f 100644 (file)
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -366,6 +366,8 @@ virLogParseOutputs;
 virLogStartup;
 virLogShutdown;
 virLogReset;
+virLogLock;
+virLogUnlock;
 
 
 # memory.h

diff --git a/src/util/logging.c b/src/util/logging.c
index 3b3c3090436a2ddfdac83e8c7b12815d89e45f14..11a4b06f181b83d432cce63b7ae153005a9b80e4 100644 (file)
--- a/src/util/logging.c
+++ b/src/util/logging.c
@@ -133,11 +133,11 @@ static int virLogResetOutputs(void);
  */
 virMutex virLogMutex;
 
-static void virLogLock(void)
+void virLogLock(void)
 {
     virMutexLock(&virLogMutex);
 }
-static void virLogUnlock(void)
+void virLogUnlock(void)
 {
     virMutexUnlock(&virLogMutex);
 }

diff --git a/src/util/logging.h b/src/util/logging.h
index 49e2f6d5101b5805c3450477bfbb292a53493615..5f61f5951e7d452d90f4e06dee1648a2ef7e733e 100644 (file)
--- a/src/util/logging.h
+++ b/src/util/logging.h
@@ -128,6 +128,8 @@ extern int virLogDefineOutput(virLogOutputFunc f, virLogCloseFunc c, void *data,
  * Internal logging API
  */
 
+extern void virLogLock(void);
+extern void virLogUnlock(void);
 extern int virLogStartup(void);
 extern int virLogReset(void);
 extern void virLogShutdown(void);

diff --git a/src/util/util.c b/src/util/util.c
index cf1290dbc9fda894b49552ea949de3f967c24a17..327ed8a1531807258a46e983728601369f5815df 100644 (file)
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -415,7 +415,15 @@ __virExec(virConnectPtr conn,
         childerr = null;
     }
 
-    if ((pid = fork()) < 0) {
+    /* Ensure we hold the logging lock, to protect child processes
+     * from deadlocking on another threads inheirited mutex state */
+    virLogLock();
+    pid = fork();
+
+    /* Unlock for both parent and child process */
+    virLogUnlock();
+
+    if (pid < 0) {
         virReportSystemError(conn, errno,
                              "%s", _("cannot fork child process"));
         goto cleanup;