From 1e65eb5462b01c025f55069b7ead13eb64ef7e09 Mon Sep 17 00:00:00 2001
From: Nadav Har'El <nyh@cloudius-systems.com>
Date: Thu, 11 Jul 2013 16:18:43 +0300
Subject: [PATCH] Fix socket poll() deadlock
In commit 7ecbf29fd3965602b8a54436e3778d08934014a4 I added to the
poll_install() stage of poll() a check of the current state of the file -
to avoid the sleep if the file became ready before we managed to "install"
its poll request.
However, I wrongly believed it was necessary to put this check inside
the FD_LOCK together with the request installation. In fact, it doesn't
need to be in the same lock - all we need is for the check to happen
*after* the installation. The call to fo_poll() doesn't need to be in
the same FD_LOCK or even in an FD_LOCK at all.
Moreover, as it turns out, it must NOT be in an FD_LOCK() because this
results in a deadlock when polling sockets, caused by two different
code paths taking locks in opposite order:
1. Before this fix, poll() took FD_LOCK and called fo_poll() which
called sopoll_generic() which took a SOCKBUF_LOCK
2. In the wake path, SOCKBUF_LOCK was taken, then so_wake_poll()
is called which calls poll_wake() which takes FD_LOCK.
---
core/poll.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/core/poll.c b/core/poll.c
index 62172bac1..daaa9a150 100644
--- a/core/poll.c
+++ b/core/poll.c
@@ -198,10 +198,9 @@ void poll_install(struct pollreq* p)
FD_LOCK(fp);
TAILQ_INSERT_TAIL(&fp->f_poll_list, pl, _link);
- // We need to check for an existing event on this file here, while
- // still holding the lock, so we don't lose an event between checking
- // and installing. Note this means that poll_scan() in the beginning
- // of poll() is redundant and may be removed in the future.
+ FD_UNLOCK(fp);
+ // We need to check if we missed an event on this file just before
+ // installing the poll request on it above.
if(fo_poll(fp, entry->events)) {
// Return immediately. poll() will call poll_scan() to get the
// full list of events, and will call poll_uninstall() to undo
@@ -209,12 +208,9 @@ void poll_install(struct pollreq* p)
mtx_lock(&p->_awake_mutex);
p->_awake = true;
mtx_unlock(&p->_awake_mutex);
- FD_UNLOCK(fp);
fdrop(fp);
return;
}
- FD_UNLOCK(fp);
-
fdrop(fp);
}
}
--
GitLab