From cd0ef0e09b718d8574f1b94d449e72c5469f9924 Mon Sep 17 00:00:00 2001 From: Cole Robinson Date: Wed, 3 Feb 2010 17:12:57 +0100 Subject: [PATCH] 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. --- src/libvirt_private.syms | 2 ++ src/util/logging.c | 4 ++-- src/util/logging.h | 2 ++ src/util/util.c | 10 +++++++++- 4 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index e5e8860a6a..7573af31ed 100644 --- 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 3b3c309043..11a4b06f18 100644 --- 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 49e2f6d510..5f61f5951e 100644 --- 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 cf1290dbc9..327ed8a153 100644 --- 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; -- GitLab