support free-threaded python3.14#1224
Conversation
|
This specific test added in #887 is behaving a bit weird on free-threaded build. |
|
Yeah, this needs a more complete fix. I had something locally that, IIRC, fixes the test that fails here. I'll check that tonight. |
|
Try this: --- a/src/anyio/_backends/_asyncio.py (revision c384f99687c64c59ed8a11c3a0f11a2d57daff71)
+++ b/src/anyio/_backends/_asyncio.py (date 1783968974553)
@@ -987,7 +987,11 @@
workers: set[WorkerThread],
idle_workers: deque[WorkerThread],
):
- super().__init__(name="AnyIO worker thread")
+ kwargs: dict[str, Any] = {}
+ if sys.version_info >= (3, 14):
+ kwargs["context"] = Context()
+
+ super().__init__(name="AnyIO worker thread", **kwargs)
self.root_task = root_task
self.workers = workers
self.idle_workers = idle_workers |
|
The same structure should be used for the other part of the fix too. |
|
ah yes, that will work. I might have been confused and looking at the wrong side of the problem. |
|
did you also notice that running the test multiple times sometimes fails? |
|
No, this one has been pretty consistent for me. Granted, I haven't spent that much time with it. |
|
I will investigate. When I run it multiple times, sometimes the first assertion errors out. |
Co-authored-by: Alex Grönholm <alex.gronholm@nextday.fi>
Okay, this got a little hairy; apparently running the test in question in isolation ( |
|
Failure on trio is fixed python-trio/trio#3473 eager task factory backend(asyncio + eager) is still flaky. |
So is |
Here it crashed at 8th run. I have a little bash script for the reason of the flakiness of this test. #!/usr/bin/env bash
RUNS=15
for ((i=1; i<=RUNS; i++)); do
echo "Run $i of $RUNS"
pytest tests/test_to_thread.py -v
# Stop immediately if the test fails
if [ $? -ne 0 ]; then
echo "Test failed on run $i"
exit 1
fi
done
echo "All $RUNS runs passed!" |
sometimes it fail on uvloop backend too. but its more prevalent on the eager task factory. I haven't figured out whats going on yet. I am not sure why that object is bleeding out. |
|
Line 384 in c384f99 I could also add a second |
|
Sounds like it's time-sensitive then. I'd really like to understand the cause first before patching just the symptoms. |
|
If it is, wouldn't my assumption of adding a check point fix that? Letting worker thread have time to clear these objects out? It doesn't seem to work for me. this object bleeding out is also something of cpython implementation detail yet I get tempted to think that the problem might not be comming from worker thread implementation being threadsafe or not? I totally agree on understanding the cause before this patch. |
|
The red warning on usage of gc.get_referrers in the docs also make me suspicious but not yet able to prove my suspicions! |
I would say though, checkpointing before we do assertion in this test does quite significantly reduce the frequency. I think its buying a little bit of time for the worker thread to remove references? anyio/src/anyio/_backends/_asyncio.py Line 1047 in c384f99 I think the cause is presumably a race happening in removing the references. given that |
for the context here, the issue on trio was also about context inheritance. |
|
We could add a checkpoint, index 93c956b..efee878 100644
--- a/tests/test_to_thread.py
+++ b/tests/test_to_thread.py
@@ -24,7 +24,7 @@ from anyio import (
)
from anyio._core._eventloop import current_async_library
from anyio.from_thread import BlockingPortalProvider
-
+from anyio.lowlevel import checkpoint
from .conftest import asyncio_params, no_other_refs
@@ -382,6 +382,11 @@ async def test_run_sync_worker_cyclic_references() -> None:
await to_thread.run_sync(foo, arg)
cvar.set(Foo())
gc.collect()
+ await checkpoint()
assert gc.get_referrers(contextval) == no_other_refs()
assert gc.get_referrers(foo) == no_other_refs()and probably add a clarifying comment and link this PR about it. But, I would like to know your thoughts on my premise. |
agronholm
left a comment
There was a problem hiding this comment.
I'd like two changes here:
- Add a tox configuration for 3.14t (one-line modification)
- Add a free-threading classifier to
pyproject.toml:Programming Language :: Python :: Free Threading :: 2 - BetaORProgramming Language :: Python :: Free Threading :: 3 - Stabledepending on how you feel about this
|
There's a strange number of jobs failing due to a timeout error around the file stream tests. I don't see how these changes could have caused that, especially on non-free-threading builds. |
|
yeah, I was just looking at it too! |
|
locally for me. They are hanging actually. |
|
The only change I made was to pass |
|
And given that only 3.14+ jobs timeout, this tracks. |
we can go with in the porting guide for this classifier they state: Free threaded usage is supported, but documentation of constraints and limitations may be incomplete. on the Stable classifier, I am not entirely sure on the clearly documented thread safety issues as they put it Supported for production use, multithreaded use is tested, and thread safety issues are clearly documented. |
|
I assume it should behave the same too, long as the flag for inheriting context is set to false. |
|
Perhaps I misunderstood what the docs say because I don't understand how you expected to get the same result by setting context to False. |
| gc.collect() | ||
| # This checkpoint only exist to give the worker thread a moment to finish releasing its own | ||
| # references before we check. on free threaded python builds, this can | ||
| # otherwise race and intermittently show a leftover reference. |
There was a problem hiding this comment.
Actually, It was not ai generated😅. I thought it might a bit strange for someone to stumble on a checkpoint there.
Okay, I had a quick spin through the cpython Thread implementation and this is not equivalent to an explicit empty context, with this context setup is not happening at all. @agronholm I think we need to go back to an explicit empty context? |
Help me out here. Here's what the docs say:
My interpretation is that |
Ah, I see where I went wrong. The parameter doesn't take any values other than |
|
So by all means revert my commits, and we should be good then. |
False being whatever the flag |
exactly! and context creation is controlled by this flag. |
NOTE Erasing or replacing the contents of this template will result in your pull
request being summarily closed without consideration!
Changes
Fixes #1220
Add
Python 3.14tto the test matrix so the test suite is also exercised on the free-threaded build. This also fixes the referenced issue as a side effect.I am leaving 3.15t out of the matrix for now, since the current released test dependencies do not install successfully on that interpreter yet. Specifically, the failure happens while building cryptography against CPython 3.15t.
Checklist
If this is a user-facing code change, like a bugfix or a new feature, please ensure that
you've fulfilled the following conditions (where applicable):
tests/) which would fail without your patchdocs/), in case of behavior changes or newfeatures
docs/versionhistory.rst).If this is a trivial change, like a typo fix or a code reformatting, then you can ignore
these instructions.
Updating the changelog
If there are no entries after the last release, use
**UNRELEASED**as the version.If, say, your patch fixes issue #123, the entry should look like this:
If there's no issue linked, just link to your pull request instead by updating the
changelog after you've created the PR.