inotify leaks

[tkukushkin]

Hello! With the new version of aiodns we've noticed the following warnings:

Failed to create DNS resolver channel with automatic monitoring of resolver configuration changes. This usually means the system ran out of inotify watches. Falling back to socket state callback. Consider increasing the system inotify watch limit: Failed to initialize c-ares channel

Reproducer:

import asyncio, aiodns

async def main():
    while True:
        resolver = aiodns.DNSResolver()
        await resolver.close()

asyncio.run(main())

It seems that closing the resolver doesn't free inotify watches.

In general, the resolver should be created once per application. But we have a lot of legacy code, which creates aiohttp.ClientSession for a single request and then closes it, like this:

async with aiohttp.ClientSession() as session, session.get(...) as response:
   ...

Each time, aiohttp creates a new resolver and then closes it.

[DarioMirovic]

I came here to report this issue, but I can see it is already reported.

I encountered this issue on Fedora 42. Version 3.2.0 is ok, but 3.3.0 to 3.5.0 leak inotify.

[saghul]

Channels are destroyed asynchonously in an auxiliary thread. If you create the fast enough I guess you could run into this problem.

Is the sample code basically a simplification of what you are currently doing? Is it representative of a real world workload?

I can think of a couple of ways to fix things, but let's understand the use case better before jumping the gun.

[tkukushkin]

Some our real world workloads just open aiohttp session for a single request. And some libraries do it too, for example slack-sdk by default creates a new session for every request.

[zYoma]

We encountered a similar issue. We also have a large number of external requests made via:

async with aiohttp.ClientSession() as session, session.get(...) as response:
   ...

At first, this leads to warnings as described in the first post. Later on, the requests start failing with the following error: AresError: Failed to initialize c-ares channel
This issue occurs simultaneously across all services that are running on the same Kubernetes nodes.

[saghul]

@bdraco we might need to disable the event thread while we figure this one out.

[CanD42]

Same issue here with alpine. Is it save to go back to version 3.4.0? I see that resolver.close() was introduced in 3.5.0
BTW: I can also reproduce it on macOS

[saghul]

Yes, you can safely go back.

[DarioMirovic]

@CanD42 @saghul just a note from my experience - I experienced the issue on 3.3.0 too, and on 3.5.0. I'm not sure about 3.4.0, but I reverted to 3.2.0 and then it worked fine.

[saghul]

That's correct, sorry. The event thread was added in b6cce69 which is what triggers the leak in case too many resolvers get created in a short amount of time.

[bdraco]

@bdraco we might need to disable the event thread while we figure this one out.

Few options:

• disable the event thread which will break seeing resolv.conf changes (downstream now relies on this) and windows without selector (probably less of a concern)
• re-look at the event loop shutdown solution in pycares which doesn't have the throughput limit. -- I still have that proposed solution in a branch mostly ready to go. Fix channel destruction race conditions with c-ares queue synchronization and safety delay saghul/pycares#249
• something else?

[bdraco]

I think the key issue here is that c-ares shutdown is never going to be thread-safe. The event thread makes the problem more likely to happen. I think we should revert the event thread even if it breaks downstream.

https://c-ares.org/docs/ares_destroy.html

There is no ability to make this function thread-safe. No additional calls using this channel may be made once this function is called.

Without an api for thread-safe shutdown in c-ares we fundamentally cannot solve this problem in a clean way since we have no way to know what the current internal state os c-ares is.

https://c-ares.org/docs/ares_threadsafety.html leads to some bad assumptions because its thread-safe except for shutdown which is not really thread-safe.

[bdraco]

Even if we revert the event thread here and go to only the socket state callback we still have the problem where we aren't destroying fast enough in pycares. Do we remove the safe shutdown there and document that pycares is not thread-safe because c-ares is not thread safe? If we don't care about pycares being thread-safe that we don't really have a problem since asyncio is single threaded and we don't need to worry about it being thread-safe. Than the only thing we have to worry about is breaking downstream.

[saghul]

The source of the problem is the combination between the evented mode in c-ares (uses more fds) and delayed destruction (since more fds are used for a period of time).

The problem is really that too many resolvers are created for no good reason, but since we cannot change that, maybe we can help.

I've been thinking about this a bit and here is what I think we can do (if time allows I'll give it a try tonight):

• Lower the time spent in the queue, 1s feels like too much, just the context switch alone should give c-ares time to cleanup
• Keep a small (say size 4) free-list of channels, if created with no options

The key part is the free-list. If a channel is created like so channel = pycares.Channel(event_thread=True) without any other options:

• Upon destruction we put it in a SimpleQueue of size 4
• If it fails because it's full, we schedulle it for destruction
• Else, it will sit there
• When a new channel is created with just the event_thread mode set, we can take a channel from the free-list, if available
• Else we create a new one
• Use atexit to destroy the channels in the free-list

This should help with the pathological case of creating too many resolvers for one-shot queries, which is arguably broken behavior.

WDYT?

[bdraco]

I've been stress testing the asyncio version with the 1s delay from saghul/pycares#249, and I'm still able to reproduce crashes after many iterations. This confirms that the current approach with the delay is fundamentally unsafe - it merely reduces the probability of hitting the race condition rather than eliminating it.

The core issue is that c-ares was designed with the assumption that destroy operations cannot be made thread-safe. Even though the event thread is the the c-ares recommended use case, using the event thread violates this assumption, creating race conditions during cleanup that can't be reliably solved with delays, locks, or even lazy cleanup approaches. Even with the 1s delay, we're essentially playing whack-a-mole with timing windows where:

• c-ares is still processing requests or cleaning up while the channel is being destroyed
• File descriptors/inotify watches are leaked because we can't cleanup fast enough when resolvers are created and destroyed rapidly

Lazy cleanup would still have the same fundamental race conditions - just deferred to a different point in time.

Given that c-ares's architecture makes thread-safe destruction impossible with the current design, I believe we need to revert the event thread feature entirely. Its just not designed for this type of churn we see happening in the real world. The risk of resource exhaustion and crashes in production environments outweighs any benefits it provides. The only truly safe approach is to ensure all c-ares operations, especially destruction, happen on a single thread which is the only safe documented way to destroy.