classification
Title: raw_input + readline: Ctrl+C during search breaks readline
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 3.6, Python 3.5, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Arfrever, martin.panter, pitrou, python-dev, r.david.murray, serhiy.storchaka, sping, tcaswell, terry.reedy, twouters
Priority: normal Keywords: patch

Created on 2015-05-22 14:58 by sping, last changed 2016-03-23 18:07 by serhiy.storchaka. This issue is now closed.

Files
File name Uploaded Description Edit
raw_input__workaround_demo.py sping, 2015-05-22 14:58
raw_input__minimal.py sping, 2015-05-22 14:58
readline-cancel.patch martin.panter, 2015-05-27 07:08 Workaround in readline module review
readline-sigcleanup.patch martin.panter, 2015-07-29 12:58 Use new Readline 7 function review
Messages (17)
msg243828 - (view) Author: (sping) Date: 2015-05-22 14:58
Hi!


A college of mine ran into a bug with raw_input.
We have a shell derived from stdlib module "cmd" here but the bug shows
with plain raw_input, as well (see demo code).

For the symptoms: the shell is executing commands from history that the
user explicitly chose not to run.  Since our shell does things like
deallocation of LVM volumes, that behavior is rather troublesome :)

How does it happen?

 * When pressing Ctrl+R, an incremental reverse history search is started.
   During that search, you normalled press Escape/Ctrl+J/Ctrl+G to end
   search or Return to pick a result.

 * When you press Ctrl+C though, the next call to the raw_input function
   believes to be in search mode still (while not showing "(reverse-i-search)"
   or indicating search some way).

 * Now when (entering some text and) pressing return now, the string last
   shown during incremental search is being return from raw_input
   (and executed in context of the cmd module).

I have re-produced the issue with Python 2.7.3, 2.7.9, 3.2.3, 3.4.2.

For a workaround, one can handle KeyboardInterrupt and internal adjust
variable rl_readline_state of the C readline library.
I'm attaching (a minimal bug demo and) the ctypes based workaround that
works well over here.
(The workaround demo also shows that readline state is not fully reset
when Ctrl+C was pressed outside of search mode, since flag RL_STATE_DONE
is not set after.)

It would rock the house if this could be fixed in Python.

I'm looking forward to your reply.

Best,



Sebastian
msg243844 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2015-05-22 18:37
3.2 and 3.3 only get security fixes.  I condensed the title to be visible in display boxes.  It would help if you signed the PSF Contributor Agreement
https://www.python.org/psf/contrib/
https://www.python.org/psf/contrib/contrib-form/
and contributed a usable patch for test_readline under that agreement.
msg243846 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-05-22 18:44
This behavior annoyed me long time, but I had no idea how to fix this.
msg243864 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-05-22 23:15
Also applies to the interactive interpreter mode.
msg243975 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-05-24 10:05
I suspect this is actually a bug with Gnu Readline. The same issue exists in GDB.

When a signal handler (such as SIGINT) raises an exception to abort the input, Python calls rl_free_line_state(). The documentation for that function, <https://cnswww.cns.cwru.edu/php/chet/readline/readline.html#IDX341> says Readline’s own SIGINT handler (which Python doesn’t normally actually use) also calls it to abort the current line. It lists various states that are “freed”. Although the search state is not in the list, it seems like it should be.
msg244146 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-05-27 07:08
I reported this to Gnu Readline: <https://lists.gnu.org/archive/html/bug-readline/2015-05/msg00014.html>. So far it sounds like there is no general solution. I am posting a patch implementing Sebastian’s workaround. But my personal philosophy is to leave Python alone if it is not doing anything wrong and this is indeed a Readline bug.

Chet also pointed out that there are other modes in addition to incremental search that are not cancelled. For instance, the “argument” mode where you press Alt+<number> etc.
msg244680 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2015-06-02 17:37
The patch is two lines, but I do not know our general policy for working around bugs in external modules.  I do know that Idle has workarounds for tk bugs (or 'os-dependencies').
msg244682 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2015-06-02 17:47
I believe that in general we do do workarounds if they don't make the code too complicated and upstream hasn't solved the problem (or we need to support older upstream versions).
msg244683 - (view) Author: (sping) Date: 2015-06-02 18:17
I guess supporting older upstream versions would be nice in this case.
msg247571 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-07-29 12:58
Readline 7.0 alpha version includes a new “feature” that should help:

i.  rl_callback_sigcleanup: a new application function that can clean up and unset any state set by readline’s callback mode.  Intended to be used after a signal.

Patch readline-sigcleanup.patch uses this function, depending on a compile-time version check. It fixes the bug when compiled against Readline 7.
msg259311 - (view) Author: Thomas Caswell (tcaswell) * Date: 2016-02-01 04:43
I do not think that readline-cancel.patch is sufficient. The clean up function that readline uses internally (http://git.savannah.gnu.org/cgit/readline.git/tree/isearch.c#n720 ) also cleans up the context that used by isearch.

The functions to cleanup the context (and a pointer to the context) are exposed on `rlprivate.h` so python _can_ get at them to support old versions of rl. Once started down that path for isearch mode, might as well vendor all of rl_callback_sigcleanup() which will end up being 50-100 LoC (some of the helper functions will also need to be vendored).
msg259313 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-02-01 04:51
I think it might be best to wait until we are sure that the new Readline 7 API is stable (Is already the case?), push something like readline-sigcleanup.patch, and don’t bother with hacks for Readline 6 (or Editline etc).
msg259314 - (view) Author: Thomas Caswell (tcaswell) * Date: 2016-02-01 05:42
For reference https://github.com/ludwigschwardt/python-gnureadline/pull/47 is what a back-port of the functionality looks like.
msg262167 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-03-22 07:31
I’m getting ready to commit my patch to support the new Readline 7 function. Is there anything else to be done? The other options don’t sound worth it to me (e.g. incomplete, depending on internal details, etc).
msg262176 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-03-22 11:18
New changeset af6e8e1d15fa by Martin Panter in branch '3.5':
Issue #24266: Cancel history search mode with Ctrl+C in Readline 7
https://hg.python.org/cpython/rev/af6e8e1d15fa

New changeset 322d30816d36 by Martin Panter in branch 'default':
Issue #24266: Merge readline Ctrl+C handling from 3.5
https://hg.python.org/cpython/rev/322d30816d36

New changeset 1c0b29441116 by Martin Panter in branch '2.7':
Issue #24266: Cancel history search mode with Ctrl+C in Readline 7
https://hg.python.org/cpython/rev/1c0b29441116
msg262213 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2016-03-22 21:49
Thanks for fixing this!
msg262294 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-03-23 18:07
Thank you Martin.
History
Date User Action Args
2016-03-23 18:07:26serhiy.storchakasetmessages: + msg262294
2016-03-22 21:49:27pitrousetmessages: + msg262213
2016-03-22 21:47:14martin.pantersetstatus: open -> closed
resolution: fixed
stage: commit review -> resolved
2016-03-22 11:18:01python-devsetnosy: + python-dev
messages: + msg262176
2016-03-22 07:31:55martin.pantersetstage: patch review -> commit review
messages: + msg262167
versions: - Python 3.4
2016-02-01 05:42:58tcaswellsetmessages: + msg259314
2016-02-01 04:51:56martin.pantersetmessages: + msg259313
2016-02-01 04:43:36tcaswellsetnosy: + tcaswell
messages: + msg259311
2015-07-29 12:58:52martin.pantersetfiles: + readline-sigcleanup.patch

messages: + msg247571
stage: test needed -> patch review
2015-06-02 19:14:35Arfreversetnosy: + Arfrever
2015-06-02 18:17:25spingsetmessages: + msg244683
2015-06-02 17:47:07r.david.murraysetnosy: + r.david.murray
messages: + msg244682
2015-06-02 17:37:51terry.reedysetmessages: + msg244680
2015-05-27 07:08:58martin.pantersetfiles: + readline-cancel.patch
keywords: + patch
messages: + msg244146
2015-05-24 10:05:37martin.pantersetmessages: + msg243975
2015-05-22 23:15:10martin.pantersetnosy: + martin.panter
messages: + msg243864
2015-05-22 18:44:26serhiy.storchakasetnosy: + pitrou, serhiy.storchaka
messages: + msg243846
2015-05-22 18:37:42terry.reedysetnosy: + terry.reedy, twouters
title: raw_input function (with readline): Ctrl+C (during search mode but not only) leaves readline in broken state -> raw_input + readline: Ctrl+C during search breaks readline
messages: + msg243844

versions: - Python 3.2, Python 3.3
stage: test needed
2015-05-22 14:58:32spingsetfiles: + raw_input__minimal.py
2015-05-22 14:58:03spingcreate