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

gh-111354: simplify detection of RESUME after YIELD_VALUE at except-depth 1 #111368

Closed
wants to merge 23 commits into from

Conversation

iritkatriel
Copy link
Member

@iritkatriel iritkatriel commented Oct 26, 2023

gen_close needs to know whether a RESUME follows a YIELD_VALUE which is at except-depth 1, for an important optimization. Currently the compiler stores this information in the oparg of YIELD_VALUE, which requires gen_close to peek into the bytecode (which is unsafe in the debugger). This PR puts this information in the oparg of the RESUME, and removes the oparg of YIELD_VALUE. The result is simpler and safer code in gen_close.

It also removes an anomaly in the bytecode definitions, where we needed to add a meaningless assertion to make the code generator understand that YIELD_VALUE has an oparg even though it doesn't use it.


📚 Documentation preview 📚: https://cpython-previews--111368.org.readthedocs.build/

@brettcannon brettcannon removed their request for review October 26, 2023 17:51
iritkatriel and others added 17 commits October 26, 2023 18:56
…thon#110884)

- `ThreadedChildWatcher.close()` is now *officially* a no-op; `_join_threads()` never did anything.
- Threads created by that class are now named `asyncio-waitpid-NNN`.
- `test.test_asyncio.utils.TestCase.close_loop()` now waits for the child watcher's threads, but not forever; if a thread hangs, it raises `RuntimeError`.
* Include Python version in cache.config key, after Python setup

* Remove EOL 3.7 from branch triggers
…GH-111336)

* Try to fix asyncio.Server.wait_closed() again

I identified the condition that `wait_closed()` is intended
to wait for: the server is closed *and* there are no more
active connections.

When this condition first becomes true, `_wakeup()` is called
(either from `close()` or from `_detach()`) and it sets `_waiters`
to `None`. So we just check for `self._waiters is None`; if it's
not `None`, we know we have to wait, and do so.

A problem was that the new test introduced in 3.12 explicitly
tested that `wait_closed()` returns immediately when the server
is *not* closed but there are currently no active connections.
This was a mistake (probably a misunderstanding of the intended
semantics). I've fixed the test, and added a separate test that
checks exactly for this scenario.

I also fixed an oddity where in `_wakeup()` the result of the
waiter was set to the waiter itself. This result is not used
anywhere and I changed this to `None`, to avoid a GC cycle.

* Update Lib/asyncio/base_events.py

---------

Co-authored-by: Carol Willing <carolcode@willingconsulting.com>
@cpython-cla-bot
Copy link

The following commit authors need to sign the Contributor License Agreement:

Click the button to sign:
CLA not signed

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

Successfully merging this pull request may close these issues.

None yet