Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX: wait_for_fd in epoll reap events when timeout is 0 #553

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 23 additions & 2 deletions io/epoll.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -299,9 +299,30 @@ ok: entry.interests |= eint;
LOG_ERROR_RETURN(EINVAL, -1, "can not wait for multiple interests");
if (unlikely(interest == 0))
return rm_interest({fd, EVENT_RWE| ONE_SHOT, 0}); // remove fd from epoll
int ret = add_interest({fd, interest | ONE_SHOT, CURRENT});
thread* current = CURRENT;
int ret = add_interest({fd, interest | ONE_SHOT, current});
if (ret < 0) LOG_ERROR_RETURN(0, -1, "failed to add event interest");
ret = thread_usleep(timeout);
// if timeout is just simple 0, wait for a tiny little moment
// so that events can be collect.
if (timeout.expired()) {
ret = -1;
wait_for_events(
0,
[current, &ret](void* data) __INLINE__ {
if ((thread*)data == current) {
ret = 0;
} else {
thread_interrupt((thread*)data, EOK);
}
},
[&]() __INLINE__ { return true; });
if (ret < 0) {
rm_interest({fd, interest, 0});
errno = ETIMEDOUT;
}
return ret;
}
ret = thread_usleep(timeout.timeout() ? timeout : Timeout(10));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case of timeout == 0, we'd better reap the events directly without blocking, so as to conform to the semantics.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

ERRNO err;
if (ret == -1 && err.no == EOK) {
return 0; // Event arrived
Expand Down
56 changes: 38 additions & 18 deletions io/kqueue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,8 @@ class KQueue : public MasterEventEngine, public CascadingEventEngine, public Res
auto entry = &_events[_n++];
EV_SET(entry, fd, event, action, event_flags, 0, udata);
if (immediate || _n == LEN(_events)) {
int ret = kevent(_kq, _events, _n, nullptr, 0, nullptr);
struct timespec tm{0, 0};
int ret = kevent(_kq, _events, _n, nullptr, 0, &tm);
if (ret < 0) {
// debug_breakpoint();
LOG_ERRNO_RETURN(0, -1, "failed to submit events with kevent()");
Expand All @@ -94,22 +95,8 @@ class KQueue : public MasterEventEngine, public CascadingEventEngine, public Res
return 0;
}

int wait_for_fd(int fd, uint32_t interests, Timeout timeout) override {
if (unlikely(interests == 0))
return 0;
short ev = (interests == EVENT_READ) ? EVFILT_READ : EVFILT_WRITE;
enqueue(fd, ev, EV_ADD | EV_ONESHOT, 0, CURRENT);
int ret = thread_usleep(timeout);
ERRNO err;
if (ret == -1 && err.no == EOK) {
return 0; // event arrived
}

errno = (ret == 0) ? ETIMEDOUT : err.no;
return -1;
}

ssize_t wait_and_fire_events(uint64_t timeout) override {
template<typename EVCB>
ssize_t do_wait_and_fire_events(uint64_t timeout, EVCB&& event_callback) {
ssize_t nev = 0;
struct timespec tm;
tm.tv_sec = timeout / 1000 / 1000;
Expand All @@ -125,7 +112,7 @@ class KQueue : public MasterEventEngine, public CascadingEventEngine, public Res
for (int i = 0; i < ret; ++i) {
if (_events[i].filter == EVFILT_USER) continue;
auto th = (thread*) _events[i].udata;
if (th) thread_interrupt(th, EOK);
if (th) event_callback(th);
}
if (ret == (int) LEN(_events)) { // there may be more events
tm.tv_sec = tm.tv_nsec = 0;
Expand All @@ -134,6 +121,39 @@ class KQueue : public MasterEventEngine, public CascadingEventEngine, public Res
return nev;
}

int wait_for_fd(int fd, uint32_t interests, Timeout timeout) override {
short ev = (interests == EVENT_READ) ? EVFILT_READ : EVFILT_WRITE;
auto current = CURRENT;
enqueue(fd, ev, EV_ADD | EV_ONESHOT, 0, current, true);
if (timeout.expired()) {
int ret = -1;
do_wait_and_fire_events(0, [current, &ret](thread* th) {
if (th == current)
ret = 0;
else
thread_interrupt(th, EOK);
});
if (ret <0) {
enqueue(fd, ev, EV_DELETE, 0, current, true);
errno = ETIMEDOUT;
}
return ret;
}
int ret = thread_usleep(timeout);
ERRNO err;
if (ret == -1 && err.no == EOK) {
return 0; // event arrived
}

errno = (ret == 0) ? ETIMEDOUT : err.no;
enqueue(fd, ev, EV_DELETE, 0, current, true);
return -1;
}

ssize_t wait_and_fire_events(uint64_t timeout) override {
return do_wait_and_fire_events(timeout, [](thread *th) { thread_interrupt(th, EOK); });
}

int cancel_wait() override {
enqueue(_kq, EVFILT_USER, EV_ONESHOT, NOTE_TRIGGER, nullptr, true);
return 0;
Expand Down
9 changes: 5 additions & 4 deletions net/pooled_socket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,10 @@ class TCPSocketPool : public ForwardSocketClient {
photon::Timer timer;

// all fd < 0 treated as socket not based on fd
// and always alive. Using such socket needs user
// and always reuseable. Using such socket needs user
// to check if connected socket is still usable.
bool stream_alive(int fd) {
// if there still have unread bytes in strema, it should be closed.
bool stream_reusable(int fd) {
return (fd < 0) || (wait_for_fd_readable(fd, 0) != 0);
}

Expand Down Expand Up @@ -196,7 +197,7 @@ class TCPSocketPool : public ForwardSocketClient {
if (!stream) {
stream = m_underlay->connect(remote, local);
if (!stream) return nullptr;
} else if (!stream_alive(stream->get_underlay_fd())) {
} else if (!stream_reusable(stream->get_underlay_fd())) {
delete stream;
goto again;
}
Expand Down Expand Up @@ -231,7 +232,7 @@ class TCPSocketPool : public ForwardSocketClient {
bool release(const EndPoint& ep, ISocketStream* stream) {
auto fd = stream->get_underlay_fd();
ERRNO err;
if (!stream_alive(fd)) return false;
if (!stream_reusable(fd)) return false;
auto node = new StreamListNode(ep, stream, fd, TTL_us);
push_into_pool(node);
errno = err.no;
Expand Down
Loading