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

raw_input + readline: Ctrl+C during search breaks readline #68454

Closed
hartwork mannequin opened this issue May 22, 2015 · 17 comments
Closed

raw_input + readline: Ctrl+C during search breaks readline #68454

hartwork mannequin opened this issue May 22, 2015 · 17 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@hartwork
Copy link
Mannequin

hartwork mannequin commented May 22, 2015

BPO 24266
Nosy @Yhg1s, @terryjreedy, @pitrou, @bitdancer, @vadmium, @serhiy-storchaka, @tacaswell, @hartwork
Files
  • raw_input__workaround_demo.py
  • raw_input__minimal.py
  • readline-cancel.patch: Workaround in readline module
  • readline-sigcleanup.patch: Use new Readline 7 function
  • 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 2016-03-22.21:47:14.534>
    created_at = <Date 2015-05-22.14:58:03.813>
    labels = ['interpreter-core', 'type-bug']
    title = 'raw_input + readline: Ctrl+C during search breaks readline'
    updated_at = <Date 2016-03-23.18:07:26.859>
    user = 'https://github.com/hartwork'

    bugs.python.org fields:

    activity = <Date 2016-03-23.18:07:26.859>
    actor = 'serhiy.storchaka'
    assignee = 'none'
    closed = True
    closed_date = <Date 2016-03-22.21:47:14.534>
    closer = 'martin.panter'
    components = ['Interpreter Core']
    creation = <Date 2015-05-22.14:58:03.813>
    creator = 'sping'
    dependencies = []
    files = ['39467', '39468', '39514', '40054']
    hgrepos = []
    issue_num = 24266
    keywords = ['patch']
    message_count = 17.0
    messages = ['243828', '243844', '243846', '243864', '243975', '244146', '244680', '244682', '244683', '247571', '259311', '259313', '259314', '262167', '262176', '262213', '262294']
    nosy_count = 10.0
    nosy_names = ['twouters', 'terry.reedy', 'pitrou', 'Arfrever', 'r.david.murray', 'python-dev', 'martin.panter', 'serhiy.storchaka', 'tcaswell', 'sping']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue24266'
    versions = ['Python 2.7', 'Python 3.5', 'Python 3.6']

    @hartwork
    Copy link
    Mannequin Author

    hartwork mannequin commented May 22, 2015

    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

    @hartwork hartwork mannequin added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error labels May 22, 2015
    @terryjreedy
    Copy link
    Member

    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.

    @terryjreedy terryjreedy changed the 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 May 22, 2015
    @serhiy-storchaka
    Copy link
    Member

    This behavior annoyed me long time, but I had no idea how to fix this.

    @vadmium
    Copy link
    Member

    vadmium commented May 22, 2015

    Also applies to the interactive interpreter mode.

    @vadmium
    Copy link
    Member

    vadmium commented May 24, 2015

    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.

    @vadmium
    Copy link
    Member

    vadmium commented May 27, 2015

    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.

    @terryjreedy
    Copy link
    Member

    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').

    @bitdancer
    Copy link
    Member

    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).

    @hartwork
    Copy link
    Mannequin Author

    hartwork mannequin commented Jun 2, 2015

    I guess supporting older upstream versions would be nice in this case.

    @vadmium
    Copy link
    Member

    vadmium commented Jul 29, 2015

    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.

    @tacaswell
    Copy link
    Mannequin

    tacaswell mannequin commented Feb 1, 2016

    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).

    @vadmium
    Copy link
    Member

    vadmium commented Feb 1, 2016

    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).

    @tacaswell
    Copy link
    Mannequin

    tacaswell mannequin commented Feb 1, 2016

    For reference ludwigschwardt/python-gnureadline#47 is what a back-port of the functionality looks like.

    @vadmium
    Copy link
    Member

    vadmium commented Mar 22, 2016

    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).

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Mar 22, 2016

    New changeset af6e8e1d15fa by Martin Panter in branch '3.5':
    Issue bpo-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 bpo-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 bpo-24266: Cancel history search mode with Ctrl+C in Readline 7
    https://hg.python.org/cpython/rev/1c0b29441116

    @vadmium vadmium closed this as completed Mar 22, 2016
    @pitrou
    Copy link
    Member

    pitrou commented Mar 22, 2016

    Thanks for fixing this!

    @serhiy-storchaka
    Copy link
    Member

    Thank you Martin.

    @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) type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants