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

use string_print() in gdb #47882

Closed
vstinner opened this issue Aug 21, 2008 · 8 comments
Closed

use string_print() in gdb #47882

vstinner opened this issue Aug 21, 2008 · 8 comments
Labels
performance Performance or resource usage stdlib Python modules in the Lib dir

Comments

@vstinner
Copy link
Member

BPO 3632
Nosy @amauryfa, @vstinner, @devdanzin
Files
  • pyobject_dump.patch: Call PyGILState_Ensure() in _PyObject_Dump()
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2008-12-15.22:29:29.165>
    created_at = <Date 2008-08-21.12:39:39.593>
    labels = ['library', 'performance']
    title = 'use string_print() in gdb'
    updated_at = <Date 2008-12-15.22:29:29.164>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2008-12-15.22:29:29.164>
    actor = 'amaury.forgeotdarc'
    assignee = 'none'
    closed = True
    closed_date = <Date 2008-12-15.22:29:29.165>
    closer = 'amaury.forgeotdarc'
    components = ['Library (Lib)']
    creation = <Date 2008-08-21.12:39:39.593>
    creator = 'vstinner'
    dependencies = []
    files = ['11193']
    hgrepos = []
    issue_num = 3632
    keywords = ['patch']
    message_count = 8.0
    messages = ['71628', '71630', '71631', '72806', '72822', '73156', '77637', '77886']
    nosy_count = 3.0
    nosy_names = ['amaury.forgeotdarc', 'vstinner', 'ajaksu2']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = None
    status = 'closed'
    superseder = None
    type = 'performance'
    url = 'https://bugs.python.org/issue3632'
    versions = ['Python 2.6']

    @vstinner
    Copy link
    Member Author

    "pyo" macro from gdbinit (see bpo-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.

    @vstinner vstinner added stdlib Python modules in the Lib dir performance Performance or resource usage labels Aug 21, 2008
    @amauryfa
    Copy link
    Member

    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.

    @vstinner
    Copy link
    Member Author

    Oh! I have a better idea: why not patching _PyObject_Dump() instead of
    string_print() :-) So here is a new patch.

    @vstinner
    Copy link
    Member Author

    vstinner commented Sep 8, 2008

    Can anyone review my new patch?

    @amauryfa
    Copy link
    Member

    amauryfa commented Sep 9, 2008

    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.

    @devdanzin
    Copy link
    Mannequin

    devdanzin mannequin commented Sep 13, 2008

    I would love to have this patch, along with those of bpo-3631 and bpo-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.

    @vstinner
    Copy link
    Member Author

    Can anyone apply this patch? Or explain me why it can not be applied?

    @amauryfa
    Copy link
    Member

    Committed in r67802.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    performance Performance or resource usage stdlib Python modules in the Lib dir
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants