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

traceback: errors in the linecache module at exit #66789

Closed
vstinner opened this issue Oct 10, 2014 · 20 comments
Closed

traceback: errors in the linecache module at exit #66789

vstinner opened this issue Oct 10, 2014 · 20 comments
Assignees
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@vstinner
Copy link
Member

BPO 22599
Nosy @pitrou, @vstinner, @vadmium, @serhiy-storchaka
Files
  • destructortest.py
  • traceback_ignore_linecache_error.patch
  • traceback_at_exit.patch
  • traceback_at_exit-2.patch
  • 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 = 'https://github.com/vstinner'
    closed_at = <Date 2014-12-05.09:27:35.302>
    created_at = <Date 2014-10-10.11:36:01.308>
    labels = ['type-bug', 'library']
    title = 'traceback: errors in the linecache module at exit'
    updated_at = <Date 2014-12-05.09:27:35.300>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2014-12-05.09:27:35.300>
    actor = 'vstinner'
    assignee = 'vstinner'
    closed = True
    closed_date = <Date 2014-12-05.09:27:35.302>
    closer = 'vstinner'
    components = ['Library (Lib)']
    creation = <Date 2014-10-10.11:36:01.308>
    creator = 'vstinner'
    dependencies = []
    files = ['36863', '36864', '36892', '36925']
    hgrepos = []
    issue_num = 22599
    keywords = ['patch']
    message_count = 20.0
    messages = ['228986', '228990', '228991', '228992', '229004', '229012', '229203', '229230', '229365', '229366', '229807', '229811', '229813', '229815', '229816', '229819', '232168', '232177', '232183', '232185']
    nosy_count = 5.0
    nosy_names = ['pitrou', 'vstinner', 'python-dev', 'martin.panter', 'serhiy.storchaka']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = None
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue22599'
    versions = ['Python 3.4', 'Python 3.5']

    @vstinner
    Copy link
    Member Author

    Attached destructortest.py script comes the issue bpo-22480. It calls traceback.print_exception() at exit. The problem is that the call occurs late during Python finalization. The object was stored in the namespace of the main module. Currently, Python deletes builtin functions and then deletes modules (in a random order?).

    Since the traceback module is used to display errors, it's annoying to get errors in the traceback module :-) I see two options to fix the specific issue of destructortest.py:

    • Ignore exceptions while calling linecache
    • Detect Python finalization and don't try to use the linecache module in this case

    Attached patch implements the second option. I'm not sure that it's the best one. It includes an unit test.

    Depending on the option, we can fix the issue only in Python 3.5, or fix Python 2.7, 3.4 and 3.5.

    Current output:
    ---

    Traceback (most recent call last):
    Exception ignored in: <bound method PrintExceptionAtExit.__del__ of <__main__.PrintExceptionAtExit object at 0x7f70b84ac878>>
    Traceback (most recent call last):
      File "destructortest.py", line 15, in __del__
      File "/home/haypo/prog/python/default/Lib/traceback.py", line 169, in print_exception
      File "/home/haypo/prog/python/default/Lib/traceback.py", line 153, in _format_exception_iter
      File "/home/haypo/prog/python/default/Lib/traceback.py", line 18, in _format_list_iter
      File "/home/haypo/prog/python/default/Lib/traceback.py", line 65, in _extract_tb_or_stack_iter
      File "/home/haypo/prog/python/default/Lib/linecache.py", line 15, in getline
      File "/home/haypo/prog/python/default/Lib/linecache.py", line 41, in getlines
      File "/home/haypo/prog/python/default/Lib/linecache.py", line 126, in updatecache
      File "/home/haypo/prog/python/default/Lib/tokenize.py", line 438, in open
    AttributeError: module 'builtins' has no attribute 'open'

    Expected output:
    ---

    Traceback (most recent call last):
      File "destructortest.py", line 7, in __init__
    ZeroDivisionError: division by zero

    @vstinner vstinner added the stdlib Python modules in the Lib dir label Oct 10, 2014
    @vstinner
    Copy link
    Member Author

    Errors in the traceback module (caused by linecache) are common when working on the asyncio module in debug mode. The asyncio tries to log exceptions at exit to help the debug to track bugs. It uses the garbage collector and destructors on objects. Example of debug tool:
    https://docs.python.org/dev/library/asyncio-dev.html#detect-exceptions-never-consumed

    See for examples issues bpo-22428 and bpo-22480.

    @pitrou
    Copy link
    Member

    pitrou commented Oct 10, 2014

    There is no patch.

    @vstinner
    Copy link
    Member Author

    There is no patch.

    Oh no :-( I revert all my local changes and I forgot to produce the patch... My change added a new sys._is_finalizing() function which exposes the private C variable "_Py_Finalizing".

    Well, here is a patch which implements the first option (ignore linecache errors).

    @serhiy-storchaka
    Copy link
    Member

    Alternative solution of this issue would be to use "from builtins import open as _open" instead of "import builtins".

    @vstinner
    Copy link
    Member Author

    See also the issue bpo-19421.

    @vstinner
    Copy link
    Member Author

    Alternative solution of this issue would be to use "from builtins import open as _open" instead of "import builtins".

    Right. I prefer your solution because it's much simpler, it doesn't make traceback less usable at exit, and it doesn't need to make assumption on the Python status.

    Here is a patch implementing this idea with an unit test.

    @serhiy-storchaka
    Copy link
    Member

    There is one downside of my solution. For now the code uses current builtin open() which can be overloaded (to handle reading from ZIP archive for example, or to check permissions). With my solution it uses builtin open() at the time of import. I don't know insofar current behavior is intentional. We should take a decision of yet one core developer.

    @vstinner
    Copy link
    Member Author

    There is one downside of my solution. For now the code uses current builtin open() which can be overloaded (to handle reading from ZIP archive for example, or to check permissions).

    Oh, does anyone really modify the builtin open() for that? If you already monkey-patch Python builtin, you are probably able to monkey-patch also tokenize._builtin_open.

    I don't think that monkey-patching builtins is promoted nor well supported. They are probably other places where a local references to builtins functions are kept.

    The importlib module provides the get_source() function which is the best way to retrieve the source code from any kind of module, including ZIP files. But tokenize.open() really expects a clear text ".py" file.

    With my solution it uses builtin open() at the time of import.

    I don't see how your solution is different than mine.

    But your solution is probably enough to tokenize needs (it only requires the builtin open funciton) and it's shorter.

    I don't know insofar current behavior is intentional. We should take a decision of yet one core developer.

    Since I wrote tokenize.open(), I can explain why I chose to import builtins :-) It's just one option to handle two functions with the same name: (builtins.)open and (tokenize.)open.

    "_open = open" is another common option to handle this issue.

    @vstinner
    Copy link
    Member Author

    traceback_at_exit-2.patch: Updated patch to remove "import builtins" from tokenize.py, it's no more needed.

    @vstinner
    Copy link
    Member Author

    traceback_at_exit-2.patch: Updated patch to remove "import builtins" from tokenize.py, it's no more needed.

    Antoine, Serhiy: What do you think about this patch?

    IMO the bug is very simple and fixes a common bug.

    @pitrou
    Copy link
    Member

    pitrou commented Oct 22, 2014

    The patch looks fragile to me: who knows whether other similar problems can appear in other situations?

    I would prefer something like traceback_ignore_linecache_error.patch, perhaps combined with a new function that would tell you whether the interpreter is shutting down.

    @serhiy-storchaka
    Copy link
    Member

    Since I wrote tokenize.open(), I can explain why I chose to import builtins :-)

    Then it is good to me.

    I would prefer something like traceback_ignore_linecache_error.patch, perhaps combined with a new function that would tell you whether the interpreter is shutting down.

    I think we can apply both solutions. Keeping a reference to open will make traceback more usable at exit and ignoring exception will make it more robust in worst case.

    And I support the idea about public function or attribute that would tell you whether the interpreter is shutting down. This would be useful in many modules. But this is different issue.

    @vstinner
    Copy link
    Member Author

    The patch looks fragile to me: who knows whether other similar problems can appear in other situations?

    Maybe we can always ignore non fatal errors when calling linecache
    from print_exception? Having the Python source code in the traceback
    is better, but it's easy to get it because the filename and line
    number is always present.

    Maybe the linecache module can provide a better exception than the
    current AttributeError?

    @vstinner
    Copy link
    Member Author

    The patch looks fragile to me: who knows whether other similar problems can appear in other situations?

    Oh, I forgot to say that yes, my patch is incomplete, but it is simple
    and it is already make the code more reliable.

    @vstinner
    Copy link
    Member Author

    Oh, I see that Antoine opened the issue bpo-22696.

    @serhiy-storchaka serhiy-storchaka added the type-bug An unexpected behavior, bug, or error label Nov 16, 2014
    @vstinner
    Copy link
    Member Author

    vstinner commented Dec 5, 2014

    Can I apply traceback_at_exit-2.patch? I know that my change on tokenize is not perfect, but it doesn't look like an hack, and it adds a new unit test.

    The linecache module can be enhanced in the issue bpo-22696.

    @serhiy-storchaka
    Copy link
    Member

    Yes, please do this.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Dec 5, 2014

    New changeset cd3244416592 by Victor Stinner in branch '3.4':
    Issue bpo-22599: Enhance tokenize.open() to be able to call it during Python
    https://hg.python.org/cpython/rev/cd3244416592

    New changeset 22f2784a276e by Victor Stinner in branch 'default':
    (Merge 3.4) Issue bpo-22599: Enhance tokenize.open() to be able to call it during
    https://hg.python.org/cpython/rev/22f2784a276e

    @vstinner
    Copy link
    Member Author

    vstinner commented Dec 5, 2014

    Yes, please do this.

    Ok done. I only added the test to default, not in the 3.4 branch. As Antoine noticed, the test is fragile, I perfer to not backport it.

    Ok, let's focus now on the issue bpo-22696 ;-)

    @vstinner vstinner closed this as completed Dec 5, 2014
    @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
    stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants