classification
Title: Python doesn't handle SIGINT well if it arrives during interpreter startup
Type: behavior Stage:
Components: Interpreter Core Versions: Python 2.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: benjamin.peterson, gjb1002, haypo
Priority: normal Keywords: patch

Created on 2008-06-19 08:04 by gjb1002, last changed 2010-06-14 23:14 by haypo. This issue is now closed.

Files
File name Uploaded Description Edit
SIGINT-2.patch haypo, 2010-03-08 23:29
Messages (19)
msg68392 - (view) Author: Geoffrey Bache (gjb1002) Date: 2008-06-19 08:04
If a python script receives SIGINT while the interpreter is starting up,
it's possible to get the message "import site failed; use -v for
traceback" printed on standard error and for execution to proceed. It
also seems to be possible to get half-imported modules and for the
script to fail later claiming that something like "os.getenv" doesn't exist.

If I do as instructed and use -v for traceback I get something like:

'import site' failed; traceback:
Traceback (most recent call last):
  File "/usr/lib/python2.4/site.py", line 61, in ?
    import os
  File "/usr/lib/python2.4/os.py", line 683, in ?
    import copy_reg as _copy_reg
  File "/usr/lib/python2.4/copy_reg.py", line 5, in ?
    """
KeyboardInterrupt 

I imagine there exists some code like
try:
    import site
except:
    sys.stderr.write("import site failed; use -v for traceback\n")

though I couldn't find any. If so, it seems clear that KeyboardInterrupt
needs to be re-raised, or Python's special handler for SIGINT installed
rather later.
msg68402 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-06-19 12:57
Yes, the C startup code in pythonrun.c has a lot of bare-except statements.
msg100617 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2010-03-08 00:10
I think initsite() should be atomic: if any kind of error occurs, Python should print it and exit directly (exit code 1 to notice the error). If initsite() fails, Python is not complelty initialized.

site is responsible to initialiaze a lot of things:

 - Set all module' __file__ attribute to an absolute path
 - Remove duplicate entries from sys.path along with making them absolute
 - Add a per user site-package to sys.path
 - Add site-packages (and possibly site-python) to sys.path
 - Define new built-ins 'quit' and 'exit'.
 - Set 'copyright' and 'credits' in __builtin__"
 - Create builtin help() function
 - On Windows, some default encodings are not provided by Python, while they are always available as "mbcs" in each locale. Make them usable by aliasing to "mbcs" in such a case.
 - import sitecustomize
 - import usercustomize
 - del sys.setdefaultencoding
 - etc.

Be able to ignore the site initializations might be a security vulnerability.

Attached patch consider any exception as fatal: display the error and exit. I moved the call to _PyGILState_Init() before initsite() to avoid a crash in Py_Finalize() => see #8063.
msg100619 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2010-03-08 00:36
Using "valgrind --log-file=/tmp/trace ./python" to slow down Python, I found another bug in the interactive interpreter: if you press CTRL+c just after the initialization of the site module the exception will be ignored.

When a signal is catched, it's stored in a table, and the real handler is called later (by Py_MakePendingCalls()). Py_AddPendingCall() is called to ensure that the signal will be handled before the next Python instruction. In my case, the next Python instruction is "decode the string written by the user using the terminal encoding" called by the tokenizer... but the tokenizer ignores all errors (not only unicode errors): see tok_stdin_decode().

Recipe to catch the code responsible to ignore the keyboard interrupt exception (using gdb):
---------------------
$ gdb ./python
(gdb) b initsite
Breakpoint 1, initsite ()
(gdb) run
...
Breakpoint 1, initsite ()
(gdb) next
<execution of the site module>
(gdb) b signal_default_int_handler
Breakpoint 2
(gdb) signal SIGINT
Breakpoint 2, signal_default_int_handler
(gdb) b PyErr_Clear
Breakpoint 3
(gdb) cont
...
Breakpoint 3, PyErr_Clear ()
(gdb) where
<HERE YOU HAVE>
---------------------

Attached patch calls Py_MakePendingCalls() before PyRun_AnyFileExFlags() to avoid the tokenizer bug. It's just a workaround, not a real bugfix.
msg100636 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2010-03-08 12:04
PyImport_Import() clears errors if a module has no globals, whereas PyEval_GetGlobals() doesn't set any exception on failure. => import_no_global.patch removes this unnecessary PyErr_Clear().
msg100637 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2010-03-08 12:10
Py_InitializeEx() calls PyErr_Clear() if PyCodec_Encoder() fails without checking for the exception type.

Attached initiliaze_pycodec_error.patch checks for error type: if the error is not an LookupError, display the error and exit (as done for initsite() iny my initsite.patch).
msg100638 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2010-03-08 12:21
tok_stdin_decode() clears error without check for the type. 

tokenizer_error.patch stops the tokenizer with an E_ERROR if the exception is not an UnicodeDecodeError. Fix also err_input() to support E_ERROR: leave the global exception (PyErr_*) unchanged.
msg100639 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2010-03-08 12:23
Remove main_pending_calls.patch hack: replaced by tokenizer_error.patch.
msg100640 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2010-03-08 13:00
> Remove main_pending_calls.patch hack: replaced by tokenizer_error.patch.

With extra tests, I realized that this patchs is still useful to process a SIGINT emitted between Py_Initialize() and PyRun_AnyFileExFlags(). Without the patch, if a SIGINT is emitted just after Py_Initialize(): it's only proceed when the user press enter in the interactive interpreter (when the tokenizer decodes the input string to the terminal charset).
msg100667 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2010-03-08 21:19
SIGINT.patch is the sum of all patches, it should be easier to review it.
msg100682 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2010-03-08 23:29
As requested by Guido, here is a new patch including the fix for the site module: catch errors in execsitecustomize() and execusercustomize().
msg100814 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2010-03-10 22:58
Commited to trunk: r78826 + r78827. I will later backport to py3k.
msg100825 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2010-03-11 01:12
My change about site initialization error raised 2 old issues: #7774 and #7880. flox commited a fix for #7880 and a workaround for #7774 (just enough to fix test_subprocess).
msg100935 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2010-03-12 14:48
> Commited to trunk: r78826

Backported to py3k as r78872. But py3k will require extra work: there are some PyErr_Clear() somewhere, eating the errors.
msg100940 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2010-03-12 16:10
Bug related to this issue: #8124, PySys_WriteStdout() and PySys_WriteStderr() ignore signal handlers errors.
msg101426 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2010-03-21 14:02
> Commited to trunk: r78826 + r78827

Partial backport to 2.6 as r79204: leave import site error handler unchanged (print the error and continue). I don't want to change Python behaviour between minor releases.
msg101462 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2010-03-21 21:50
> Backported to py3k as r78872

And backported to 3.1 as r79247.

> But py3k will require extra work: there are some PyErr_Clear() somewhere, eating the errors.

Leave this issue open until #8124 is fixed.
msg101466 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2010-03-21 22:10
>> Backported to py3k as r78872
> And backported to 3.1 as r79247.

I reverted the change on initsite(): as for Python 2.6, I don't want to change import site error handler between minor releases.
msg107841 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2010-06-14 23:14
> Bug related to this issue: #8124, PySys_WriteStdout() and PySys_WriteStderr() ignore signal handlers errors.
> ...
> Leave this issue open until #8124 is fixed.

I fixed both issues, so it's time to close this one.
History
Date User Action Args
2010-06-14 23:14:35hayposetstatus: open -> closed
resolution: fixed
messages: + msg107841
2010-03-21 22:10:51hayposetmessages: + msg101466
2010-03-21 21:50:12hayposetmessages: + msg101462
2010-03-21 14:02:58hayposetmessages: + msg101426
2010-03-12 16:10:16hayposetmessages: + msg100940
2010-03-12 14:48:05hayposetmessages: + msg100935
2010-03-11 01:12:03hayposetmessages: + msg100825
2010-03-10 22:58:28hayposetmessages: + msg100814
2010-03-08 23:29:57hayposetfiles: - SIGINT.patch
2010-03-08 23:29:53hayposetfiles: - main_pending_calls.patch
2010-03-08 23:29:49hayposetfiles: - import_no_global.patch
2010-03-08 23:29:45hayposetfiles: - tokenizer_error.patch
2010-03-08 23:29:41hayposetfiles: - initiliaze_pycodec_error.patch
2010-03-08 23:29:38hayposetfiles: - initsite.patch
2010-03-08 23:29:30hayposetfiles: + SIGINT-2.patch

messages: + msg100682
2010-03-08 21:19:42hayposetfiles: + SIGINT.patch

messages: + msg100667
2010-03-08 13:00:16hayposetfiles: + main_pending_calls.patch

messages: + msg100640
2010-03-08 12:25:28hayposetfiles: + import_no_global.patch
2010-03-08 12:23:14hayposetmessages: + msg100639
2010-03-08 12:23:10hayposetfiles: - main_pending_calls.patch
2010-03-08 12:21:49hayposetfiles: + tokenizer_error.patch

messages: + msg100638
2010-03-08 12:10:26hayposetfiles: + initiliaze_pycodec_error.patch

messages: + msg100637
2010-03-08 12:04:59hayposetmessages: + msg100636
2010-03-08 00:36:30hayposetfiles: + main_pending_calls.patch

messages: + msg100619
2010-03-08 00:10:18hayposetfiles: + initsite.patch

nosy: + haypo
messages: + msg100617

keywords: + patch
2008-06-19 12:57:35benjamin.petersonsettype: behavior
messages: + msg68402
nosy: + benjamin.peterson
2008-06-19 08:04:36gjb1002create