bpo-17799: Explain real behaviour of sys.settrace and sys.setprofile #4056

pablogsal · 2017-10-20T09:04:10Z

The docs for sys.settrace mention that:

event is a string: 'call', 'line', 'return', 'exception', 'c_call', 'c_return', or 'c_exception'

But in the code for ceval.c the only point where call_trace is invoked with PyTrace_C_CALL or PyTrace_C_RETURN is under the C_TRACE macro. In this macro this line prevents any function set up using sys.settrace to call call_trace with the mentioned arguments:

if (tstate->use_tracing && tstate->c_profilefunc)

Notice that from the code of PyEval_SetTrace and PyEval_SetProfile, only the later sets tstate->c_profilefunc and therefore only functions installed using sys.setprofile will recieve a c_call for the event:

void
PyEval_SetProfile(Py_tracefunc func, PyObject *arg)
{
     ...
    tstate->c_profilefunc = func;
    tstate->c_profileobj = arg;
    tstate->use_tracing = (func != NULL) || (tstate->c_tracefunc != NULL);
}

void
PyEval_SetTrace(Py_tracefunc func, PyObject *arg)
{
     ...
    tstate->c_tracefunc = func;
    tstate->c_traceobj = arg;
    tstate->use_tracing = ((func != NULL) || (tstate->c_profilefunc != NULL));
}

This PR updates the documentation clarifying the real behaviour of both functions.

https://bugs.python.org/issue17799

zhangyangyu

The C API part https://docs.python.org/3.7/c-api/init.html?highlight=pyeval_settrace#profiling-and-tracing also needs editing.

bedevere-bot · 2018-01-22T11:32:12Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

pablogsal · 2018-01-22T21:12:47Z

I have made the requested changes; please review again

bedevere-bot · 2018-01-22T21:12:49Z

Thanks for making the requested changes!

@zhangyangyu: please review the changes made to this pull request.

csabella · 2018-01-22T21:41:56Z

Doc/c-api/init.rst

   | :const:`PyTrace_C_RETURN`    | Function object being called.        |
   +------------------------------+--------------------------------------+

+   Any trace function registered using :c:func:`PyEval_SetTrace` will not recieve


/s/recieve/receive

Thanks! Corrected in 18b88ec

zhangyangyu · 2018-01-23T15:54:37Z

Doc/c-api/init.rst

+   Any trace function registered using :c:func:`PyEval_SetTrace` will not receive
+   :const:`PyTrace_C_CALL`, :const:`PyTrace_C_EXCEPTION`, :const:`PyTrace_C_RETURN`
+   or :const:`PyTrace_LINE` as a value for the *what* parameter.
+


Why line event is also here?

Sorry, I misread that part.

zhangyangyu · 2018-01-23T15:55:25Z

Doc/c-api/init.rst

   Set the tracing function to *func*.  This is similar to
   :c:func:`PyEval_SetProfile`, except the tracing function does receive line-number
-   events.
+   events or any event related to C function objects being called.


It is does receive, so I think it is wrong here. Maybe just declare it does not receive PyTrace_C_CALL/RETURN/EXCEPTION here, so don't have to write the previous added block.

zhangyangyu · 2018-01-23T15:57:10Z

Doc/library/sys.rst


+   Profile functions should have three arguments: *frame*, *event*, and
+   *arg*. *frame* is the current stack frame.  *event* is a string: ``'call'``,
+   ``'line'``, ``'return'``, ``'exception'``, ``'c_call'``, ``'c_return'``, or


line will be received here?

zhangyangyu · 2018-01-23T16:05:13Z

Doc/library/sys.rst

+
+   ``'call'``
+      A function is called (or some other code block entered).  The
+      global trace function is called; *arg* is ``None``; the return value


Although the previous doc says profile function is called similiarly to trace function, but from the implementation, there is no concept like global trace function or local trace function. And at least we call it profile function here not trace function.

pablogsal · 2018-01-23T16:13:11Z

@zhangyangyu Thank you very much for the comments. I have addressed your feedback in a9f381f and 3887c6c.

zhangyangyu · 2018-01-23T16:18:36Z

Doc/library/sys.rst

+      profile function is called; *arg* is ``None``.
+
+   ``'return'``
+      A function (or other code block) is about to return.  The local trace


Also here and the following return, exception part about the global/local and name.

zhangyangyu · 2018-01-23T16:20:12Z

Doc/library/sys.rst

+   ``'c_exception'``
+      A C function has raised an exception.  *arg* is the C function object.
+
+   ``'opcode'``


opcode events won't be triggered for profile functions, also note the above list needs also be modified.

zhangyangyu · 2018-01-23T16:21:28Z

Doc/c-api/init.rst

+   Any trace function registered using :c:func:`PyEval_SetTrace` will not receive
+   :const:`PyTrace_C_CALL`, :const:`PyTrace_C_EXCEPTION` or :const:`PyTrace_C_RETURN`
+   as a value for the *what* parameter.
+


I prefer to combine this section with the specified PyEval_SetTrace part.

pablogsal · 2018-01-23T16:29:01Z

@zhangyangyu Upon inspection and testing I think that profile functions cannot receive "exception" events as well. Can you confirm this?

zhangyangyu · 2018-01-23T16:37:42Z

Yes. But profile functions receive even PyTrace_C_EXCEPTION, why not Python exception events?

zhangyangyu · 2018-01-23T16:44:21Z

From the unitest test_sys_setprofile, exception events should never be received by profile functions, so seems it's by design.

…not being received.

zhangyangyu · 2018-01-23T17:22:28Z

PyEval_SetProfile should also be updated that not only line number events but also Python exception events.

…ceived.

zhangyangyu · 2018-01-24T02:46:03Z

I think the following paragraph need some modification, something like "... but it is called with different events, for example ..."

The system's profile function is called similarly to the
system's trace function (see :func:settrace), but it isn't called for each
executed line of code (only on call and return, but the return event is reported
even when an exception has been set).

And your NEWS entry needs updated too. It's not only about C events now. BTW, we need a new issue to complete the C part about PyTrace_OPCODE.

miss-islington · 2018-01-24T12:57:52Z

Thanks @pablogsal for the PR, and @zhangyangyu for merging it 🌮🎉.. I'm working now to backport this PR to: 2.7, 3.6.
🐍🍒⛏🤖

zhangyangyu · 2018-01-24T12:58:21Z

@pablogsal I merged it! Nice work!

miss-islington · 2018-01-24T12:58:55Z

Sorry, @pablogsal and @zhangyangyu, I could not cleanly backport this to 3.6 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 131fd7f96c619bc7eaea956e45c6337175f4b27f 3.6

miss-islington · 2018-01-24T12:59:54Z

Sorry, @pablogsal and @zhangyangyu, I could not cleanly backport this to 2.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 131fd7f96c619bc7eaea956e45c6337175f4b27f 2.7

…ofile (pythonGH-4056). (cherry picked from commit 131fd7f)

bedevere-bot · 2018-01-24T13:43:02Z

GH-5298 is a backport of this pull request to the 3.6 branch.

…ofile (pythonGH-4056). (cherry picked from commit 131fd7f)

bedevere-bot · 2018-01-24T13:58:12Z

GH-5299 is a backport of this pull request to the 2.7 branch.

…H-4056) (#5298) (cherry picked from commit 131fd7f)

…H-4056). (#5299) (cherry picked from commit 131fd7f)

the-knights-who-say-ni added the CLA signed label Oct 20, 2017

bedevere-bot added the awaiting review label Oct 20, 2017

Explain real behaviour of sys.settrace and sys.setprofile

2199f40

pablogsal force-pushed the 17799 branch from 75ee0ee to 2199f40 Compare October 20, 2017 10:59

zhangyangyu requested changes Jan 22, 2018

View reviewed changes

bedevere-bot added awaiting changes and removed awaiting review labels Jan 22, 2018

Add documentation in the c-api section

62918e4

bedevere-bot added awaiting change review and removed awaiting changes labels Jan 22, 2018

Add News entry

778c098

csabella reviewed Jan 22, 2018

View reviewed changes

Correct typo

18b88ec

zhangyangyu reviewed Jan 23, 2018

View reviewed changes

pablogsal added 2 commits January 23, 2018 16:05

Update init.rst

a9f381f

Update sys.rst

3887c6c

zhangyangyu reviewed Jan 23, 2018

View reviewed changes

Remove local/global references in setprofile. Remove events that are …

076a702

…not being received.

pablogsal added 2 commits January 23, 2018 16:57

Corrected typo in PyEval_SetTrace section

d0dce97

Remove exception event from profile paragraph

9301d56

pablogsal added 2 commits January 23, 2018 17:27

Update PyEval_SetProfile to indicate that exception events are not re…

b4531a9

…ceived.

Remove line about exception events in the setprofile section.

72ca9cb

pablogsal added 2 commits January 24, 2018 10:19

Improve description of profile function

3061991

Update NEWS entry

426368c

zhangyangyu added needs backport to 3.6 docs Documentation in the Doc dir labels Jan 24, 2018

zhangyangyu approved these changes Jan 24, 2018

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting change review labels Jan 24, 2018

zhangyangyu merged commit 131fd7f into python:master Jan 24, 2018

bedevere-bot removed the awaiting merge label Jan 24, 2018

zhangyangyu pushed a commit to zhangyangyu/cpython that referenced this pull request Jan 24, 2018

[3.6] bpo-17799: Explain real behaviour of sys.settrace and sys.setpr…

3793d06

…ofile (pythonGH-4056). (cherry picked from commit 131fd7f)

bedevere-bot removed the needs backport to 3.6 label Jan 24, 2018

zhangyangyu pushed a commit to zhangyangyu/cpython that referenced this pull request Jan 24, 2018

[2.7] bpo-17799: Explain real behaviour of sys.settrace and sys.setpr…

7eaa652

…ofile (pythonGH-4056). (cherry picked from commit 131fd7f)

bedevere-bot removed the needs backport to 2.7 label Jan 24, 2018

zhangyangyu added a commit that referenced this pull request Jan 24, 2018

bpo-17799: Explain real behaviour of sys.settrace and sys.setprofile (G…

fd844ef

…H-4056) (#5298) (cherry picked from commit 131fd7f)

zhangyangyu added a commit that referenced this pull request Jan 24, 2018

bpo-17799: Explain real behaviour of sys.settrace and sys.setprofile (G…

e64a47b

…H-4056). (#5299) (cherry picked from commit 131fd7f)

pablogsal deleted the 17799 branch May 19, 2021 18:56

Uh oh!

bpo-17799: Explain real behaviour of sys.settrace and sys.setprofile #4056

bpo-17799: Explain real behaviour of sys.settrace and sys.setprofile #4056

Uh oh!

Conversation

pablogsal commented Oct 20, 2017 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zhangyangyu left a comment

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Jan 22, 2018

Uh oh!

pablogsal commented Jan 22, 2018

Uh oh!

bedevere-bot commented Jan 22, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pablogsal commented Jan 23, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pablogsal commented Jan 23, 2018

Uh oh!

zhangyangyu commented Jan 23, 2018

Uh oh!

zhangyangyu commented Jan 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zhangyangyu commented Jan 23, 2018

Uh oh!

zhangyangyu commented Jan 24, 2018

Uh oh!

miss-islington commented Jan 24, 2018

Uh oh!

zhangyangyu commented Jan 24, 2018

Uh oh!

miss-islington commented Jan 24, 2018

Uh oh!

miss-islington commented Jan 24, 2018

Uh oh!

bedevere-bot commented Jan 24, 2018

Uh oh!

bedevere-bot commented Jan 24, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

pablogsal commented Oct 20, 2017 •

edited by bedevere-bot

Loading

zhangyangyu commented Jan 23, 2018 •

edited

Loading