From 548b4e0ea943da765a4fec46e73484b0742f58f8 Mon Sep 17 00:00:00 2001 From: "K.G. Wang" Date: Tue, 9 Jul 2024 11:29:44 +0800 Subject: [PATCH] Calculate the actual mask to be removed in the eventloop before aeApiDelEvent (#725) for kqueue: EV_DELETE fails if the specified fd is not associated with the kqfd. If EVFILT_WRITE is associated but EVFILT_READ is not, then calling aeApiDelEvent with mask = -1 or `(AE_READABLE|AE_WRITABLE)` will cause the kevent() to fail with errno = 2(No such file or directory) and EVFILT_WRITE not dissociated. So we need to calculate the actual mask to be removed, instead of passing in whatever user provides. for evport: The comment clearly states that aeApiDelEvent "rely on the fact that our caller has already updated the mask in the eventLoop". for epoll: There's no need to calculate the "actual mask" twice, once in `aeDeleteFileEvent` and another in `aeApiDelEvent`, let's just use the mask recorded in the eventLoop. Fixes #715 Signed-off-by: wkgcass Co-authored-by: Andy Pan Co-authored-by: Binbin --- src/ae.c | 13 ++++++++++++- src/ae_epoll.c | 6 ++++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/ae.c b/src/ae.c index 28b50c660f..b6a1ce0b10 100644 --- a/src/ae.c +++ b/src/ae.c @@ -183,7 +183,9 @@ void aeDeleteFileEvent(aeEventLoop *eventLoop, int fd, int mask) { * is removed. */ if (mask & AE_WRITABLE) mask |= AE_BARRIER; - aeApiDelEvent(eventLoop, fd, mask); + /* Only remove attached events */ + mask = mask & fe->mask; + fe->mask = fe->mask & (~mask); if (fd == eventLoop->maxfd && fe->mask == AE_NONE) { /* Update the max fd */ @@ -193,6 +195,15 @@ void aeDeleteFileEvent(aeEventLoop *eventLoop, int fd, int mask) { if (eventLoop->events[j].mask != AE_NONE) break; eventLoop->maxfd = j; } + + /* Check whether there are events to be removed. + * Note: user may remove the AE_BARRIER without + * touching the actual events. */ + if (mask & (AE_READABLE | AE_WRITABLE)) { + /* Must be invoked after the eventLoop mask is modified, + * which is required by evport and epoll */ + aeApiDelEvent(eventLoop, fd, mask); + } } void *aeGetFileClientData(aeEventLoop *eventLoop, int fd) { diff --git a/src/ae_epoll.c b/src/ae_epoll.c index 78820b99bf..c8b4ac743f 100644 --- a/src/ae_epoll.c +++ b/src/ae_epoll.c @@ -87,10 +87,12 @@ static int aeApiAddEvent(aeEventLoop *eventLoop, int fd, int mask) { return 0; } -static void aeApiDelEvent(aeEventLoop *eventLoop, int fd, int delmask) { +static void aeApiDelEvent(aeEventLoop *eventLoop, int fd, int mask) { aeApiState *state = eventLoop->apidata; struct epoll_event ee = {0}; /* avoid valgrind warning */ - int mask = eventLoop->events[fd].mask & (~delmask); + + /* We rely on the fact that our caller has already updated the mask in the eventLoop. */ + mask = eventLoop->events[fd].mask; ee.events = 0; if (mask & AE_READABLE) ee.events |= EPOLLIN;