Skip to content
Snippets Groups Projects
Commit 1e65eb54 authored by Nadav Har'El's avatar Nadav Har'El
Browse files

Fix socket poll() deadlock

In commit 7ecbf29f 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.
parent 8ebb1693
No related branches found
No related tags found
No related merge requests found
......@@ -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);
}
}
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment