classification
Title: Py_Main() does not return on SystemExit
Type: behavior Stage: resolved
Components: Documentation Versions: Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: docs@python Nosy List: Rakeka, Rogi, Trundle, belopolsky, eric.frederich, georg.brandl, haypo, mhammond, python-dev, santoso.wijaya
Priority: normal Keywords: needs review, patch

Created on 2009-07-16 23:30 by Rogi, last changed 2011-05-15 06:51 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
bug-6498.patch mhammond, 2011-03-29 04:40 doc patch to correct Py_Main description.
bug-6498.patch mhammond, 2011-03-29 07:27 updated doc patch
issue6498_27.patch Trundle, 2011-03-29 20:57 updated patch of the updated doc patch
issue6498_31.patch Trundle, 2011-03-29 20:57
Messages (29)
msg90596 - (view) Author: Rogi (Rogi) Date: 2009-07-16 23:30
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.
msg90601 - (view) Author: Raphaela (Rakeka) Date: 2009-07-17 02:32
I'm having the same problem. The source of the problem seem to be in
PyRun_InteractiveOneFlags(). It prints and clears the last error.
msg90614 - (view) Author: Rogi (Rogi) Date: 2009-07-17 08:37
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?
msg91873 - (view) Author: Rogi (Rogi) Date: 2009-08-22 20:07
bump
msg91874 - (view) Author: Raphaela (Rakeka) Date: 2009-08-22 20:15
bump =op
msg91983 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2009-08-26 22:18
Can you propose a patch fixing this issue?
msg92041 - (view) Author: Rogi (Rogi) Date: 2009-08-28 19:08
I will try to fix and submit a patch. Just a second =op
msg132375 - (view) Author: Mark Hammond (mhammond) * (Python committer) Date: 2011-03-27 22:21
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.
msg132379 - (view) Author: Rogi (Rogi) Date: 2011-03-27 23:55
@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.
msg132383 - (view) Author: Mark Hammond (mhammond) * (Python committer) Date: 2011-03-28 07:02
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.
msg132399 - (view) Author: Eric Frederich (eric.frederich) Date: 2011-03-28 13:28
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.
msg132407 - (view) Author: Rogi (Rogi) Date: 2011-03-28 17:00
Sorry, some years have passed since I posted this. I will take a look on teh
code again.
msg132413 - (view) Author: Rogi (Rogi) Date: 2011-03-28 19:05
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().
msg132430 - (view) Author: Mark Hammond (mhammond) * (Python committer) Date: 2011-03-28 21:59
@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.
msg132446 - (view) Author: Rogi (Rogi) Date: 2011-03-29 00:10
@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.
msg132450 - (view) Author: Rogi (Rogi) Date: 2011-03-29 00:32
@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.
msg132452 - (view) Author: Mark Hammond (mhammond) * (Python committer) Date: 2011-03-29 01:26
@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.
msg132461 - (view) Author: Rogi (Rogi) Date: 2011-03-29 04:15
@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.
msg132463 - (view) Author: Mark Hammond (mhammond) * (Python committer) Date: 2011-03-29 04:21
> 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.
msg132464 - (view) Author: Rogi (Rogi) Date: 2011-03-29 04:38
Fair. Still I dont liek it very much. I will return!
msg132465 - (view) Author: Mark Hammond (mhammond) * (Python committer) Date: 2011-03-29 04:40
For completeness, here is a doc patch against 2.6 which corrects the documentation.
msg132466 - (view) Author: Rogi (Rogi) Date: 2011-03-29 06:57
I may be wrong, but I think Py_Main() will _never_ return 1.
msg132467 - (view) Author: Rogi (Rogi) Date: 2011-03-29 07:01
Oh, and other functions on this doc, such as PyRun_SimpleStringFlags(),
have this same documentation problem about SystemError.
msg132468 - (view) Author: Mark Hammond (mhammond) * (Python committer) Date: 2011-03-29 07:07
It will return 1 if you specify a script to run and that has an unhandled exception.
msg132469 - (view) Author: Mark Hammond (mhammond) * (Python committer) Date: 2011-03-29 07:27
Good catch - new patch with PyRun_SimpleStringFlags() corrected too.
msg132498 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2011-03-29 18:53
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 #5227.
msg132499 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2011-03-29 18:56
The updated doc patch is missing :exc: markup on one of "SystemExit"s.
msg132527 - (view) Author: Andreas Stührk (Trundle) Date: 2011-03-29 20:57
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.
msg136010 - (view) Author: Roundup Robot (python-dev) Date: 2011-05-15 06:51
New changeset 0311f62714f7 by Georg Brandl in branch '3.1':
Closes #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 #6498 fix from 3.1.
http://hg.python.org/cpython/rev/7089afd69a1a

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

New changeset 97a75fccd7c8 by Georg Brandl in branch '2.7':
Port #6498 fix: fix several misspellings of "SystemExit" as "SystemError".
http://hg.python.org/cpython/rev/97a75fccd7c8
History
Date User Action Args
2011-05-15 06:51:41python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg136010

resolution: fixed
stage: resolved
2011-03-31 17:53:27eric.araujosetversions: + Python 2.7, - Python 2.6
2011-03-29 20:57:57Trundlesetfiles: + issue6498_31.patch
2011-03-29 20:57:32Trundlesetfiles: + issue6498_27.patch
nosy: + Trundle
messages: + msg132527

2011-03-29 18:56:01belopolskysetmessages: + msg132499
2011-03-29 18:53:48belopolskysetnosy: + belopolsky
messages: + msg132498
2011-03-29 07:27:08mhammondsetfiles: + bug-6498.patch
keywords: + patch
messages: + msg132469
2011-03-29 07:07:35mhammondsetmessages: + msg132468
2011-03-29 07:01:00Rogisetmessages: + msg132467
2011-03-29 06:57:19Rogisetmessages: + msg132466
2011-03-29 04:40:59mhammondsetkeywords: + needs review, - patch
2011-03-29 04:40:05mhammondsetfiles: + bug-6498.patch
keywords: + patch
messages: + msg132465
2011-03-29 04:38:28Rogisetmessages: + msg132464
2011-03-29 04:21:19mhammondsetmessages: + msg132463
2011-03-29 04:15:33Rogisetmessages: + msg132461
2011-03-29 01:26:07mhammondsetmessages: + msg132452
2011-03-29 00:32:36Rogisetmessages: + msg132450
2011-03-29 00:10:07Rogisetmessages: + msg132446
2011-03-28 21:59:32mhammondsetmessages: + msg132430
2011-03-28 19:05:56Rogisetmessages: + msg132413
2011-03-28 18:37:05santoso.wijayasetnosy: + santoso.wijaya
2011-03-28 17:00:45Rogisetmessages: + msg132407
2011-03-28 13:28:43eric.frederichsetnosy: + eric.frederich
messages: + msg132399
2011-03-28 07:02:42mhammondsetmessages: + msg132383
2011-03-27 23:55:54Rogisetmessages: + msg132379
2011-03-27 22:21:59mhammondsetnosy: + mhammond
messages: + msg132375
2010-10-29 10:07:21adminsetassignee: georg.brandl -> docs@python
2009-08-28 19:08:23Rogisetmessages: + msg92041
2009-08-26 22:18:57hayposetnosy: + haypo
messages: + msg91983
2009-08-22 20:15:36Rakekasetmessages: + msg91874
2009-08-22 20:07:43Rogisetmessages: + msg91873
2009-07-17 08:37:07Rogisetmessages: + msg90614
2009-07-17 02:32:29Rakekasetnosy: + Rakeka
messages: + msg90601
2009-07-16 23:30:21Rogicreate