-
-
Notifications
You must be signed in to change notification settings - Fork 193
Fix D++ from stalling for a second before sending REST requests using poll. #1485
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
Conversation
✅ Deploy Preview for dpp-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
braindigitalis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what exactly is this doing? it looks like it continually inserts writeable udp fd's into the poll set?
|
It inserts that wakeup udp socket into poll fds only once in constructor. Purpose of that socket is to allow poll() to exit before reaching timeout if needed. If other thread updates or registers new socket while poll() is already running and waiting for new socket events, it can't exit that state until timeout reaches or one of previously registered sockets receives event. In other socket engine implementations, like epoll(), early exits are possible and are used by lib. This additional wakeup socket basically adds this interrupt functionality to default poll(), allowing it to exit early on demand. refresh_pool() writes a byte to wakeup socket and drain_wakeup() discards that byte basically. |
|
I'm personally happy with this PR, but since I didn't work on the original implementation and I'm still a bit clueless on how the socket engine works, I'd rather @braindigitalis approve of this |
|
I've approved runners and updated the name, as this is reproduceable on Linux. |
Quite a while ago i noticed an issue that makes windows D++ builds delay sending messages, responses, and other REST requests with 1000ms delay. This PR fixes that.
Basically i added a dummy socket into poll implementation so that when poll() is running and waiting for timeout it can be interrupted by event of that dummy socket and refresh its file descriptors on demand.
To properly test this, increase poll timeout to 5000 ms and try simple ping-pong example bot. Before this change bot would hang for 5s and only then reply. Or slightly faster if heartbeat event occurs, which triggers poll activation. Issue persisted on all platforms when using default poll socket engine, both Linux and Windows.
Feel free to pinpoint in flaws that i may missed or request changes.
(they are as documented as they were before me)