gera2ld / async_dns

DNS library based on asyncio
MIT License
70 stars 17 forks source link

Subsequent calls to DNS Client query fails #26

Closed leandroltavares closed 3 years ago

leandroltavares commented 3 years ago

Subsequent calls to DnsClient fails . Both tests pass independently, but when I run the whole suite the second one timeouts. The following code was used to reproduce the issue using the latest alpha ==2.0.0a2. Python version is 3.9.5

import unittest
from unittest import IsolatedAsyncioTestCase

from async_dns.resolver.client import DNSClient
from async_dns.core import types, Address

class TestDnsResolver(IsolatedAsyncioTestCase):
    async def test_resolve_mx(self):
        dns = DNSClient()
        resolved = await dns.query('gmail.com', types.MX, Address.parse("8.8.8.8"))
        self.assertEqual(5, len(resolved.an))

    async def test_resolve_mx2(self):
        dns = DNSClient()
        resolved = await dns.query('gmail.com', types.MX, Address.parse("8.8.8.8"))
        self.assertEqual(5, len(resolved.an))

image

Unclear to me why this happens, but I noticed the CallbackProtocol does not implement the methods error_received and connection_lost exposed at the example on python docs. Can this behaviour be related to UDP socket handling, maybe some disposal routines or anything like that?

gera2ld commented 3 years ago

It seems that IsolatedAsyncioTestCase creates a new event loop for each test case.

async_dns shares the same protocol for all clients, so it runs into trouble when the dispatcher which holds the protocol is created with an event loop and the new client is created with another.

https://github.com/gera2ld/async_dns/blob/fe2ff8c74861dcbfb16eb0757f0a5ee6f141486b/async_dns/request/udp.py#L64-L65

In your case, simply adding a tearDown method would help:

from async_dns.request.udp import Dispatcher

# ...
    def tearDown(self):
        Dispatcher.data.clear()

I guess the dispatcher / protocol should not be destroyed after each request internally. So maybe I should add this to async_dns.request.clean so that you can call request.clean explicitly when you need a clean initial state.

leandroltavares commented 3 years ago

Looks like the tearDown works, I agree it would be nice to have this clean up routine in cases like that.

leandroltavares commented 3 years ago

Following on this, when I ran several tests, I face some ResourceWarnings, looks like the socket was not closed, probably the clean up routine should take care of closing it.

image

gera2ld commented 3 years ago

Normally this should not happen because we don't force clear cache this way. Anyway I will close the socket in request.clean.

gera2ld commented 3 years ago

In the latest alpha ==2.0.0a3, you should use request.clean instead:

from async_dns.request import clean

# ...
    def tearDown(self):
        clean()