Connection leak if asyncio TimeoutError happens

[Peter200lx]

This is another leak as mentioned in #66
https://github.com/facebookexperimental/doh-proxy/blob/999a2ae9825919aa4196a917dafa75fb20ec80d6/dohproxy/server_protocol.py#L53-L57
https://github.com/facebookexperimental/doh-proxy/blob/999a2ae9825919aa4196a917dafa75fb20ec80d6/dohproxy/server_protocol.py#L62-L66
Both create_datagram_endpoint and create_connection return a tuple of transport, protocol. You need to capture the transport reference, so that when you hit an asyncio.TimeoutError you can properly run transport.close(). If you don't do this and are sending lots of queries, eventually you'll hit your ulimit -n maximum open files limit from your system. This can be solved by passing the transport object into _try_query and then closing it in
https://github.com/facebookexperimental/doh-proxy/blob/999a2ae9825919aa4196a917dafa75fb20ec80d6/dohproxy/server_protocol.py#L73-L75

[chantra]

Thanks @Peter200lx

Seems you have it all sorted out. Any chances you submit a PR with the fix?

[Peter200lx]

Thanks @Peter200lx

Seems you have it all sorted out. Any chances you submit a PR with the fix?

I opened #77 but then closed it as there are test errors I don't want to dig into and I'd rather not sign legal stuff today. Please don't feel any need to use or avoid using my implementation of this, it is a straight-forward change based on asyncio protocol/transport behaviors.

[chantra]

I opened #77 but then closed it as there are test errors I don't want to dig into and I'd rather not sign legal stuff today. Please don't feel any need to use or avoid using my implementation of this, it is a straight-forward change based on asyncio protocol/transport behaviors.

Thanks. I will look into the errors. As to your solution, I think the route I will take is to store the transport as an attribute of DNSClient so I don't need to inject it all along. there is probably better ways to handle this has part of task cancelation, but it may require more code shifted around.

[chantra]

@Peter200lx would you be able to test #79 and report back if it works for you?

[chantra]

Thanks @Peter200lx for the report and pointing out that storing transport within DNSClient would not cut it for all cases!