[PATCH] node_device_udev: Don't hold the object lock while joining the udev thread
We found libvirt has a race condition bug and it causes systemd timeout during system shutdown. This issue does not exist on libvirt 10.0.0 with Ubuntu 24.04 but found in Ubuntu 26.04 which uses libvirt 12.0.0. nodeStateShutdownWait() joins the udev event thread while holding @priv's object lock: VIR_WITH_OBJECT_LOCK_GUARD(priv) { if (priv->udevThread) virThreadJoin(priv->udevThread); } but udevEventHandleThread() needs that same lock to make progress. Its only clean exit path is VIR_WITH_OBJECT_LOCK_GUARD(priv) { while (!priv->udevDataReady && !priv->udevThreadQuit) { if (virCondWait(&priv->udevThreadCond, &priv->parent.lock)) ... } if (priv->udevThreadQuit) return; so the thread has to re-acquire the lock - either on return from virCondWait() or to re-enter the guard at the top of its loop - before it can observe @priv->udevThreadQuit and return. nodeStateShutdownPrepare() sets udevThreadQuit, signals the condition and drops the lock. Which thread grabs the lock next then decides the outcome: - the udev thread wins: it sees udevThreadQuit, returns, and the subsequent join completes immediately. - nodeStateShutdownWait() wins: it takes the lock and blocks in virThreadJoin(), so the udev thread can never re-acquire the lock, never observes udevThreadQuit and never returns. Both threads are then stuck for good. Holding the lock across the join serves no purpose - udevThreadQuit has already been published by nodeStateShutdownPrepare(), @priv->udevThread is only assigned in nodeStateInitialize() and released by udevEventDataDispose(), and the udev thread holds a reference to @priv. Join outside the lock guard instead, which also restores the ordering nodeStateCleanup() used before commit e89d39f5b8f3 ("node_device_udev: Introduce and use `stateShutdownPrepare` and `stateShutdownWait`"): signal under the lock, release it, then join. Fixes: e89d39f5b8f30f0b3284ed2bd71cee1a8a3d707d Signed-off-by: Yifan Li <yifan2.li@intel.com> Signed-off-by: Jianfeng Gao <jianfeng.gao@intel.com> Signed-off-by: Lili Li <lili.li@intel.com> --- How found: shutdown on Ubuntu 26.04 with libvirt 12.0.0(monolithic libvirtd, socket activated), found it was killed by systemd due to timeout. In our testing(repeated S5 cycles), it is about 4% hit rate. State of a hung daemon: tid comm wchan 14039 libvirtd poll_schedule_timeout 14108 udev-event futex_do_wait 14165 daemon-shutdown futex_do_wait Thread "daemon-shutdown" (LWP 14165): #5 __pthread_clockjoin_ex () #6 libvirt_driver_nodedev.so #7 virStateShutdownWait () from libvirt.so.0 Thread "udev-event" (LWP 14108): #1 __GI___lll_lock_wait (futex=0x7d55d001cc58) #2 __pthread_mutex_cond_lock (mutex=0x7d55d001cc58) #4 ___pthread_cond_wait () #5 virCondWait () from libvirt.so.0 #6 libvirt_driver_nodedev.so (gdb) x/4dw 0x7d55d001cc58 0x7d55d001cc58: 2 0 14165 2 __lock __count __owner __nusers __owner is the daemon-shutdown thread itself, parked in pthread_join() waiting for the very thread that needs that lock in order to exit. src/node_device/node_device_udev.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 104433fb88..ac9be10382 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -2482,10 +2482,17 @@ nodeStateShutdownWait(void) if (!priv) return 0; - VIR_WITH_OBJECT_LOCK_GUARD(priv) { - if (priv->udevThread) - virThreadJoin(priv->udevThread); - } + /* Do not hold @priv's lock while joining the udev thread. + * udevEventHandleThread() must acquire that very lock in order to + * observe @priv->udevThreadQuit - already set by + * nodeStateShutdownPrepare() - and return. Joining while holding the + * lock therefore deadlocks the two threads against each other. + * + * @priv->udevThread is only ever assigned in nodeStateInitialize() and + * released by udevEventDataDispose(), and the udev thread holds a + * reference to @priv, so accessing it here without the lock is safe. */ + if (priv->udevThread) + virThreadJoin(priv->udevThread); if (priv->workerPool) virThreadPoolDrain(priv->workerPool); -- 2.48.1
On 9/22/26 12:27, Yifan Li wrote:
We found libvirt has a race condition bug and it causes systemd timeout during system shutdown. This issue does not exist on libvirt 10.0.0 with Ubuntu 24.04 but found in Ubuntu 26.04 which uses libvirt 12.0.0.
nodeStateShutdownWait() joins the udev event thread while holding @priv's object lock:
VIR_WITH_OBJECT_LOCK_GUARD(priv) { if (priv->udevThread) virThreadJoin(priv->udevThread); }
but udevEventHandleThread() needs that same lock to make progress. Its only clean exit path is
VIR_WITH_OBJECT_LOCK_GUARD(priv) { while (!priv->udevDataReady && !priv->udevThreadQuit) { if (virCondWait(&priv->udevThreadCond, &priv->parent.lock)) ... } if (priv->udevThreadQuit) return;
so the thread has to re-acquire the lock - either on return from virCondWait() or to re-enter the guard at the top of its loop - before it can observe @priv->udevThreadQuit and return.
nodeStateShutdownPrepare() sets udevThreadQuit, signals the condition and drops the lock. Which thread grabs the lock next then decides the outcome:
- the udev thread wins: it sees udevThreadQuit, returns, and the subsequent join completes immediately. - nodeStateShutdownWait() wins: it takes the lock and blocks in virThreadJoin(), so the udev thread can never re-acquire the lock, never observes udevThreadQuit and never returns. Both threads are then stuck for good.
Holding the lock across the join serves no purpose - udevThreadQuit has already been published by nodeStateShutdownPrepare(), @priv->udevThread is only assigned in nodeStateInitialize() and released by udevEventDataDispose(), and the udev thread holds a reference to @priv. Join outside the lock guard instead, which also restores the ordering nodeStateCleanup() used before commit e89d39f5b8f3 ("node_device_udev: Introduce and use `stateShutdownPrepare` and `stateShutdownWait`"): signal under the lock, release it, then join.
Fixes: e89d39f5b8f30f0b3284ed2bd71cee1a8a3d707d Signed-off-by: Yifan Li <yifan2.li@intel.com> Signed-off-by: Jianfeng Gao <jianfeng.gao@intel.com> Signed-off-by: Lili Li <lili.li@intel.com> --- How found: shutdown on Ubuntu 26.04 with libvirt 12.0.0(monolithic libvirtd, socket activated), found it was killed by systemd due to timeout. In our testing(repeated S5 cycles), it is about 4% hit rate.
State of a hung daemon:
tid comm wchan 14039 libvirtd poll_schedule_timeout 14108 udev-event futex_do_wait 14165 daemon-shutdown futex_do_wait
Thread "daemon-shutdown" (LWP 14165): #5 __pthread_clockjoin_ex () #6 libvirt_driver_nodedev.so #7 virStateShutdownWait () from libvirt.so.0
Thread "udev-event" (LWP 14108): #1 __GI___lll_lock_wait (futex=0x7d55d001cc58) #2 __pthread_mutex_cond_lock (mutex=0x7d55d001cc58) #4 ___pthread_cond_wait () #5 virCondWait () from libvirt.so.0 #6 libvirt_driver_nodedev.so
(gdb) x/4dw 0x7d55d001cc58 0x7d55d001cc58: 2 0 14165 2 __lock __count __owner __nusers
__owner is the daemon-shutdown thread itself, parked in pthread_join() waiting for the very thread that needs that lock in order to exit.
src/node_device/node_device_udev.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> and merged. Congratulations on your first libvirt contribution! Michal
participants (2)
-
Michal Prívozník -
Yifan Li