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

Add c-api to set callback function on enter and exit of PyContext #119333

Closed
fried opened this issue May 21, 2024 · 1 comment
Closed

Add c-api to set callback function on enter and exit of PyContext #119333

fried opened this issue May 21, 2024 · 1 comment
Labels
topic-C-API type-feature A feature request or enhancement

Comments

@fried
Copy link
Contributor

fried commented May 21, 2024

Feature or enhancement

Proposal:

In python every time a context is switched, two c-api are called.

PyContext_Enter(PyObject *);
PyContext_Exit(PyObject *);

I need a way to know when those api's are called on a context. The purpose is that at Meta we have a similar system to contextvars in C++ called folly::requestcontext. When the python context switches we need to inform C++ to swap its active context. Right now we have been carry forward patches to Asyncio.Task to do this on the _step() since way back in py3.6 which is generally the wrong place to do it.

There is prior art in #91054 which provides watching creation/destruction of Code objects. The proposed solution would mirror that functionality closely.

Has this already been discussed elsewhere?

I have already discussed this feature proposal on Discourse

Links to previous discussion of this feature:

https://discuss.python.org/t/request-can-we-get-a-c-api-hook-into-pycontext-enter-and-pycontext-exit/51730

There was an in person meeting between @gvanrossum, @itamaro and myself where we discussed the plausibility of this feature request.

Linked PRs

@rhansen
Copy link
Contributor

rhansen commented Sep 27, 2024

@fried I'm not sure that context entry and exit are the appropriate semantics for this. Don't you actually want to know when the "current context" switches to a different context?

Why this matters: #99633 is proposing to add the ability to use contextvars.Context as a context manager, which involves temporarily changing the "current context" (but not necessarily entering and exiting contexts) while a generator or coroutine is executing. See #99633 (comment) for some technical details.

rhansen added a commit to rhansen/cpython that referenced this issue Sep 28, 2024
The PyContext struct is not intended to be public, and users of the
API don't need anything more specific than PyObject.  Also see
pythongh-78943.
rhansen added a commit to rhansen/cpython that referenced this issue Sep 28, 2024
The PyContext struct is not intended to be public, and users of the
API don't need anything more specific than PyObject.  Also see
pythongh-78943.
rhansen added a commit to rhansen/cpython that referenced this issue Sep 29, 2024
…back

I believe that the value of a simpler API (and defense against poorly
written callbacks) outweighs the cost of backing up and restoring the
thread's exception state.
rhansen added a commit to rhansen/cpython that referenced this issue Sep 29, 2024
…back

I believe that the value of a simpler API (and defense against poorly
written callbacks) outweighs the cost of backing up and restoring the
thread's exception state.
rhansen added a commit to rhansen/cpython that referenced this issue Sep 29, 2024
  * Add definitions for "context", "current context", and "context
    management protocol".
  * Update related definitions to be consistent with the new
    definitions.
  * Restructure the documentation for the `contextvars.Context` class
    to prepare for adding context manager support, and for consistency
    with the definitions.
  * Use `testcode` and `testoutput` to test the `Context.run` example.
  * Expand the documentation for the `Py_CONTEXT_EVENT_ENTER` and
    `Py_CONTEXT_EVENT_EXIT` events to clarify and to prepare for
    planned changes.
rhansen added a commit to rhansen/cpython that referenced this issue Sep 29, 2024
  * Add definitions for "context", "current context", and "context
    management protocol".
  * Update related definitions to be consistent with the new
    definitions.
  * Restructure the documentation for the `contextvars.Context` class
    to prepare for adding context manager support, and for consistency
    with the definitions.
  * Use `testcode` and `testoutput` to test the `Context.run` example.
  * Expand the documentation for the `Py_CONTEXT_EVENT_ENTER` and
    `Py_CONTEXT_EVENT_EXIT` events to clarify and to prepare for
    planned changes.
rhansen added a commit to rhansen/cpython that referenced this issue Sep 30, 2024
The PyContext struct is not intended to be public, and users of the
API don't need anything more specific than PyObject.  Also see
pythongh-78943.
rhansen added a commit to rhansen/cpython that referenced this issue Sep 30, 2024
The exception is ignored so change the return type from `int` to
`void` to discourage callbacks from raising an exception in the first
place.
rhansen added a commit to rhansen/cpython that referenced this issue Sep 30, 2024
rhansen added a commit to rhansen/cpython that referenced this issue Sep 30, 2024
rhansen added a commit to rhansen/cpython that referenced this issue Sep 30, 2024
rhansen added a commit to rhansen/cpython that referenced this issue Sep 30, 2024
…ched"

Users want to know when the current context switches to a different
context object.  Right now this happens when and only when a context
is entered or exited, so the enter and exit events are synonymous with
"switched".  However, if the changes proposed for pythongh-99633 are
implemented, the current context will also switch for reasons other
than context enter or exit.  Since users actually care about context
switches and not enter or exit, replace the enter and exit events with
a single switched event.

The former exit event was emitted just before exiting the context.
The new switched event is emitted after the context is exited to match
the semantics users expect of an event with a past-tense name.  If
users need the ability to clean up before the switch takes effect,
another event type can be added in the future.  It is not added here
because YAGNI.

I skipped 0 in the enum as a matter of practice.  Skipping 0 makes it
easier to troubleshoot when code forgets to set zeroed memory, and it
aligns with best practices for other tools (e.g.,
https://protobuf.dev/programming-guides/dos-donts/#unspecified-enum).
rhansen added a commit to rhansen/cpython that referenced this issue Sep 30, 2024
rhansen added a commit to rhansen/cpython that referenced this issue Sep 30, 2024
rhansen added a commit to rhansen/cpython that referenced this issue Sep 30, 2024
rhansen added a commit to rhansen/cpython that referenced this issue Sep 30, 2024
rhansen added a commit to rhansen/cpython that referenced this issue Sep 30, 2024
…ched"

Users want to know when the current context switches to a different
context object.  Right now this happens when and only when a context
is entered or exited, so the enter and exit events are synonymous with
"switched".  However, if the changes proposed for pythongh-99633 are
implemented, the current context will also switch for reasons other
than context enter or exit.  Since users actually care about context
switches and not enter or exit, replace the enter and exit events with
a single switched event.

The former exit event was emitted just before exiting the context.
The new switched event is emitted after the context is exited to match
the semantics users expect of an event with a past-tense name.  If
users need the ability to clean up before the switch takes effect,
another event type can be added in the future.  It is not added here
because YAGNI.

I skipped 0 in the enum as a matter of practice.  Skipping 0 makes it
easier to troubleshoot when code forgets to set zeroed memory, and it
aligns with best practices for other tools (e.g.,
https://protobuf.dev/programming-guides/dos-donts/#unspecified-enum).
rhansen added a commit to rhansen/cpython that referenced this issue Sep 30, 2024
…tchCallback

I changed my mind about where to back up and restore the exception.
Instead of backing up and restoring once outside the loop over the
registered callbacks, the exception is backed up and restored each
time a callback is called.

This avoids concerns about what happens if a callback raises an
exception but returns non-negative.

This is also expected to be more efficient, not less, because the
common case is no watchers at all, in which case there is no
backup/restore overhead.  The second most common case is one watcher,
which is no less efficient than before.  The only time this is more
expensive is when there are two or more watchers, in which case the
overhead of the watcher callbacks probably dwarfs the overhead of the
extra backups anyways.
rhansen added a commit to rhansen/cpython that referenced this issue Sep 30, 2024
rhansen added a commit to rhansen/cpython that referenced this issue Sep 30, 2024
rhansen added a commit to rhansen/cpython that referenced this issue Sep 30, 2024
rhansen added a commit to rhansen/cpython that referenced this issue Sep 30, 2024
…ched"

Users want to know when the current context switches to a different
context object.  Right now this happens when and only when a context
is entered or exited, so the enter and exit events are synonymous with
"switched".  However, if the changes proposed for pythongh-99633 are
implemented, the current context will also switch for reasons other
than context enter or exit.  Since users actually care about context
switches and not enter or exit, replace the enter and exit events with
a single switched event.

The former exit event was emitted just before exiting the context.
The new switched event is emitted after the context is exited to match
the semantics users expect of an event with a past-tense name.  If
users need the ability to clean up before the switch takes effect,
another event type can be added in the future.  It is not added here
because YAGNI.

I skipped 0 in the enum as a matter of practice.  Skipping 0 makes it
easier to troubleshoot when code forgets to set zeroed memory, and it
aligns with best practices for other tools (e.g.,
https://protobuf.dev/programming-guides/dos-donts/#unspecified-enum).
rhansen added a commit to rhansen/cpython that referenced this issue Oct 1, 2024
…ched"

Users want to know when the current context switches to a different
context object.  Right now this happens when and only when a context
is entered or exited, so the enter and exit events are synonymous with
"switched".  However, if the changes proposed for pythongh-99633 are
implemented, the current context will also switch for reasons other
than context enter or exit.  Since users actually care about context
switches and not enter or exit, replace the enter and exit events with
a single switched event.

The former exit event was emitted just before exiting the context.
The new switched event is emitted after the context is exited to match
the semantics users expect of an event with a past-tense name.  If
users need the ability to clean up before the switch takes effect,
another event type can be added in the future.  It is not added here
because YAGNI.

I skipped 0 in the enum as a matter of practice.  Skipping 0 makes it
easier to troubleshoot when code forgets to set zeroed memory, and it
aligns with best practices for other tools (e.g.,
https://protobuf.dev/programming-guides/dos-donts/#unspecified-enum).
rhansen added a commit to rhansen/cpython that referenced this issue Oct 1, 2024
rhansen added a commit to rhansen/cpython that referenced this issue Oct 1, 2024
rhansen added a commit to rhansen/cpython that referenced this issue Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-C-API type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

4 participants