classification
Title: faulthandler prints tracebacks in reverse order
Type: behavior Stage: resolved
Components: Extension Modules Versions: Python 3.5
process
Status: closed Resolution: wont fix
Dependencies: Superseder:
Assigned To: Nosy List: ncoghlan, pitrou, vstinner
Priority: normal Keywords: patch

Created on 2012-04-24 22:17 by pitrou, last changed 2014-02-01 23:39 by ncoghlan. This issue is now closed.

Files
File name Uploaded Description Edit
reverse_frames.patch pitrou, 2012-04-24 22:38 review
Messages (7)
msg159223 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-04-24 22:17
Python usually prints traceback from the outer to the inner function: the inner frame is printed last. But faulthandler prints the inner frame first and the outer frame last.
msg159227 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-04-24 22:38
Here is a patch.
msg159229 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2012-04-24 22:51
faulthandler has to be as simple as possible because Python internal state may be completly corrupted. faulthandler was written to display the traceback on bugs like invalid memory read or write. I chose to print the traceback as it is stored in memory (from the current frame to the bottom of the stack using f_back).

If the traceback is corrupted in the middle of the frame stack, faulthandler displays the first half of the traceback, whereas it would not display anything with your patch.

The number of frames is also limited to avoid unlimited loops. The internal state may be corrupted and I don't want to track already seen frames or something like that. With your patch, you may miss where the bug occurred if the stack contains more than 100 frames. The limit is hardcoded and cannot be changed at runtime yet.

I already seen a segfault while faulthandler was trying to display the traceback of a segfault, more than once :-)
msg159252 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2012-04-25 05:49
Victor's argument makes sense to me. What I'd be inclined to do is shout at the reader a bit in the traceback header by making it:

    Traceback (most recent call FIRST):

And put a comment in the source code with the gist of what Victor wrote above (i.e. internal state may be corrupted, we do the best we can with what we've got, and that means printing the traceback from the inside out)
msg159260 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2012-04-25 07:10
> Victor's argument makes sense to me. What I'd be inclined to do is shout at the reader a bit in the traceback header by making it:
>
>    Traceback (most recent call FIRST):

The output is something like:

Thread 0xf758d8d0:

 File "/home/edcjones/programs/python/Python-3.3.0a2/Lib/socket.py",
line 407 in create_connection
 File "/home/edcjones/programs/python/Python-3.3.0a2/Lib/imaplib.py",
line 236 in _create_socket
 File "/home/edcjones/programs/python/Python-3.3.0a2/Lib/imaplib.py",
line 1201 in _create_socket
...

"Thread 0xf758d8d0:" can be replaced with "Thread 0xf758d8d0 (most
recent call FIRST):".

A "reverse=False" parameter can be added to
faulthandler.dump_traceback() and
faulthandler.dump_tracebacks_later(). But I prefer to not add it to
faulthandler.enable() nor faulthandler.register() because
dump_traceback() is called from a signal handler. It may be surprising
to have an option on some functions but not on all functions.
msg209847 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2014-01-31 23:58
The issue #19306 changed faulthandler output to mention explicitly the frame order. I'm still opposed to reverse_frames.patch for the reason I gave above. Can I close this issue as "wont fix"?
msg209926 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2014-02-01 23:39
For additional context, Guido's patch in issue 19306 changed "Traceback" to "Stack" and added the "(most recent call first)" to some places where it was missing.

Given the technical arguments in favour of keeping this code as simple as possible, agreed this is a "Won't Fix".
History
Date User Action Args
2014-02-01 23:39:22ncoghlansetstatus: open -> closed
resolution: wont fix
messages: + msg209926

stage: resolved
2014-01-31 23:58:40vstinnersetmessages: + msg209847
2014-01-31 22:07:45yselivanovsetversions: + Python 3.5, - Python 3.2, Python 3.3
2012-04-25 07:10:29vstinnersetmessages: + msg159260
2012-04-25 05:49:08ncoghlansetmessages: + msg159252
2012-04-24 22:51:14vstinnersetmessages: + msg159229
2012-04-24 22:38:12pitrousetfiles: + reverse_frames.patch

nosy: + ncoghlan
messages: + msg159227

keywords: + patch
2012-04-24 22:17:14pitroucreate