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 <[email protected]>
> Signed-off-by: Jianfeng Gao <[email protected]>
> Signed-off-by: Lili Li <[email protected]>
> ---
> 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 <[email protected]>
and merged. Congratulations on your first libvirt contribution!
Michal
