-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Use of EAGAIN vs. ETIMEDOUT #2713
Comments
This is one for @somdoron I think :-) |
Good point. I think only those method explicitly accept timeout as parameter while the rest don't. |
We can consider the change, however the API already in use (although marked as draft) so people might hate us. |
Eh... if we cannot change it then why is it marked as draft? Apart from that, I think it is a good thing that zmq_poller_* is still marked as draft, since e.g. #2623 is still open, which might involve more significant changes than aligning the error codes. There is not even documentation for zmq_poller_* yet... |
Yeah I agree, if it's marked as DRAFT it's fair game, it's documented well enough (I think?) why that system exists and what it implies to enable those APIs |
For the docs I opened this to track it: #2188 |
PR is of coursed welcomed |
@somdoron I already fixed it, will send a PR tonight. I wanted to "assign" the issue (and others) to myself, but apparently I cannot do that. Isn't it sufficient that I am a "member"? |
Solution: to support the change in the DRAFT zmq_poller API, check for EAGAIN too. See: zeromq/libzmq#2713
The change makes sense. Not hating anyone, barely annoyed. But no hate. :) It's marked as draft, after all. |
Almost all libzmq public API functions that work with a timeout return EAGAIN when the timeout has been reached. However, zmq_poller_wait, zmq_poller_wait_all and zmq_poller_poll return ETIMEDOUT instead.
At least under Windows ETIMEDOUT resp. WSAETIMEDOUT is used only to indicate connection timeouts, which is also an argument for EAGAIN.
Is there a specific reason for using ETIMEDOUT? Otherwise I would change these uses to EAGAIN as well.
The text was updated successfully, but these errors were encountered: