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

Event Subsystem: No longer require integrators to have their own #696

Merged
merged 29 commits into from Jan 24, 2024

Conversation

bradh352
Copy link
Member

@bradh352 bradh352 commented Jan 21, 2024

This PR implements an event thread to process all events on file descriptors registered by c-ares. Prior to this feature, integrators were required to understand the internals of c-ares and how to monitor file descriptors and timeouts and process events.

Implements OS-specific efficient polling such as epoll(), kqueue(), or IOCP, and falls back to poll() or select() if otherwise unsupported. At this point, it depends on basic threading primitives such as pthreads or windows threads.

If enabled via the ARES_OPT_EVENT_THREAD option passed to ares_init_options(), then socket callbacks cannot be used.

Fixes Bug: #611
Fix By: Brad House (@bradh352)

@bradh352 bradh352 marked this pull request as draft January 21, 2024 21:09
@bradh352
Copy link
Member Author

@piscisaureus you appear to be the expert in Windows AFD and I was wondering if you'd be kind enough to answer a few questions.

I noticed in your "wepoll" you open \Device\Afd\wepoll and enqueue all IOCTL_AFD_POLL for each socket against that open handle, but in libuv, instead you created an OVERLAPPED peer socket with WSASocketW() and then register that with IOCP and submit IOCTL_AFD_POLL against that peer socket.

That said, it appears libuv has a lot of fallback handling for failures for the WSASocketW() sequence then fall back to a slow path. I'm not really seeing any fallback mechanisms in "wepoll".

I've implemented the latter (libuv) style approach in src/lib/ares_event_win32.c as I didn't really like the container_of() or CONTAINING_RECORD() approaches to determining the right address of the private per-connection data structure, as I can just register it as the CompletionKey (as a side note, I'm not sure why libuv is using CONTAINING_RECORD() instead of the CompletionKey).

The main question I have, is WSASocketW() known to fail in some circumstances? If so what are those? Is opening a handle to \Device\Afd\blah considered safer, more reliable, or more versatile in any way?

Thanks!

@bradh352 bradh352 marked this pull request as ready for review January 22, 2024 20:32
@bradh352 bradh352 requested a review from bagder January 22, 2024 20:32
@bradh352 bradh352 changed the title Event Subsystem: No longer require implementors to have their own Event Subsystem: No longer require integrators to have their own Jan 24, 2024
@bradh352 bradh352 merged commit 7963c51 into c-ares:main Jan 24, 2024
38 checks passed
@bradh352 bradh352 deleted the eventthread branch January 24, 2024 13:43
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Jan 28, 2024
## c-ares version 1.26.0 - Jan 26 2024

Features:

* Event Thread support.  Integrators are no longer requried to monitor the
  file descriptors registered by c-ares for events and call `ares_process()`
  when enabling the event thread feature via `ARES_OPT_EVENT_THREAD` passed
  to `ares_init_options()`. [PR #696](c-ares/c-ares#696)
* Added flags to `are_dns_parse()` to force RAW packet parsing.
  [PR #693](c-ares/c-ares#693)

Changes:

* Mark `ares_fds()` as deprected.
  [PR #691](c-ares/c-ares#691)

Bugfixes:

* `adig`: Differentiate between internal and server errors.
  [e10b16a](c-ares/c-ares@e10b16a)
* Autotools allow make to override CFLAGS/CPPFLAGS/CXXFLAGS.
  [PR #695](c-ares/c-ares#695)
* Autotools: fix building for 32bit windows due to stdcall symbol mangling.
  [PR #689](c-ares/c-ares#689)
* RR Name should not be sanity checked against the Question.
  [PR #685](c-ares/c-ares#685)
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.

None yet

1 participant