getdnsapi / getdns

A modern asynchronous DNS API https://getdnsapi.net/
Other
461 stars 127 forks source link

register only a single poll_t with libuv #475

Closed elindsey closed 4 years ago

elindsey commented 4 years ago

Most of the time we only need a read or a write callback registered with libuv - for example, on a UDP request a write callback is registered, when executed the write callback performs the write, deregisters itself, and registers a read callback.

However there is one case where getdns registers both read and write callbacks: when a backlog of TCP requests is going to the same upstream resolver, we use a single fd and queue the requests. In this instance we want to listen for both read (to get responses for requests we've already sent) and write (to continue to send our pending requests).

libuv, like most event libraries, only allows one callback to be registered per fd. To get notification for both reads and writes, you should examine the event flags and have appropriate conditional logic within the single callback. Today getdns incorrectly tries to register two separate poll_t with libuv, one for read and one for write - this results in a crash (internal libuv assertion guaranteeing that only a single poll_t is registered per fd).

Note that a higher qps trigger a different getdns/libuv crashing bug that occurs when the TCP backlog grows so large that requests start to time out. That crash is not addressed in this PR.

elindsey commented 4 years ago

Argh, my apologies. I was trying to submit this PR to a forked version, but clicked wrong. I'll upstream this patch after more testing though. 🙂