From: Olivier Houchard Date: Mon, 6 Jul 2026 13:36:08 +0000 (+0200) Subject: BUG/MEDIUM: fd: Fix a deadlock when closing other tgroups fds X-Git-Url: http://git.kaiwu.me/stylesheets/%22data:,/static/gitweb.js?a=commitdiff_plain;h=753aa44416f705fb16a957a1323e6090c2fcf165;p=haproxy.git BUG/MEDIUM: fd: Fix a deadlock when closing other tgroups fds Commit 061754b249b9903913d6766c1ab31bb393ee5c0d attempted to make it possible to close file descriptors belonging to other thread groups by using thread isolation. The problem is, closing other thread groups' fds usually happens when we destroy a listener, in which case we hold the listener lock. If any other thread tries to get that lock while we're waiting for the thread isolation, then they will deadlock. This can happen with any type of listener, but it is easier to reproduce with a suspend/resume loop with ABNS sockets, as a suspend translates to a close here. To fix that, instead of using thread isolation, do something similar to what's done when the fd belongs to our thread group. Increase the tgid ref counter, so that we're sure nobody will close the fd while we're dealing with it, then set the FD_MUST_CLOSE bit and set thread_mask to 0. At this point, if no thread was running on that fd, no one will and we can safely close it. So just call _fd_delete_orphan() if the running_mask is 0 and if the FD_MUST_CLOSE bit is still there, otherwise we can safely assume another thread will take care of it. This should be backported up to 2.8. --- diff --git a/src/fd.c b/src/fd.c index 6e7276f38..7df4ad0ae 100644 --- a/src/fd.c +++ b/src/fd.c @@ -363,10 +363,10 @@ void _fd_delete_orphan(int fd) /* Deletes an FD from the fdsets. The file descriptor is also closed, possibly * asynchronously. It is safe to call it from another thread from the same - * group as the FD's or from a thread from a different group. However if called - * from a thread from another group, there is an extra cost involved because - * the operation is performed under thread isolation, so doing so must be - * reserved for ultra-rare cases (e.g. stopping a listener). + * group as the FD's or from a thread from a different group. + * Calling from a thread from another group is handled by grabbing the + * reference count, adding the FD_MUST_CLOSE and doing the close ourselves + * only if there is no other thread currently using the fd. */ void fd_delete(int fd) { @@ -387,23 +387,33 @@ void fd_delete(int fd) * face the situation where we try to close an fd that was reassigned. * However there is one corner case where this happens, it's when an * attempt to pause a listener fails (e.g. abns), leaving the listener - * in fault state and it is forcefully stopped. This needs to be done - * under isolation, and it's quite rare (i.e. once per such FD per - * process). Since we'll be isolated we can clear the thread mask and - * close the FD ourselves. + * in fault state and it is forcefully stopped. + * To make it work when the fd does not belong to our thread group, + * first increase the tgid refcount, so that we're sure nobody will + * close it before we figure out if we should, then set the + * FD_MUST_CLOSE bit, so that any running thread will take care + * of closing it. + * Once we set the thread_mask to 0, we know for sure that if + * nobody is running, nobody ever will, so at this point, if + * there is no running thread, and the FD_MUST_CLOSE bit is still + * set, we know it is safe to actually close the fd. */ if (unlikely(fd_tgid(fd) != ti->tgid)) { - int must_isolate = !thread_isolated() && !(global.mode & MODE_STOPPING); + uint tgrp; - if (must_isolate) - thread_isolate(); + tgrp = fd_take_tgid(fd); + HA_ATOMIC_OR(&fdtab[fd].state, FD_MUST_CLOSE); HA_ATOMIC_STORE(&fdtab[fd].thread_mask, 0); - HA_ATOMIC_STORE(&fdtab[fd].running_mask, 0); - _fd_delete_orphan(fd); - if (must_isolate) - thread_release(); + if (!HA_ATOMIC_LOAD(&fdtab[fd].running_mask) && + HA_ATOMIC_BTR(&fdtab[fd].state, FD_MUST_CLOSE_BIT)) { + if (tgrp) + fd_drop_tgid(fd); + _fd_delete_orphan(fd); + } + else if (tgrp) + fd_drop_tgid(fd); return; }