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-3108: Avoid materializing f_locals for KI protection #3110

Open
wants to merge 38 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
5d51c56
gh-3108: avoid materializing f_locals by using weakrefs to code objec…
graingert Oct 13, 2024
ea42ad0
enable ki protection on async_generator objects
graingert Oct 13, 2024
c755593
avoid adding an extra coroutine wrapper to Task coros
graingert Oct 13, 2024
6bf63f2
fix returning the wrong object in (enable|disable)_ki_protection
graingert Oct 13, 2024
65aa8f4
remove KIProtectionSignature from _check_type_completeness.json
graingert Oct 13, 2024
748f876
fix refcycles
graingert Oct 13, 2024
fccf4ff
add newsfragment
graingert Oct 13, 2024
56696a2
fix mypy
graingert Oct 13, 2024
a7272d8
now that the type annotation for enable_ki_protection is fixed, we ne…
graingert Oct 13, 2024
3ea6617
pre-commit
graingert Oct 13, 2024
0ed35a7
add test for ki protection leaking accross local functions
graingert Oct 14, 2024
ec30d7b
add fix for ki protection leaking accross local functions
graingert Oct 14, 2024
6998a5c
do slots properly
graingert Oct 14, 2024
1f0fc6f
python 3.8 support
graingert Oct 14, 2024
480aa01
test reading currently_ki_protected doesn't freeze locals
graingert Oct 14, 2024
b7a1132
cover some tricky bits of ki.py
graingert Oct 14, 2024
05b1fa1
cover a potentially impossible scenario
graingert Oct 14, 2024
f8dc61d
eek out some last coverage of the eeking out coverage tests
graingert Oct 14, 2024
9e6bddb
even more partial coverage
graingert Oct 14, 2024
d5e539f
Update src/trio/_core/_ki.py
graingert Oct 14, 2024
408d1ae
cleaner _IdRef.__eq__
graingert Oct 14, 2024
5ba9bd8
if the current_task().coro.cr_frame is in the stack ki_protection_ena…
graingert Oct 15, 2024
49bd5c3
Update newsfragments/3108.bugfix.rst
graingert Oct 16, 2024
76b0c79
avoid copying code objects for ki protected function
graingert Oct 18, 2024
fa6cd8e
Update src/trio/_core/_ki.py
graingert Oct 18, 2024
be38f4c
Update src/trio/_core/_ki.py
graingert Oct 18, 2024
8136a82
Merge branch 'main' of github.com:python-trio/trio into ki-with-code-…
graingert Oct 23, 2024
b21bde6
remove workaround for 3.8
graingert Oct 23, 2024
0b2dd1a
Add docs and update news
graingert Oct 23, 2024
12329c7
wrap ki protection locals demos in async def so they work
graingert Oct 23, 2024
97a9eb2
add newsfragment for 2670
graingert Oct 23, 2024
022bd3d
Apply suggestions from code review
graingert Oct 24, 2024
4fe4d36
use peo614
graingert Oct 24, 2024
0c80de3
add tests for passing on inspect flags
graingert Oct 25, 2024
9bbaef0
Merge branch 'ki-with-code-objects' of github.com:graingert/trio into…
graingert Oct 25, 2024
1e10e62
'return; yield' isn't considered covered
graingert Oct 25, 2024
0cd1feb
Update newsfragments/3108.bugfix.rst
graingert Oct 26, 2024
6e97f9a
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 26, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions newsfragments/3108.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
avoid materializing frame.f_locals in KI protection code, this causes locals to persist in the frame.f_locals snapshot even if they are deleted on 3.12 and below
graingert marked this conversation as resolved.
Show resolved Hide resolved
7 changes: 3 additions & 4 deletions src/trio/_core/_generated_instrumentation.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,9 @@
# *************************************************************
from __future__ import annotations

import sys
from typing import TYPE_CHECKING

from ._ki import LOCALS_KEY_KI_PROTECTION_ENABLED
from ._ki import enable_ki_protection
from ._run import GLOBAL_RUN_CONTEXT

if TYPE_CHECKING:
Expand All @@ -15,6 +14,7 @@
__all__ = ["add_instrument", "remove_instrument"]


@enable_ki_protection
def add_instrument(instrument: Instrument) -> None:
"""Start instrumenting the current run loop with the given instrument.
Expand All @@ -24,13 +24,13 @@ def add_instrument(instrument: Instrument) -> None:
If ``instrument`` is already active, does nothing.
"""
sys._getframe().f_locals[LOCALS_KEY_KI_PROTECTION_ENABLED] = True
try:
return GLOBAL_RUN_CONTEXT.runner.instruments.add_instrument(instrument)
except AttributeError:
raise RuntimeError("must be called from async context") from None


@enable_ki_protection
def remove_instrument(instrument: Instrument) -> None:
"""Stop instrumenting the current run loop with the given instrument.
Expand All @@ -44,7 +44,6 @@ def remove_instrument(instrument: Instrument) -> None:
deactivated.
"""
sys._getframe().f_locals[LOCALS_KEY_KI_PROTECTION_ENABLED] = True
try:
return GLOBAL_RUN_CONTEXT.runner.instruments.remove_instrument(instrument)
except AttributeError:
Expand Down
8 changes: 4 additions & 4 deletions src/trio/_core/_generated_io_epoll.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import sys
from typing import TYPE_CHECKING

from ._ki import LOCALS_KEY_KI_PROTECTION_ENABLED
from ._ki import enable_ki_protection
from ._run import GLOBAL_RUN_CONTEXT

if TYPE_CHECKING:
Expand All @@ -18,6 +18,7 @@
__all__ = ["notify_closing", "wait_readable", "wait_writable"]


@enable_ki_protection
async def wait_readable(fd: int | _HasFileNo) -> None:
"""Block until the kernel reports that the given object is readable.
Expand All @@ -40,13 +41,13 @@ async def wait_readable(fd: int | _HasFileNo) -> None:
if another task calls :func:`notify_closing` while this
function is still working.
"""
sys._getframe().f_locals[LOCALS_KEY_KI_PROTECTION_ENABLED] = True
try:
return await GLOBAL_RUN_CONTEXT.runner.io_manager.wait_readable(fd)
except AttributeError:
raise RuntimeError("must be called from async context") from None


@enable_ki_protection
async def wait_writable(fd: int | _HasFileNo) -> None:
"""Block until the kernel reports that the given object is writable.
Expand All @@ -59,13 +60,13 @@ async def wait_writable(fd: int | _HasFileNo) -> None:
if another task calls :func:`notify_closing` while this
function is still working.
"""
sys._getframe().f_locals[LOCALS_KEY_KI_PROTECTION_ENABLED] = True
try:
return await GLOBAL_RUN_CONTEXT.runner.io_manager.wait_writable(fd)
except AttributeError:
raise RuntimeError("must be called from async context") from None


@enable_ki_protection
def notify_closing(fd: int | _HasFileNo) -> None:
"""Notify waiters of the given object that it will be closed.
Expand All @@ -91,7 +92,6 @@ def notify_closing(fd: int | _HasFileNo) -> None:
step, so other tasks won't be able to tell what order they happened
in anyway.
"""
sys._getframe().f_locals[LOCALS_KEY_KI_PROTECTION_ENABLED] = True
try:
return GLOBAL_RUN_CONTEXT.runner.io_manager.notify_closing(fd)
except AttributeError:
Expand Down
14 changes: 7 additions & 7 deletions src/trio/_core/_generated_io_kqueue.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import sys
from typing import TYPE_CHECKING, Callable, ContextManager

from ._ki import LOCALS_KEY_KI_PROTECTION_ENABLED
from ._ki import enable_ki_protection
from ._run import GLOBAL_RUN_CONTEXT

if TYPE_CHECKING:
Expand All @@ -29,18 +29,19 @@
]


@enable_ki_protection
def current_kqueue() -> select.kqueue:
"""TODO: these are implemented, but are currently more of a sketch than
anything real. See `#26
<https://github.com/python-trio/trio/issues/26>`__.
"""
sys._getframe().f_locals[LOCALS_KEY_KI_PROTECTION_ENABLED] = True
try:
return GLOBAL_RUN_CONTEXT.runner.io_manager.current_kqueue()
except AttributeError:
raise RuntimeError("must be called from async context") from None


@enable_ki_protection
def monitor_kevent(
ident: int,
filter: int,
Expand All @@ -49,13 +50,13 @@ def monitor_kevent(
anything real. See `#26
<https://github.com/python-trio/trio/issues/26>`__.
"""
sys._getframe().f_locals[LOCALS_KEY_KI_PROTECTION_ENABLED] = True
try:
return GLOBAL_RUN_CONTEXT.runner.io_manager.monitor_kevent(ident, filter)
except AttributeError:
raise RuntimeError("must be called from async context") from None


@enable_ki_protection
async def wait_kevent(
ident: int,
filter: int,
Expand All @@ -65,7 +66,6 @@ async def wait_kevent(
anything real. See `#26
<https://github.com/python-trio/trio/issues/26>`__.
"""
sys._getframe().f_locals[LOCALS_KEY_KI_PROTECTION_ENABLED] = True
try:
return await GLOBAL_RUN_CONTEXT.runner.io_manager.wait_kevent(
ident,
Expand All @@ -76,6 +76,7 @@ async def wait_kevent(
raise RuntimeError("must be called from async context") from None


@enable_ki_protection
async def wait_readable(fd: int | _HasFileNo) -> None:
"""Block until the kernel reports that the given object is readable.

Expand All @@ -98,13 +99,13 @@ async def wait_readable(fd: int | _HasFileNo) -> None:
if another task calls :func:`notify_closing` while this
function is still working.
"""
sys._getframe().f_locals[LOCALS_KEY_KI_PROTECTION_ENABLED] = True
try:
return await GLOBAL_RUN_CONTEXT.runner.io_manager.wait_readable(fd)
except AttributeError:
raise RuntimeError("must be called from async context") from None


@enable_ki_protection
async def wait_writable(fd: int | _HasFileNo) -> None:
"""Block until the kernel reports that the given object is writable.

Expand All @@ -117,13 +118,13 @@ async def wait_writable(fd: int | _HasFileNo) -> None:
if another task calls :func:`notify_closing` while this
function is still working.
"""
sys._getframe().f_locals[LOCALS_KEY_KI_PROTECTION_ENABLED] = True
try:
return await GLOBAL_RUN_CONTEXT.runner.io_manager.wait_writable(fd)
except AttributeError:
raise RuntimeError("must be called from async context") from None


@enable_ki_protection
def notify_closing(fd: int | _HasFileNo) -> None:
"""Notify waiters of the given object that it will be closed.

Expand All @@ -149,7 +150,6 @@ def notify_closing(fd: int | _HasFileNo) -> None:
step, so other tasks won't be able to tell what order they happened
in anyway.
"""
sys._getframe().f_locals[LOCALS_KEY_KI_PROTECTION_ENABLED] = True
try:
return GLOBAL_RUN_CONTEXT.runner.io_manager.notify_closing(fd)
except AttributeError:
Expand Down
20 changes: 10 additions & 10 deletions src/trio/_core/_generated_io_windows.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import sys
from typing import TYPE_CHECKING, ContextManager

from ._ki import LOCALS_KEY_KI_PROTECTION_ENABLED
from ._ki import enable_ki_protection
from ._run import GLOBAL_RUN_CONTEXT

if TYPE_CHECKING:
Expand All @@ -32,6 +32,7 @@
]


@enable_ki_protection
async def wait_readable(sock: _HasFileNo | int) -> None:
"""Block until the kernel reports that the given object is readable.

Expand All @@ -54,13 +55,13 @@ async def wait_readable(sock: _HasFileNo | int) -> None:
if another task calls :func:`notify_closing` while this
function is still working.
"""
sys._getframe().f_locals[LOCALS_KEY_KI_PROTECTION_ENABLED] = True
try:
return await GLOBAL_RUN_CONTEXT.runner.io_manager.wait_readable(sock)
except AttributeError:
raise RuntimeError("must be called from async context") from None


@enable_ki_protection
async def wait_writable(sock: _HasFileNo | int) -> None:
"""Block until the kernel reports that the given object is writable.

Expand All @@ -73,13 +74,13 @@ async def wait_writable(sock: _HasFileNo | int) -> None:
if another task calls :func:`notify_closing` while this
function is still working.
"""
sys._getframe().f_locals[LOCALS_KEY_KI_PROTECTION_ENABLED] = True
try:
return await GLOBAL_RUN_CONTEXT.runner.io_manager.wait_writable(sock)
except AttributeError:
raise RuntimeError("must be called from async context") from None


@enable_ki_protection
def notify_closing(handle: Handle | int | _HasFileNo) -> None:
"""Notify waiters of the given object that it will be closed.

Expand All @@ -105,33 +106,32 @@ def notify_closing(handle: Handle | int | _HasFileNo) -> None:
step, so other tasks won't be able to tell what order they happened
in anyway.
"""
sys._getframe().f_locals[LOCALS_KEY_KI_PROTECTION_ENABLED] = True
try:
return GLOBAL_RUN_CONTEXT.runner.io_manager.notify_closing(handle)
except AttributeError:
raise RuntimeError("must be called from async context") from None


@enable_ki_protection
def register_with_iocp(handle: int | CData) -> None:
"""TODO: these are implemented, but are currently more of a sketch than
anything real. See `#26
<https://github.com/python-trio/trio/issues/26>`__ and `#52
<https://github.com/python-trio/trio/issues/52>`__.
"""
sys._getframe().f_locals[LOCALS_KEY_KI_PROTECTION_ENABLED] = True
try:
return GLOBAL_RUN_CONTEXT.runner.io_manager.register_with_iocp(handle)
except AttributeError:
raise RuntimeError("must be called from async context") from None


@enable_ki_protection
async def wait_overlapped(handle_: int | CData, lpOverlapped: CData | int) -> object:
"""TODO: these are implemented, but are currently more of a sketch than
anything real. See `#26
<https://github.com/python-trio/trio/issues/26>`__ and `#52
<https://github.com/python-trio/trio/issues/52>`__.
"""
sys._getframe().f_locals[LOCALS_KEY_KI_PROTECTION_ENABLED] = True
try:
return await GLOBAL_RUN_CONTEXT.runner.io_manager.wait_overlapped(
handle_,
Expand All @@ -141,6 +141,7 @@ async def wait_overlapped(handle_: int | CData, lpOverlapped: CData | int) -> ob
raise RuntimeError("must be called from async context") from None


@enable_ki_protection
async def write_overlapped(
handle: int | CData,
data: Buffer,
Expand All @@ -151,7 +152,6 @@ async def write_overlapped(
<https://github.com/python-trio/trio/issues/26>`__ and `#52
<https://github.com/python-trio/trio/issues/52>`__.
"""
sys._getframe().f_locals[LOCALS_KEY_KI_PROTECTION_ENABLED] = True
try:
return await GLOBAL_RUN_CONTEXT.runner.io_manager.write_overlapped(
handle,
Expand All @@ -162,6 +162,7 @@ async def write_overlapped(
raise RuntimeError("must be called from async context") from None


@enable_ki_protection
async def readinto_overlapped(
handle: int | CData,
buffer: Buffer,
Expand All @@ -172,7 +173,6 @@ async def readinto_overlapped(
<https://github.com/python-trio/trio/issues/26>`__ and `#52
<https://github.com/python-trio/trio/issues/52>`__.
"""
sys._getframe().f_locals[LOCALS_KEY_KI_PROTECTION_ENABLED] = True
try:
return await GLOBAL_RUN_CONTEXT.runner.io_manager.readinto_overlapped(
handle,
Expand All @@ -183,26 +183,26 @@ async def readinto_overlapped(
raise RuntimeError("must be called from async context") from None


@enable_ki_protection
def current_iocp() -> int:
"""TODO: these are implemented, but are currently more of a sketch than
anything real. See `#26
<https://github.com/python-trio/trio/issues/26>`__ and `#52
<https://github.com/python-trio/trio/issues/52>`__.
"""
sys._getframe().f_locals[LOCALS_KEY_KI_PROTECTION_ENABLED] = True
try:
return GLOBAL_RUN_CONTEXT.runner.io_manager.current_iocp()
except AttributeError:
raise RuntimeError("must be called from async context") from None


@enable_ki_protection
def monitor_completion_key() -> ContextManager[tuple[int, UnboundedQueue[object]]]:
"""TODO: these are implemented, but are currently more of a sketch than
anything real. See `#26
<https://github.com/python-trio/trio/issues/26>`__ and `#52
<https://github.com/python-trio/trio/issues/52>`__.
"""
sys._getframe().f_locals[LOCALS_KEY_KI_PROTECTION_ENABLED] = True
try:
return GLOBAL_RUN_CONTEXT.runner.io_manager.monitor_completion_key()
except AttributeError:
Expand Down
Loading
Loading