Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Anyio version v3 breaks AsyncContentsManager for python3.6 #505

Closed
mwakaba2 opened this issue May 3, 2021 · 16 comments · Fixed by #521
Closed

Anyio version v3 breaks AsyncContentsManager for python3.6 #505

mwakaba2 opened this issue May 3, 2021 · 16 comments · Fixed by #521
Assignees
Labels

Comments

@mwakaba2
Copy link
Member

mwakaba2 commented May 3, 2021

Description

AsyncFileContentsManager tests are timing out in a python 3.6 environment due to a AsyncLibraryNotFoundError error.

DEBUG    ServerApp:log.py:54 200 GET /a%40b/api/contents/foo/a.txt?content=1 (127.0.0.1) 2.04ms
ERROR    asyncio:base_events.py:1296 Exception in callback WorkerThread.stop(<Task finishe...> result=None>)
handle: <Handle WorkerThread.stop(<Task finishe...> result=None>)>
Traceback (most recent call last):
  File "/Users/mwaka/opt/anaconda3/envs/jp_test_3.6/lib/python3.6/asyncio/events.py", line 145, in _run
    self._callback(*self._args)
  File "/Users/mwaka/opt/anaconda3/envs/jp_test_3.6/lib/python3.6/site-packages/anyio/_backends/_asyncio.py", line 708, in stop
    _threadpool_workers.get().discard(self)
  File "/Users/mwaka/opt/anaconda3/envs/jp_test_3.6/lib/python3.6/site-packages/anyio/lowlevel.py", line 113, in get
    return self._current_vars[self._name]
  File "/Users/mwaka/opt/anaconda3/envs/jp_test_3.6/lib/python3.6/site-packages/anyio/lowlevel.py", line 97, in _current_vars
    token = current_token()
  File "/Users/mwaka/opt/anaconda3/envs/jp_test_3.6/lib/python3.6/site-packages/anyio/lowlevel.py", line 54, in current_token
    return get_asynclib().current_token()
  File "/Users/mwaka/opt/anaconda3/envs/jp_test_3.6/lib/python3.6/site-packages/anyio/_core/_eventloop.py", line 109, in get_asynclib
    asynclib_name = sniffio.current_async_library()
  File "/Users/mwaka/opt/anaconda3/envs/jp_test_3.6/lib/python3.6/site-packages/sniffio/_impl.py", line 82, in current_async_library
    "unknown async library, or not in async context"
sniffio._impl.AsyncLibraryNotFoundError: unknown async library, or not in async context

Reproduce

$ pytest -vv jupyter_server/tests/services/contents/test_api.py

Expected behavior

The tests should pass in ~10 seconds.

pytest -vv jupyter_server/tests/services/contents/test_api.py  6.81s user 2.50s system 89% cpu 10.358 total

Context

  • Operating System and version: MacOS, 10.15.7 (Catalina)
  • Python version: Python 3.6.13 |Anaconda, Inc.| (default, Feb 23 2021, 12:58:59)
@mwakaba2 mwakaba2 added the bug label May 3, 2021
@mwakaba2 mwakaba2 mentioned this issue May 3, 2021
1 task
@agronholm
Copy link

AnyIO relies on sniffio to detect the current async library. It cannot function without this detection. The only difference between py3.6 and 3.7 that would matter here is the support for context variables. This looks like it happens in the callback where the asyncio thread pool is being shut down. I will try to figure this out.

@mwakaba2
Copy link
Member Author

mwakaba2 commented May 9, 2021

@agronholm I think I found the issue.

For python 3.6, the contextvar current_async_library_cvar in sniffio is not getting set because of this "if condition" (code)

if (3, 7) <= sys.version_info:
  # asyncio has contextvars support, and we're in a task, so
  # we can safely cache the sniffed value
  current_async_library_cvar.set("asyncio")

After updating the "if condition" to the following, the tests are passing now.

if sys.version_info <= (3, 7):

However, I noticed that this "if condition" was removed recently here.

I tested against the latest sniffio commit, and I'm still getting the same error message.
The "if condition" is still needed to prevent the AsyncLibraryNotFoundError error message, so I made a PR: Sniffio #24 to add it back in.

One strange thing I noticed was that the AsyncLibraryNotFoundError error is still getting raised even after the function reaches the return "asyncio" statement. This may be because of tasks running asynchronously..

@agronholm
Copy link

Good to know. So it works if you downgrade sniffio?

@agronholm
Copy link

agronholm commented May 9, 2021

Lookin at the commit you pointed to, it's just an optimization. It would seem strange if something was broken by it. Can you create a minimal reproducing script for this?

@mwakaba2
Copy link
Member Author

mwakaba2 commented May 9, 2021

Downgrading won't help because the if condition isn't checking the version_info correctly. It's checking whether (3, 7) is less than py3.6, but that'll return False.

@agronholm
Copy link

agronholm commented May 9, 2021

It just means it won't cache the result. It's a performance optimization.

@mwakaba2
Copy link
Member Author

mwakaba2 commented May 9, 2021

Sorry I misunderstood. I thought setting the context var was needed for versions less than 3.7, but it's for 3.7 and up.

Can you create a minimal reproducing script for this?

Yeah, I can look into that. Also, do you know why the optimization was removed recently?
python-trio/sniffio@9126c63#diff-acce973c41d5df27520af82067e245d6f961d963ea70e214028ce8889b5dc8e8

@njsmith
Copy link

njsmith commented May 9, 2021

We realized that sniffio should use a thread-local instead of a context-local variable, to better handle some complicated cases involving where different async libraries are calling into each other. The optimization only works for context-locals, so we had to remove it when we moved away from using context-locals.

@mwakaba2
Copy link
Member Author

mwakaba2 commented May 9, 2021

ah I see, thanks for the clarification!

@mwakaba2
Copy link
Member Author

mwakaba2 commented May 9, 2021

@agronholm I was able to create a minimal reproducing script. I included that in a new anyio issue: agronholm/anyio#286

I'd appreciate if you can take a look.

@mwakaba2 mwakaba2 self-assigned this May 11, 2021
@mwakaba2
Copy link
Member Author

Hi @agronholm I noticed the fix has been merged. Do you know when anyio will release the new fix?

@agronholm
Copy link

I have a couple more PRs to go through and then I'll make the v3.1.0 release. ETA 1-3 days.

@mwakaba2
Copy link
Member Author

sounds good! thanks for the update!

@agronholm
Copy link

agronholm commented May 16, 2021

Still working out PR agronholm/anyio#289. After that's dealt with I will make a release.

@agronholm
Copy link

v3.1.0 is out now.

@mwakaba2
Copy link
Member Author

thank you @agronholm ! I'll update the anyio version in jupyter server.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants