Usedns.asyncresolver for asynchronous lookups instead of blocking 'dns.resolver.resolve'

[py-mine-bot]

Authored by ba1dr
Feb 13, 2022

---

Across the code for the asynchronous functions please replace usage of sync dns resolving.

Example - MinecraftServer.async_query:

answers = dns.resolver.resolve(host, "A")

can be replaced with:

from dns import asyncresolver
...
answers = await asyncio.wait_for(
    asyncresolver.resolve(host, "A"),
    timeout=timeout)

[py-mine-bot]

Authored by ItsDrike
Feb 13, 2022

---

DNS resolving is pretty fast and usually only happens once, on MinecraftServer.lookup (sync), this is an alternative constructor method and there is no async alternative because we generally don't expect people to make too many new instances like this, and so there was never a need to make this asynchronous, but we certainly could add an async lookup version there, which would make the SRV DNS lookup asynchronously and pass the values to __init__ afterwards.

The more complicated part to address is the query function, which currently performs the DNS resolving each time it's called. This is somewhat unnecessary and it may be a good idea to resolve this IP ahead of time. I was able to think of 4 basic ways in which we could address this:

• We could simply perform the lookup on __init__ itself and store it into a variable which the query function uses, though it isn't great because __init__ is synchronous and so it would block async workflow each time the class is initialized. However at least it would only happen once (on initialization)
• To avoid the above hassle with resolving always being synchronous, since __init__ itself is synchronous, we could simply add this A record DNS resolving into the lookup functions (which would now have both sync and async versions) and simply pass the resolved IP over to __init__ as a parameter. However this completely breaks backwards compatibility just to pass an IP address along with host and port, and this IP would only actually be used in query, which isn't that commonly used by most people anyway, making it probably not worth the compatibility breakage
• We could also do the simplest fix of just running this A record resolve function each time, but for the async version it would run asynchronously while on the sync version, it wouldn't be changed and it'd keep running synchronously.
• The above fix is very easy, however it also keeps things pretty slow for no reason, if we don't need to resolve the IP address with each call, we shouldn't, but since we also can't do it on __init__ and just making it take this resolved IP as a parameter completely breaks backwards compatibility, I believe the best solution would be to indeed perform this resolving when query is called, however once it's performed the first time, (asynchronously for the async query call, and synchronously for the sync call) we should simply cache that result (if we wanted to also preserve mutability possibilities, we could also construct a cache key, being a hash of host and port values, and make the call if this key changed).

I don't think there are any other cases of us performing DNS lookups like these. Though I do have another solution for the problem of query, which actually resolves a much deeper problem about how our UDP async connections don't resolve the address and expect IP only.

With the sync TCP versions, we can get away with passing host and port easily, because socket module implements socket.create_connection function, which performs the DNS resolving for us, and for the async TCP connections, there's asyncio.make_connection, which can also perform this resolving. However with the async UDP connections, we're using asyncio_dgram.connect and this does not run the DNS resolving, which is very weird since it makes our Connection classes inconsistent and annoying to deal with. Instead we should simply look into how asyncio.make_connection is handling DNS resolving and replicate that for the UDP async connections directly. The same issue occurs with synchronous UDP connections, since we're just initializing socket.socket directly, which doesn't perform any DNS resolving.