classification
Title: use string_print() in gdb
Type: performance Stage:
Components: Library (Lib) Versions: Python 2.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: ajaksu2, amaury.forgeotdarc, haypo
Priority: normal Keywords: patch

Created on 2008-08-21 12:39 by haypo, last changed 2008-12-15 22:29 by amaury.forgeotdarc. This issue is now closed.

Files
File name Uploaded Description Edit
pyobject_dump.patch haypo, 2008-08-21 12:57 Call PyGILState_Ensure() in _PyObject_Dump()
Messages (8)
msg71628 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2008-08-21 12:39
"pyo" macro from gdbinit (see #3631) uses _PyObject_Dump() to display 
an object. This function calls (indirectly) string_print() to display 
one line of text. But if the GIL is released (I guess it's the GIL or 
is it called the "thread state"?), gdb crashs Python:

   object  : Fatal Python error: PyEval_SaveThread: NULL tstate
   Program received signal SIGABRT, Aborted.
   0xffffe410 in __kernel_vsyscall ()

Workaround: ensure GIL before Py_BEGIN_ALLOW_THREADS... That sounds 
ugly but it works :-) So I propose to enable it in debug mode (#ifdef 
Py_DEBUG) with a patch.

I guess that the issue is very specific to (gdb) debugging and should 
not affect normal usage of Python. That's why I choosed to enable it 
only in debug mode.
msg71630 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2008-08-21 12:56
I once fell into the same issue, but the patch should modify
_PyObject_Dump(), around the call to PyObject_Print.

string_print() is not the only function called by _PyObject_Dump, by far...

And beware that many people routinely run python in "debug mode" outside
gdb, for example during development.
That's why it's better to modify _PyObject_Dump only; we can be sure
that it only affects a debugger session.
msg71631 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2008-08-21 12:57
Oh! I have a better idea: why not patching _PyObject_Dump() instead of 
string_print() :-) So here is a new patch.
msg72806 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2008-09-08 23:28
Can anyone review my new patch?
msg72822 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2008-09-09 06:47
The patch is fine.
I don't know if it can make the 2.6 release, but it is very simple, and
affect only a function used in debugger macros.
msg73156 - (view) Author: Daniel Diniz (ajaksu2) Date: 2008-09-13 00:17
I would love to have this patch, along with those of #3631 and #3610,
included in Misc as diffs. This would make it easier to get the improved
functionality in a new development box, besides allowing distributions
to apply them at will.
msg77637 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2008-12-11 23:07
Can anyone apply this patch? Or explain me why it can not be applied?
msg77886 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2008-12-15 22:29
Committed in r67802.
History
Date User Action Args
2008-12-15 22:29:29amaury.forgeotdarcsetstatus: open -> closed
resolution: accepted -> fixed
messages: + msg77886
2008-12-11 23:07:36hayposetmessages: + msg77637
2008-09-13 00:17:04ajaksu2setnosy: + ajaksu2
messages: + msg73156
2008-09-09 06:47:02amaury.forgeotdarcsetresolution: accepted
messages: + msg72822
2008-09-08 23:28:20hayposetmessages: + msg72806
2008-08-21 12:57:23hayposetfiles: - string_print.patch
2008-08-21 12:57:19hayposetfiles: + pyobject_dump.patch
2008-08-21 12:57:01hayposetmessages: + msg71631
2008-08-21 12:56:02amaury.forgeotdarcsetnosy: + amaury.forgeotdarc
messages: + msg71630
2008-08-21 12:39:39haypocreate