fix(client/async): replace asyncify with asyncio.to_thread (#1827)

[jeongsu-an]

• I understand that this repository is auto-generated and my pull request may not be merged

Changes being requested

This Pull Request addresses an issue where AnyIO worker threads do not terminate properly when using asyncify(get_platform) in an asynchronous context. This can cause tests to hang, especially when using pytest, due to non-daemon worker threads remaining alive.

Changes Made:

• Replaced asyncify(get_platform) with asyncio.to_thread(get_platform) in the _request method. This ensures that worker threads terminate correctly after completing their tasks, preventing the hanging issue in asynchronous environments.
• Updated the pythonVersion setting in pyright from 3.7 to 3.9.18 to match the current Python version and to support asyncio.to_thread, which is available from Python 3.9 onwards. This change allows linters to recognize asyncio.to_thread and pass linting checks.

Additional context & links

#1827

[RobertCraigie]

thanks for the PR, can we instead close the worker threads after they're done? It also sounds like this could be an underlying issue with anyio or just that we're using it incorrectly... do you have any insight there?

[jeongsu-an]

Thank you for reviewing my PR.

thanks for the PR, can we instead close the worker threads after they're done?

After investigating, it appears that we cannot close the worker threads after they're done when using AnyIO.

AnyIO does not provide an API to close or terminate these worker threads. According to the AnyIO documentation:

"Note, however, that the thread will still continue running – only its outcome will be ignored."

Source: AnyIO Documentation on Threads

It also sounds like this could be an underlying issue with anyio or just that we're using it incorrectly... do you have any insight there?

The issue occurs because AnyIO's worker threads are non-daemon threads. When AnyIO creates a worker thread, it does not set the daemon attribute, so it inherits the daemon status from the current thread, which is the main thread. Since the main thread is non-daemon (daemon=False), the worker threads are also non-daemon.
anyio/src/anyio/_backends/_asyncio.py
cpython/Lib/threading.py

Non-daemon threads prevent the program from exiting until they have completed. In AnyIO, these worker threads remain alive even after their tasks are done, causing the program to hang, especially in testing environments like pytest.

To resolve this, I replaced asyncify(get_platform) with asyncio.to_thread(get_platform). The asyncio.to_thread function creates daemon threads, which terminate when the main program exits, preventing the hanging issue.

Regarding Deleting asyncify:

I've confirmed that the asyncify function is only used in this part of the code. If it's acceptable, I can remove the related utility function and update the commit accordingly. Please let me know if this is okay.

[afbarbaro]

Any update on this? It's caused issues on a github actions we're using and this just makes it hang.

[spokeydokeys]

I have been suffering from the same bug as @jeongsu-an when using the openai python library in github workflows. Specifically, the process stalls at the end of the script I'm running and if I print out threads there is a leftover AnyIO thread that is not closing.

I tested the behavior in github workflows using @jeongsu-an's PR branch and it fixes the issues I was having and does not introduce any other issues.

[RobertCraigie]

would anyone be able to share a reproduction for this issue? I can't reproduce it with this script running both standard python and pytest

import asyncio
import anyio
import anyio.to_thread

from openai._base_client import get_platform

async def test_main() -> None:
    result = await anyio.to_thread.run_sync(get_platform)
    print(result)

asyncio.run(test_main())

[spokeydokeys]

Include nest_asyncio.apply() and your code will trigger the issue.

import asyncio
import anyio
import anyio.to_thread
import nest_asyncio

from openai._base_client import get_platform

async def test_main() -> None:
    result = await anyio.to_thread.run_sync(get_platform)
    print(result)

nest_asyncio.apply()
asyncio.run(test_main())

[RobertCraigie]

Okay thanks @spokeydokeys, I can confirm that reproduces the issue.

[RobertCraigie]

We can't switch to just using asyncio.to_thread() itself as that was only added in Python 3.9 but we support Python 3.8+

Thankfully it looks like it mostly just delegates to loop.run_in_executor() under the hood which is available in older versions.

Can you instead change the asyncify function to use asyncio.to_thread() on 3.9 and fallback to a copied version of the source? or just always use the copied version? I don't feel strongly either way.

You can remove the other cancellable and limiter args from asyncify as I don't think they make sense anymore.

[spokeydokeys]

@RobertCraigie, since I don't have access to @jeongsu-an 's repository, I forked the repo and put up a PR for the changes you requested in #1853. Happy to iterate there if you have suggestions or changes you'd like.

[RobertCraigie]

closing in favour of #1853