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

When interrupted during startup, Python should not call abort() but exit() #64182

Closed
JurkoGospodneti mannequin opened this issue Dec 15, 2013 · 14 comments
Closed

When interrupted during startup, Python should not call abort() but exit() #64182

JurkoGospodneti mannequin opened this issue Dec 15, 2013 · 14 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) OS-windows type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@JurkoGospodneti
Copy link
Mannequin

JurkoGospodneti mannequin commented Dec 15, 2013

BPO 19983
Nosy @malemburg, @tim-one, @pitrou, @vstinner, @tjguk, @briancurtin
Files
  • crash-info-10.txt: More detailed information on a specific crash.
  • crash-info-1-9 - Python tracebacks only.txt: Crash related Python tracebacks included in the original mailing list posting.
  • crash-info-11.txt: More detailed information on a specific crash.
  • init_error.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 = None
    closed_at = <Date 2019-05-27.14:55:55.788>
    created_at = <Date 2013-12-15.02:28:58.106>
    labels = ['interpreter-core', 'OS-windows', 'type-crash']
    title = 'When interrupted during startup, Python should not call abort() but exit()'
    updated_at = <Date 2019-05-27.14:55:55.788>
    user = 'https://bugs.python.org/JurkoGospodneti'

    bugs.python.org fields:

    activity = <Date 2019-05-27.14:55:55.788>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2019-05-27.14:55:55.788>
    closer = 'vstinner'
    components = ['Interpreter Core', 'Windows']
    creation = <Date 2013-12-15.02:28:58.106>
    creator = 'Jurko.Gospodneti\xc4\x87'
    dependencies = []
    files = ['33137', '33138', '33140', '33215']
    hgrepos = []
    issue_num = 19983
    keywords = ['patch']
    message_count = 14.0
    messages = ['206212', '206213', '206214', '206246', '206282', '206284', '206287', '206288', '206289', '206626', '215126', '215127', '215136', '343634']
    nosy_count = 7.0
    nosy_names = ['lemburg', 'tim.peters', 'pitrou', 'vstinner', 'tim.golden', 'brian.curtin', 'Jurko.Gospodneti\xc4\x87']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'crash'
    url = 'https://bugs.python.org/issue19983'
    versions = ['Python 3.4', 'Python 3.5']

    @JurkoGospodneti
    Copy link
    Mannequin Author

    JurkoGospodneti mannequin commented Dec 15, 2013

    If you press Ctrl-C during Python startup on Windows you may get interpreter crashes with different Python tracebacks displayed on the standard system error output stream.

    Reproduced using:

    • Windows 7 SP1 x64
    • Python 3.3.3 (64-bit) as downloaded from 'http://www.python.org/download/releases/3.3.3' (but seen with different earlier Python versions as well).
    • either a non-trivial Python script, one containing only a '#! python3' shabang line, or a completely empty one
    • default site.py

    To reproduce simply run the Python interpreter with a prepared Python script as input and press Ctrl-C immediately afterwards.

    Possible results:

    • Script finishes before your Ctrl-C kicks in.
    • You get a clean KeyboardInterrupt traceback and the script exits.
    • You get a KeyboardInterrupt traceback and the interpreter process crashes.

    I'm attaching more detailed information on specific crash instances.

    For some more information & background see the devel mailing list thread started at: 'https://mail.python.org/pipermail/python-dev/2013-December/130750.html'.

    @JurkoGospodneti JurkoGospodneti mannequin added interpreter-core (Objects, Python, Grammar, and Parser dirs) OS-windows type-crash A hard crash of the interpreter, possibly with a core dump labels Dec 15, 2013
    @JurkoGospodneti
    Copy link
    Mannequin Author

    JurkoGospodneti mannequin commented Dec 15, 2013

    I can reproduce this most easily if I run a command like:

    clean.cmd & run.py

    where clean.cmd is any short batch script and run.py is a file containing only the '#! python3' shabang line.

    The batch script in front is not necessary, and I've originally been reproducing the issue without it, but the problem seems much easier to reproduce with it, most likely because is slightly delays the Python startup and thus makes it easier for the Ctrl-C signal to kick in early enough during Python startup.

    @JurkoGospodneti
    Copy link
    Mannequin Author

    JurkoGospodneti mannequin commented Dec 15, 2013

    I reproduced the issue about 10 more times to see if I'd get some more useful C tracebacks in Visual Studio, but they seems to be the pretty much the same every time (as seen in the attached http://bugs.python.org/file33137/crash-info-10.txt file).

    I'm attaching another one, just for the record. The only difference I see between crash #10 and this one is that second thread has a bit different name, but that is most likely just some internal Windows API worker thread and not something explicitly started by Python or relevant to this report.

    @pitrou
    Copy link
    Member

    pitrou commented Dec 15, 2013

    Ah, ok. So it's a controlled crash: Python fails initializing the standard streams and so it decides to bail out (by using Py_FatalError, since Py_Initialize doesn't return an error code).

    What we could do is call initsigs() after initstdio() (but still before initsite(), since initsite() can call arbitrary Python code).

    I'm a bit surprised that you manage to press Ctrl-C so fast that it occurs right during initialization of standard streams, by the way :-)

    @pitrou pitrou changed the title Ctrl-C causes startup crashes on Windows Ctrl-C at startup can end in a Py_FatalError call Dec 15, 2013
    @vstinner
    Copy link
    Member

    I modified initstdio() to add raise(SIGINT); at the beginning of the function. I get:

    $ ./python
    Fatal Python error: Py_Initialize: can't initialize sys standard streams
    Traceback (most recent call last):
      File "<frozen importlib._bootstrap>", line 2157, in _find_and_load
    KeyboardInterrupt
    Abandon (core dumped)

    You can also inject SIGINT in gdb if you set a breakpoint on initstdio():

    (gdb) b initstdio
    (gdb) run
    <python stopped at initstdio enter>
    (gdb) signal SIGINT

    I don't consider this as a bug, but I understand that you would prefer a different behaviour. The question is which behaviour do you want? You want to ignore CTRL+c during initialization? Do you prefer to quit without calling abort(), ex: exit with exit code 1?

    Maybe we should modify Py_FatalError() to call exit(1) in release mode, and only call abort() in debug mode? Dumping a core dump, opening a Windows fatal error popup, calling Fedora ABRT handler, etc. is maybe not very useful, especially for the KeyboardInterrupt case.

    What we could do is call initsigs() after initstdio() (but still before initsite(), since initsite() can call arbitrary Python code).

    initfsencoding() calls also Python code. It loads at least 3 Python scripts: encodings/init.py, encodings/aliases.py and encodings/NAME.py where NAME is your locale encoding.

    IMO signal handlers should be set up before any Python code is loaded, so initsigs() should be called before initfsencoding().

    @malemburg
    Copy link
    Member

    On 16.12.2013 10:02, STINNER Victor wrote:

    Maybe we should modify Py_FatalError() to call exit(1) in release mode, and only call abort() in debug mode? Dumping a core dump, opening a Windows fatal error popup, calling Fedora ABRT handler, etc. is maybe not very useful, especially for the KeyboardInterrupt case.

    I don't think changing Py_FatalError() is a good idea. However,
    its use in this particular case (streams not initializing) appears
    wrong.

    Python should simply exit with an error code in such a case; which then
    also allows the calling script or application to react to the error.

    The fatal error is reserved for cases where you cannot continue
    and can't even report back an error, not even as error code.
    Those are rare situations. You usually only use abort() if you need
    to debug the situation via a core dump, which is not needed in
    this case, since we know that the user caused a keyboard
    interrupt and in most other cases know that it's a config problem,
    not a problem in the C implementation of Python.

    @vstinner
    Copy link
    Member

    2013/12/16 Marc-Andre Lemburg <report@bugs.python.org>:

    I don't think changing Py_FatalError() is a good idea. However,
    its use in this particular case (streams not initializing) appears
    wrong.

    Python should simply exit with an error code in such a case; which then
    also allows the calling script or application to react to the error.

    Before exiting, you need a message. If there is also an exception, you
    may want to display it. If there is no exception, you may want to
    display the Python traceback. All these tasks are already implemented
    in Py_FatalError.

    If the defaullt behaviour of Py_FatalError() cannot be modified, a new
    function should be be added, a function sharing its code with
    Py_FatalError().

    Example: a new private function "void initerror(const char *message)"
    only used during Py_Initialize().

    @malemburg
    Copy link
    Member

    On 16.12.2013 10:30, STINNER Victor wrote:

    STINNER Victor added the comment:

    2013/12/16 Marc-Andre Lemburg <report@bugs.python.org>:
    > I don't think changing Py_FatalError() is a good idea. However,
    > its use in this particular case (streams not initializing) appears
    > wrong.
    >
    > Python should simply exit with an error code in such a case; which then
    > also allows the calling script or application to react to the error.

    Before exiting, you need a message. If there is also an exception, you
    may want to display it. If there is no exception, you may want to
    display the Python traceback. All these tasks are already implemented
    in Py_FatalError.

    If the defaullt behaviour of Py_FatalError() cannot be modified, a new
    function should be be added, a function sharing its code with
    Py_FatalError().

    Example: a new private function "void initerror(const char *message)"
    only used during Py_Initialize().

    Sounds reasonable.

    BTW: Why can't we make this an official API function, e.g.
    Py_Terminate() ?

    @vstinner
    Copy link
    Member

    BTW: Why can't we make this an official API function, e.g. Py_Terminate() ?

    Exiting Python immediatly is bad practice, there is already Py_FatalError() for that.

    Instead of adding a second public function, I would prefer to remove most calls to Py_FatalError() and write nicer error handlers :-)

    @vstinner
    Copy link
    Member

    init_error.patch: modify Py_Initialize() to exit with exit(1) instead of abort(), to not call the sytem fault handler (ex: dump a coredump on Linux, or open a popup on Windows).

    The patch calls also initsigs() before initfsencoding(), because initfsencoding() may call Python code (encodings implemented in Python).

    Example with gdb (to simulate a CTRL+c during Python startup):

    $ gdb ./python
    (gdb) b initfsencoding 
    (gdb) run
    Breakpoint 1, initfsencoding (interp=0x971420) at Python/pythonrun.c:972
    (gdb) signal SIGINT
    (gdb) cont
    Python initialization error: Unable to get the locale encoding
    Traceback (most recent call last):
      File "<frozen importlib._bootstrap>", line 2152, in _find_and_load
    KeyboardInterrupt
    [Inferior 1 (process 15566) exited with code 01]

    The process exited with exit code 1, not with SIGABRT.

    @vstinner vstinner changed the title Ctrl-C at startup can end in a Py_FatalError call When interrupted during startup, Python should not call abort() but exit() Feb 13, 2014
    @JurkoGospodneti
    Copy link
    Mannequin Author

    JurkoGospodneti mannequin commented Mar 29, 2014

    Anyone have an eye on this item?
    What are the chances of the suggested patch being applied for
    Python 3.5?
    Or the issue resolved in some other way?
    Anything I can do to help?

    P.S.
    I'm not sure exactly how the regular development protocol
    goes around here so, just for the record, please know that no
    offense in intended with this ping. :-)

    @JurkoGospodneti
    Copy link
    Mannequin Author

    JurkoGospodneti mannequin commented Mar 29, 2014

    err... by Python 3.5 I meant Python 3.4.1 as well :-)

    @ericsnowcurrently
    Copy link
    Member

    P.S.
    I'm not sure exactly how the regular development protocol
    goes around here so, just for the record, please know that no
    offense in intended with this ping. :-)

    What you did is just right. The offer to help move things along is
    both effective and appreciated!

    One thing you can do is determine which patch (likely the most recent
    one) is the most appropriate and then apply it to a local clone of the
    repo and run the test suite. It probably won't apply cleanly but I'm
    sure it wouldn't take much to fix that. Then you can make a note in
    the issue as to what you find. Doing this is probably unnecessary
    (and I don't want to waste your time), but it would also be a good way
    to get familiar with our workflow, particularly relative to mercurial.

    Another thing you can do is see if there are any outstanding review
    comments that have not been addressed. Next to each patch should be a
    review link that will take you to the web-based code review tool we
    use. If the patch has any unresolved feedback, you could address it
    in your own "fork" of the patch and attach it to this ticket.

    Finally, you could review the patch yourself through the same link.
    My guess is that Victor hadn't gotten sufficient eyes on his patch.
    Furthermore we typically don't commit anything until we get another
    committer to have a look, so that may be a blocker as well.

    Posting to the core-mentorship list
    (https://mail.python.org/mailman/listinfo/core-mentorship) will help
    get more eyes on this issue if you are interested in getting more
    involved, which it sounds like you are.

    FYI, we've been working on a comprehensive guide to our development
    process. You will find it at http://docs.python.org/devguide/. That
    should help get you rolling. :)

    @vstinner
    Copy link
    Member

    This issue has been fixed by the PEP-587 in bpo-36763: when Py_Initialize() fails, it no longer calls abort() but only exit(1).

    @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
    interpreter-core (Objects, Python, Grammar, and Parser dirs) OS-windows type-crash A hard crash of the interpreter, possibly with a core dump
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants