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

Conversation

Coldwings
Copy link
Collaborator

socket pool use wait_for_fd to check if released fd still have bytes left to read, but set timeout to 0.

since thread_usleep will always timedout when sleep time left 0, set 10us timeout, so that thread can actually put into sleep and wait for notify.

Copy link
Collaborator

@HanLee13 HanLee13 left a comment

Choose a reason for hiding this comment

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

kqueue needs exactly the same fix, which will also fix macos CI problems in client_function_test unittest.

@beef9999
Copy link
Collaborator

beef9999 commented Sep 9, 2024

merge to 0.8?

@HanLee13
Copy link
Collaborator

merge to 0.8?

Better merging from 0.6 ?

io/epoll.cpp Outdated
ret = thread_usleep(timeout);
// if timeout is just simple 0, wait for a tiny little moment
// so that events can be collect.
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

io/kqueue.cpp Outdated
@@ -99,7 +99,7 @@ class KQueue : public MasterEventEngine, public CascadingEventEngine, public Res
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);
int 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

@Coldwings Coldwings changed the title FIX: wait_for_fd in epoll should atleast put into sleep for a tiny moment FIX: wait_for_fd in epoll reap events when timeout is 0 Sep 11, 2024
@Coldwings
Copy link
Collaborator Author

Once this PR checked and approved, I will request other PR for releases from 0.6 to 0.8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants