Skip to content
This repository has been archived by the owner on May 4, 2018. It is now read-only.

Commit

Permalink
unix: update events from pevents between polls
Browse files Browse the repository at this point in the history
Watchers could be stopped between two `kevent()`/`epoll_wait()` calls
(which may happen in the same loop in `uv__io_poll()`), in such cases
`watcher->events` could be stale and won't be updated to
`watcher->pevents`.

Try to use and rely on `watcher->pevents` instead of blindly expecting
`watcher->events` to be always correct.
  • Loading branch information
indutny committed Oct 30, 2013
1 parent 08e0e63 commit 29fdb34
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 3 deletions.
4 changes: 2 additions & 2 deletions src/unix/kqueue.c
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ void uv__io_poll(uv_loop_t* loop, int timeout) {
revents = 0;

if (ev->filter == EVFILT_READ) {
if (w->events & UV__POLLIN) {
if (w->pevents & UV__POLLIN) {
revents |= UV__POLLIN;
w->rcount = ev->data;
} else {
Expand All @@ -205,7 +205,7 @@ void uv__io_poll(uv_loop_t* loop, int timeout) {
}

if (ev->filter == EVFILT_WRITE) {
if (w->events & UV__POLLOUT) {
if (w->pevents & UV__POLLOUT) {
revents |= UV__POLLOUT;
w->wcount = ev->data;
} else {
Expand Down
11 changes: 10 additions & 1 deletion src/unix/linux-core.c
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ void uv__io_poll(uv_loop_t* loop, int timeout) {
uv__io_t* w;
uint64_t base;
uint64_t diff;
unsigned int masked_events;
int nevents;
int count;
int nfds;
Expand Down Expand Up @@ -208,7 +209,15 @@ void uv__io_poll(uv_loop_t* loop, int timeout) {
continue;
}

w->cb(loop, w, pe->events);
/*
* Give users only events they're interested in. Prevents spurious
* callbacks when previous callback invocation in this loop has stopped
* the current watcher. Also, filters out events that users has not
* requested us to watch.
*/
masked_events = pe->events & w->pevents;
if (masked_events != 0)
w->cb(loop, w, masked_events);
nevents++;

This comment has been minimized.

Copy link
@bnoordhuis

bnoordhuis Nov 8, 2013

Contributor

I didn't spot this during review but there's a conceptual bug here. uv__io_poll() is only supposed to return when either:

a) the timeout expires, or
b) one or more callbacks have been made

nevents is used to keep track of b) - it's only supposed to get incremented when a callback is made. It's probably not very harmful but there's now a possibility of returning from uv__io_poll() when a) nor b) are true.

This comment has been minimized.

Copy link
@indutny

indutny Nov 8, 2013

Author Contributor

Ouch... I missed that too, want me to open a follow-up PR?

This comment has been minimized.

Copy link
@bnoordhuis

bnoordhuis Nov 8, 2013

Contributor

Well, the reason I spotted it now is that I was working on something that touches the code above. I'll fix this up while I'm there.

}

Expand Down

0 comments on commit 29fdb34

Please sign in to comment.