From 2261b6f8fd6dbee9fa0d7da36f909f0a2e423c99 Mon Sep 17 00:00:00 2001 From: lihuiba Date: Thu, 21 Mar 2024 12:12:38 +0800 Subject: [PATCH] add judgement for conflicted ONE_SHOT flag in add_interests() improve logic in add_interests() and rm_interests() --- io/epoll.cpp | 92 +++++++++++++++++++++++++++++----------------------- 1 file changed, 52 insertions(+), 40 deletions(-) diff --git a/io/epoll.cpp b/io/epoll.cpp index ded985f4..5c3bc7fb 100644 --- a/io/epoll.cpp +++ b/io/epoll.cpp @@ -49,6 +49,8 @@ struct InFlightEvent { void* error_data; }; +const static uint32_t EVENT_RWEO = EVENT_RWE | ONE_SHOT; + class EventEngineEPoll : public MasterEventEngine, public CascadingEventEngine, public ResetHandle { public: int _evfd = -1; @@ -97,9 +99,9 @@ class EventEngineEPoll : public MasterEventEngine, public CascadingEventEngine, int ret = epoll_ctl(_engine_fd, op, fd, &ev); if (ret < 0) { ERRNO err; - if (err.no != no_log_errno_1 && - err.no != no_log_errno_2) { // deleting a non-existing fd is - // considered OK + // some errno may be ignored, such as deleting a non-existing fd, etc. + if ((no_log_errno_1 == 0 || no_log_errno_1 != err.no) && + (no_log_errno_2 == 0 || no_log_errno_2 != err.no)) { auto events = HEX(ev.events); auto data = ev.data.ptr; LOG_WARN("failed to call epoll_ctl(`, `, `, {`, `})", @@ -115,38 +117,50 @@ class EventEngineEPoll : public MasterEventEngine, public CascadingEventEngine, virtual int add_interest(Event e) override { if (e.fd < 0) LOG_ERROR_RETURN(EINVAL, -1, "invalid file descriptor ", e.fd); + if (unlikely(!e.interests)) + return 0; if (unlikely((size_t)e.fd >= _inflight_events.size())) _inflight_events.resize(e.fd * 2); + + e.interests &= EVENT_RWEO; auto& entry = _inflight_events[e.fd]; - auto intersection = e.interests & entry.interests & EVENT_RWE; - auto data = (entry.reader_data != e.data) * EVENT_READ | - (entry.writer_data != e.data) * EVENT_WRITE | - (entry.error_data != e.data) * EVENT_ERROR ; - if (intersection & data) - LOG_ERROR_RETURN(EALREADY, -1, "conflicted interest(s)"); + auto eint = entry.interests & EVENT_RWEO; + int op; + if (!eint) { + eint = e.interests; + op = EPOLL_CTL_ADD; + } else { + if ((eint ^ e.interests) & ONE_SHOT) + LOG_ERROR_RETURN(EALREADY, -1, "conflicted ONE_SHOT flag"); + auto intersection = e.interests & eint; + auto data = (entry.reader_data != e.data) * EVENT_READ | + (entry.writer_data != e.data) * EVENT_WRITE | + (entry.error_data != e.data) * EVENT_ERROR ; + if (intersection & data) + LOG_ERROR_RETURN(EALREADY, -1, "conflicted interest(s)"); + + eint |= e.interests; + op = EPOLL_CTL_MOD; + } - int ret; - auto eint = entry.interests; - auto x = (eint | e.interests) & EVENT_RWE; - auto events = evmap.translate_bitwisely(x); - if (likely(e.interests & ONE_SHOT)) { + auto events = evmap.translate_bitwisely(eint); + if (likely(eint & ONE_SHOT)) { events |= EPOLLONESHOT; - if (likely(eint & ONE_SHOT)) { - ret = ctl(e.fd, EPOLL_CTL_MOD, events); - if (unlikely(ret < 0 && errno == ENOENT)) - ret = ctl(e.fd, EPOLL_CTL_ADD, events); - } else { - if (eint != 0) LOG_ERROR_RETURN(EINVAL, -1, "conflicted interest(s) regarding ONE_SHOT"); - ret = ctl(e.fd, EPOLL_CTL_ADD, events); + if (likely(op == EPOLL_CTL_MOD)) { + // This may falsely fail with errno == ENOENT, + // if the fd was closed and created again. In + // such cases we should suppress the error log, + if (likely(ctl(e.fd, op, events, ENOENT) == 0)) goto ok; + else if (errno != ENOENT) /* unexpected */ goto fail; + + op = EPOLL_CTL_ADD; // and do it again with EPOLL_CTL_ADD. } - } else { - auto op = (eint & EVENT_RWE) ? EPOLL_CTL_MOD : EPOLL_CTL_ADD; - ret = ctl(e.fd, op, events); } - if (ret < 0) - LOG_ERROR_RETURN(0, ret, "failed to add_interest()"); + if (ctl(e.fd, op, events) < 0) { fail: + LOG_ERROR_RETURN(0, -1, "failed to add_interest()"); + } - entry.interests |= e.interests; +ok: entry.interests |= eint; if (e.interests & EVENT_READ) entry.reader_data = e.data; if (e.interests & EVENT_WRITE) entry.writer_data = e.data; if (e.interests & EVENT_ERROR) entry.error_data = e.data; @@ -158,25 +172,23 @@ class EventEngineEPoll : public MasterEventEngine, public CascadingEventEngine, LOG_ERROR_RETURN(EINVAL, -1, "invalid file descriptor ", e.fd); if (unlikely(e.interests == 0)) return 0; auto& entry = _inflight_events[e.fd]; - auto eint = entry.interests & (EVENT_RWE | ONE_SHOT); + auto eint = entry.interests & EVENT_RWEO; auto intersection = e.interests & eint; if (intersection == 0) return 0; auto remain = eint ^ intersection; // ^ is to flip intersected bits - if (remain == ONE_SHOT) {/* no need to epoll_ctl() */} else { - uint32_t op, events = 0; - if (remain) { - events = evmap.translate_bitwisely(remain); - if (remain & ONE_SHOT) events |= EPOLLONESHOT; - op = EPOLL_CTL_MOD; - } else { - op = EPOLL_CTL_DEL; - } - if (ctl(e.fd, op, events) < 0) - LOG_ERROR_RETURN(0, -1, "failed to rm_interest()"); + if (likely(remain == ONE_SHOT)) { + /* no need to epoll_ctl() */ + } else if (likely(!remain)) { + ctl(e.fd, EPOLL_CTL_DEL, 0, ENOENT); + } else { + auto events = evmap.translate_bitwisely(remain); + if (remain & ONE_SHOT) events |= EPOLLONESHOT; + if (ctl(e.fd, EPOLL_CTL_MOD, events) < 0) + LOG_ERROR_RETURN(0, -1, "failed to rm_interest()"); } - entry.interests ^= intersection; + entry.interests ^= intersection; // ^ is to flip intersected bits if (intersection & EVENT_READ) entry.reader_data = nullptr; if (intersection & EVENT_WRITE) entry.writer_data = nullptr; if (intersection & EVENT_ERROR) entry.error_data = nullptr;