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

Py_Main() does not return on SystemExit #50747

Closed
Rogi mannequin opened this issue Jul 16, 2009 · 29 comments
Closed

Py_Main() does not return on SystemExit #50747

Rogi mannequin opened this issue Jul 16, 2009 · 29 comments
Labels
docs Documentation in the Doc dir type-bug An unexpected behavior, bug, or error

Comments

@Rogi
Copy link
Mannequin

Rogi mannequin commented Jul 16, 2009

BPO 6498
Nosy @mhammond, @birkenfeld, @abalkin, @vstinner, @Trundle, @ericfrederich
Files
  • bug-6498.patch: doc patch to correct Py_Main description.
  • bug-6498.patch: updated doc patch
  • issue6498_27.patch: updated patch of the updated doc patch
  • issue6498_31.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 2011-05-15.06:51:41.545>
    created_at = <Date 2009-07-16.23:30:21.925>
    labels = ['type-bug', 'docs']
    title = 'Py_Main() does not return on SystemExit'
    updated_at = <Date 2011-05-15.06:51:41.543>
    user = 'https://bugs.python.org/Rogi'

    bugs.python.org fields:

    activity = <Date 2011-05-15.06:51:41.543>
    actor = 'python-dev'
    assignee = 'docs@python'
    closed = True
    closed_date = <Date 2011-05-15.06:51:41.545>
    closer = 'python-dev'
    components = ['Documentation']
    creation = <Date 2009-07-16.23:30:21.925>
    creator = 'Rogi'
    dependencies = []
    files = ['21447', '21448', '21461', '21462']
    hgrepos = []
    issue_num = 6498
    keywords = ['patch', 'needs review']
    message_count = 29.0
    messages = ['90596', '90601', '90614', '91873', '91874', '91983', '92041', '132375', '132379', '132383', '132399', '132407', '132413', '132430', '132446', '132450', '132452', '132461', '132463', '132464', '132465', '132466', '132467', '132468', '132469', '132498', '132499', '132527', '136010']
    nosy_count = 10.0
    nosy_names = ['mhammond', 'georg.brandl', 'belopolsky', 'vstinner', 'Trundle', 'Rogi', 'Rakeka', 'santoso.wijaya', 'python-dev', 'eric.frederich']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue6498'
    versions = ['Python 2.7']

    @Rogi
    Copy link
    Mannequin Author

    Rogi mannequin commented Jul 16, 2009

    From teh docs:

    http://docs.python.org/c-api/veryhigh.html

    int Py_Main(int argc, char **argv
    The main program for the standard interpreter. This is made
    

    available for programs which embed Python. The argc and argv parameters
    should be prepared exactly as those which are passed to a C program’s
    main() function. It is important to note that the argument list may be
    modified (but the contents of the strings pointed to by the argument
    list are not). The return value will be the integer passed to the
    sys.exit() function, 1 if the interpreter exits due to an exception, or
    2 if the parameter list does not represent a valid Python command line.

    Note that if an otherwise unhandled SystemError is raised, this
    

    function will not return 1, but exit the process, as long as
    Py_InspectFlag is not set.

    Py_Main() still does not return on SystemExit.

    @Rogi Rogi mannequin assigned birkenfeld Jul 16, 2009
    @Rogi Rogi mannequin added docs Documentation in the Doc dir type-bug An unexpected behavior, bug, or error labels Jul 16, 2009
    @Rakeka
    Copy link
    Mannequin

    Rakeka mannequin commented Jul 17, 2009

    I'm having the same problem. The source of the problem seem to be in
    PyRun_InteractiveOneFlags(). It prints and clears the last error.

    @Rogi
    Copy link
    Mannequin Author

    Rogi mannequin commented Jul 17, 2009

    As a workaround, I copied teh function PyRun_InteractiveOneFlags() to my
    own source and modified it so it would not print or clear exceptions.
    However, I have never found documentation about PyArena* or
    PyParser_AST*. Are those functions public or just for internal use?

    @Rogi
    Copy link
    Mannequin Author

    Rogi mannequin commented Aug 22, 2009

    bump

    @Rakeka
    Copy link
    Mannequin

    Rakeka mannequin commented Aug 22, 2009

    bump =op

    @vstinner
    Copy link
    Member

    Can you propose a patch fixing this issue?

    @Rogi
    Copy link
    Mannequin Author

    Rogi mannequin commented Aug 28, 2009

    I will try to fix and submit a patch. Just a second =op

    @admin admin mannequin assigned docspython and unassigned birkenfeld Oct 29, 2010
    @mhammond
    Copy link
    Contributor

    Isn't the only problem here that the docs refer to SystemError instead of SystemExit - eg 'raise SystemError("foo")' in an interactive session doesn't terminate the process at all (and I don't believe it should) whereas SystemExit obviously does.

    @Rogi
    Copy link
    Mannequin Author

    Rogi mannequin commented Mar 27, 2011

    @mhammond

    Wat did you say?

    I tried to fix this issue at teh time with no success. It would require too
    much rewriting. I have not found any way to handle exceptions at C level using
    this function up to teh present time.

    @mhammond
    Copy link
    Contributor

    Note the quoted documentation in comment 1, the paragraph "Note that if an otherwise unhandled SystemError ..."

    I don't think that paragraph is correct - SystemError doesn't seem to terminate Py_Main - but if you replace "SystemError" with "SystemExit", that paragraph is correct, including the comment about Py_InspectFlag.

    So I think there are 2 simple documentation bugs (the component of this bug is "Documentation") and not necessarily a behaviour bug (even though the "type" is set to behaviour :)

    • The first paragraph should have references to sys.exit() removed (I didn't mention this in my previous comment!)

    • The second paragraph should s/SystemError/SystemExit/ and optionally a note that calling sys.exit() will result in a SystemExit exception.

    I just traced through this in Python 2.6 - PyRun_InteractiveOneFlags winds up calling PyErr_PrintEx() and this function explicitly checks for SystemExit and calls handle_system_exit, which calls exit(). There doesn't seem to be any special handling for SystemError at all.

    @ericfrederich
    Copy link
    Mannequin

    ericfrederich mannequin commented Mar 28, 2011

    So there is a disconnect.

    You can either change the documentation to match the behavior or you can change the code to match the documentation.

    I would prefer to leave the documentation alone and make Py_Main return rather than exit on sys.exit. That's just my 2 cents.

    @Rogi
    Copy link
    Mannequin Author

    Rogi mannequin commented Mar 28, 2011

    Sorry, some years have passed since I posted this. I will take a look on teh
    code again.

    @Rogi
    Copy link
    Mannequin Author

    Rogi mannequin commented Mar 28, 2011

    I reviewed teh problem and rewrote teh text to better explain what is
    happening. Also, I noticed teh change to teh docs, and in fact it is
    not correct that teh functions call exit() on SystemError.

    *** From teh docs:

    http://docs.python.org/c-api/veryhigh.html

    int Py_Main(int argc, char **argv
    The main program for the standard interpreter. This is made
    

    available for programs which embed Python. The argc and argv parameters
    should be prepared exactly as those which are passed to a C program¿s
    main() function. It is important to note that the argument list may be
    modified (but the contents of the strings pointed to by the argument
    list are not). The return value will be the integer passed to the
    sys.exit() function, 1 if the interpreter exits due to an exception, or
    2 if the parameter list does not represent a valid Python command line.

    Note that if an otherwise unhandled SystemError is raised, this
    

    function will not return 1, but exit the process, as long as
    Py_InspectFlag is not set.

    *** Teh problem:

    Teh Py_Main() function does not return on any unhandled
    

    exception. If a unhandled exception is caught, teh function prints teh
    error and go to teh prompt. Try:

    	...
    	>>> raise(SystemError)
    	Traceback (most recent call last):
    	  File "<stdin>", line 1, in <module>
    	SystemError
    	>>>

    Consequently, Py_Main() does not return on unhandled SystemExit. But not
    only that. It calls exit() which terminate teh process. Try:

    	...
    	Py_Main(argc, argv);
    	fprintf(stderr, "reached!");
    	...
    	...
    	>>> raise(SystemExit)
    	$ 

    note it did not print after Py_Main() termination.

    *** Analysis:

    After greping python source, I found that this behaviour is
    

    triggered in function PyErr_PrintEx(). For some reason, SystemExit is
    handled here, in a function that, from its name, should print, not
    handle, and its handling involves a call to exit(), which terminate teh
    teh process. Also, not only Py_Main() is affected by this, but any other
    function that calls PyErr_PrintEx(), such as PyRun_SimpleString().

    I tried to fix this but it would require some rewriting. I think
    

    this could be accomplished with some set/longjmp, but still some
    rewriting would be needed to fix other functions that rely on
    PyErr_PrintEx() behaviour.

    *** Possible solutions:

    • fix teh source to match teh docs
    • fix teh docs to match teh source

    *** Proposed solution:

    Teh current behaviour hit me when I was writing an app which
    

    had Py_Main() running on a thread. If teh user typed exit() in Python,
    teh program would crash from exiting on a bad place. Based on this I
    propose a review of teh source that adhere to teh following:

    1 - PyErr_PrintEx() should not be handling any kind of
    

    exception. This should be done by a function liek
    PyErr_HandleException().

    2 - In functions liek PyRun_SimpleString(), teh default
    

    exception handling should only occur after giving teh programmer a
    chance to handle and clear teh exception himself.

    3 - No calls to exit() should be made by teh Python interpreter,
    

    except, maybe, in some function which is a explicit wrapper.

    Since proposing is easy and doing is not, It would be nice to
    

    have, for now, a warning at teh docs for every bad behaving function,
    specially those which call exit().

    @mhammond
    Copy link
    Contributor

    @rogi - you seem to have a problem with your keyboard - the 'h' and 'e' keys seem to have been swapped.

    The docs are wrong regardless - I don't think anyone would suggest the behaviour match the docs regarding SystemError - having Py_Main return on SystemError would be backwards incompatible.

    Given the discussion in those docs about SystemError, I'm confident that the person who wrote the docs was simply attempting to document the actual implementation and made an error. The implementation has, as far as I know, always worked the way it does (ie, there has never been a regression in this). So IMO there are 2 issues:

    • The documentation should be made to reflect the actual and historic behaviour. The fact it doesn't is a clear bug and it should be addressed ASAP and without changing the behaviour so people are not misled in the future. This change should be easy and non-controversial.

    • You may have a desire to change the actual and historic implementation, which I can understand. However, this should be tackled in a new "feature request". If the feature request is accepted, then both the behaviour and the docs should be changed to match the new agreed behaviour. This change would be harder and may well be controversial, depending on a perception on how many people rely on the current behaviour regardless of what the docs say.

    @Rogi
    Copy link
    Mannequin Author

    Rogi mannequin commented Mar 29, 2011

    @mhammond

    Maybe its just me but it seems to be a really bad idea to let
    

    those functions terminate your process as they wish. Teh programmer
    should be allowed to control teh flow of his application.

    However, I am not teh only one affected by this. I recall seeing
    

    blender crash when I typed raise(SystemExit) on its interactive console.
    Teh new blender beta worked around this by UNDEFINING SystemExit, which,
    to me, seems a UGLY HACK.

    If backwards compatibility is teh problem, things could still be
    

    hacked to maintain it and achieve teh desired behaviour during
    transition. For example, one could set a macro GOOD_BEHAVING_PYTHON and
    obtain teh desired behaviour, without breaking old stuff. It's ugly but
    it works.

    Teh new behaviour could be something liek that:
    
    	...
    	#define GOOD_BEHAVING_PYTHON 1
    	#include <Python.h>
    	...
    	PyRun_SimpleString(...);
    /* allow programmer to do what he wants \*/
    
    	/* if he wants to, he can do teh following */
    	PyErr_Print();
    	PyErr_Handle();
    	...

    this way, Py_Main() could be implemented in a way it would return to its
    caller instead of terminating teh process.

    Adn about my keyboadr, its not juts teh 'h' adn 'e' keys.
    

    Cheers.

    @Rogi
    Copy link
    Mannequin Author

    Rogi mannequin commented Mar 29, 2011

    @mhammond

    After reading your post again I noticed you talk about
    

    SystemError and not SystemExit. In fact, Py_Main() should not return on
    SystemError. Teh documentation states "if the interpreter exits due to
    an exception", which implies teh interpreter _exiting_, not simply
    catching a exception.

    It is just a BAD IDEA to allow python functions to print
    

    messages or call exit() as they wish. They should be split so teh
    programmer could put printing and exiting where he finds appropriate.
    Teh printing part is just another nuisance, but teh exit() is serious.

    Again, all of this could be accomplished while maintaining
    

    backward compatibility.

    Cheers.

    @mhammond
    Copy link
    Contributor

    @rogi - you might like to re-read my responses a couple more times:

    • I refer to SystemError as the docs *you quoted* refer to SystemError. Therefore, we should *not* make the implementation match the docs - the docs would be wrong *even if* we change Python to work how you want. IOW, the docs need changing regardless of the outcome (and this is the bug where that should be managed, given the Components and assignee already on this bug)

    • I understand your point that calling exit() directly is undesirable but that should be handled as a new separate feature request bug. But while that bug goes through the process, the docs should be fixed to reflect the current reality (this bug). Once your new bug is accepted and fixed, the docs should be changed again to reflect that new reality.

    @Rogi
    Copy link
    Mannequin Author

    Rogi mannequin commented Mar 29, 2011

    @mhammond

    You said:

    "The docs are wrong regardless - I don't think anyone would
    suggest the behaviour match the docs regarding SystemError -
    having Py_Main return on SystemError would be backwards
    incompatible."
    

    but teh problem is:

    Py_Main() calls exit() on SystemExit instead of returning.
    

    and documentation says:

    "The return value will be the integer passed to the sys.exit()
    function, 1 if the interpreter exits due to an exception, or 2
    if the parameter list does not represent a valid Python command
    line."
    

    which seems to be teh desired and broken behaviour.

    Similar problems happen with other functions, such as
    PyRun_SimpleString(), and teh solution to all those issues are related.
    What teh docs says currently about SystemError calling exit() is just
    _WRONG_.

    Also, I am not asking for a new feature. I'm pointing that there is
    something wrong, with teh docs or teh code, and that it is probably teh
    code.

    I hope this clarified teh situation.

    @mhammond
    Copy link
    Contributor

    What teh docs says currently about SystemError calling
    exit() is just _WRONG_.

    Correct - the docs should be fixed - which is what this bug is currently addressing (see the "Components" and "Assigned To" fields)

    Also, I am not asking for a new feature. I'm pointing that
    there is something wrong, with teh docs or teh code, and
    that it is probably teh code.

    Nope - it is the docs. The docs were written after the code and incorrectly describe it in 2 important ways.

    If you want the code to change the way it has been forever, then you are asking for a new feature.

    @Rogi
    Copy link
    Mannequin Author

    Rogi mannequin commented Mar 29, 2011

    Fair. Still I dont liek it very much. I will return!

    @mhammond
    Copy link
    Contributor

    For completeness, here is a doc patch against 2.6 which corrects the documentation.

    @Rogi
    Copy link
    Mannequin Author

    Rogi mannequin commented Mar 29, 2011

    I may be wrong, but I think Py_Main() will _never_ return 1.

    @Rogi
    Copy link
    Mannequin Author

    Rogi mannequin commented Mar 29, 2011

    Oh, and other functions on this doc, such as PyRun_SimpleStringFlags(),
    have this same documentation problem about SystemError.

    @mhammond
    Copy link
    Contributor

    It will return 1 if you specify a script to run and that has an unhandled exception.

    @mhammond
    Copy link
    Contributor

    Good catch - new patch with PyRun_SimpleStringFlags() corrected too.

    @abalkin
    Copy link
    Member

    abalkin commented Mar 29, 2011

    The SystemExit vs. SystemError is clearly a typo in the doc. The VC comment accompanying the addition of that note says "note that Py_Main doesnt return on SystemExit." See [e5d8f0c0d634] and bpo-5227.

    @abalkin
    Copy link
    Member

    abalkin commented Mar 29, 2011

    The updated doc patch is missing :exc: markup on one of "SystemExit"s.

    @Trundle
    Copy link
    Mannequin

    Trundle mannequin commented Mar 29, 2011

    The comment in the source that describes Py_InspectFlag also says SystemError instead of SystemExit. I changed that and added the missing :exc: for the SystemExit and created a patch for 3.1.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented May 15, 2011

    New changeset 0311f62714f7 by Georg Brandl in branch '3.1':
    Closes bpo-6498: fix several misspellings of "SystemExit" as "SystemError".
    http://hg.python.org/cpython/rev/0311f62714f7

    New changeset 7089afd69a1a by Georg Brandl in branch '3.2':
    Merge bpo-6498 fix from 3.1.
    http://hg.python.org/cpython/rev/7089afd69a1a

    New changeset 94e3c44b0662 by Georg Brandl in branch 'default':
    Merge bpo-6498 fix from 3.2.
    http://hg.python.org/cpython/rev/94e3c44b0662

    New changeset 97a75fccd7c8 by Georg Brandl in branch '2.7':
    Port bpo-6498 fix: fix several misspellings of "SystemExit" as "SystemError".
    http://hg.python.org/cpython/rev/97a75fccd7c8

    @python-dev python-dev mannequin closed this as completed May 15, 2011
    @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
    docs Documentation in the Doc dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants