From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23435 invoked by alias); 28 Aug 2014 11:22:54 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 23424 invoked by uid 89); 28 Aug 2014 11:22:53 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Thu, 28 Aug 2014 11:22:47 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s7SBMiDC022717 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Thu, 28 Aug 2014 07:22:44 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s7SBMgb3013846; Thu, 28 Aug 2014 07:22:43 -0400 Message-ID: <53FF1102.8050709@redhat.com> Date: Thu, 28 Aug 2014 11:22:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.7.0 MIME-Version: 1.0 To: Patrick Palka CC: gdb-patches@sourceware.org Subject: Re: [PATCH] Fix the processing of Meta-key commands in TUI References: <1408740286-29326-1-git-send-email-patrick@parcs.ath.cx> <53FE0FF9.9010008@redhat.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2014-08/txt/msg00598.txt.bz2 On 08/27/2014 07:25 PM, Patrick Palka wrote: > On Wed, 27 Aug 2014, Pedro Alves wrote: >> The main reason I think there's a larger problem here, is that >> if curses is reading more than one char from stdin, then that means >> that is must be blocking for a bit waiting for the next character, >> which is a no-no in an async world, where we want to be processing >> target events at the same time. The man page says: >> >> While interpreting an input escape sequence, wgetch sets a timer while waiting for the next character. If notimeout(win, TRUE) is called, then wgetch does not >> set a timer. The purpose of the timeout is to differentiate between sequences received from a function key and those typed by a user. >> >> Looks like there's a default timeout of 1 second. Indeed if I set a >> breakpoint in wgetch and another right after wgetch is called, and >> then I press escape, I see that gdb is stuck inside wgetch for around >> one second. During that time, gdb's own event loop isn't being processed. >> >> Not sure exactly how this is usually handled. Seems like there >> are several knobs that might be able to turn this delay off. >> Sounds like we should enable that (whatever the option is), >> and handle the timeout ourselves? > > I don't think the timeout is the issue here. Even if the timeout is > disabled via notimeout(), wgetch() will still attempt to interpret keypad > sequences by reading multiple characters from stdin -- except that the > read will now be a non-blocking one instead of a blocking one. > > One way or another, someone must read multiple keys from stdin in order > to semantically distinguish between keypad keys and regular key > sequences. And when it turns out that the input is not or cannot be a > keypad key then that someone must place the extraneous keys into a > buffer and notify GDB's event handler that we missed their stdin events. Right, that's a given. What I was talking about is fixing the 1 second block in case the input stops before a sequence is complete. > If we handle the timeout ourselves (for instance by disabling keypad() > and enabling notimeout()) then we'll be responsible for doing the > lookahead, interpreting the sequences and buffering the keypresses. I > say let ncurses continue to handle the timeout so that we'll only be > responsible for notifying the event handler. > > Though I may just be misunderstanding your proposal. The main idea was to not let ncurses ever block, as that prevents gdb's event loop from handling target events. If ncurses internally already handled the timeout by comparing the time between wgetch calls instead of doing a blocking select/poll internally, then it'd be a bit easier, but it looks like it doesn't (GetEscdelay always starts with the window's configured delay, on each wgetch call?), so we'd need to track the timeout ourselves. Even if it did, it wouldn't be that different though. What we'd need is: #1 - set ncurses to be _always_ non-blocking/no-delay. #2 - on input, call wgetch, as today. #3 - due to no-delay, that now only looks ahead for as long as there are already bytes ready to be read from the input file / stdin. #4 - if the sequence looks like a _known_ escape sequence, but it isn't complete yet, then wgetch leaves already-read bytes in the fifo, and returns ERR. That's the "the keys stay uninterpreted" comment in lib_getch.c. #5 - at this point, we need to wait for either: (a) further input, in case further input finishes the sequence. (b) the timeout to elapse, meaning no further input, and we should pass the raw chars to readline. For #5/(a), there's nothing to do, that's already what the stdin handler does. For #5/(b), because we don't want to block in the stdin handler (tui_getc) blocked waiting for the timeout, we would instead install a timer in gdb's event loop whose callback was just be the regular TUI stdin input handler. This time, given enough time had elapsed with no further input, we want the raw chars. If ncurses internally knows that sufficient time has passed, then good, we only have to call wgetch, and we know ncurses gives us the raw keys (the incomplete sequence). If it doesn't (looks like it doesn't, but I may be wrong), then we just temporarily switch off the keypad, and read one char, which returns us the raw escape char at the head of the fifo, and leaves the rest in the fifo for subsequent reads. As the fifo is now missing the escape char, we can go back to normal, with the keypad enabled, and the next time we call wgetch should return us the head of the fifo immediately, if there's anything there. Going back to step #4, in case the sequence is _unknown_ or the timeout has expired: #4.2 - if the sequence in the fifo is definitely an _unknown_ escape sequence, wgetch returns the first char in the fifo, leaving the remainder of the bytes in the fifo. TBC, in this case, we _don't_ get back ERR. As there are more bytes in the fifo, then we need to compensate for the missed stdin events, like in your patch. (*) (*) - so it sounds your patch would be necessary anyway. Oddly, even when doing: nodelay (w, TRUE); notimeout (w, TRUE); in tui_getc, I _still_ get that a one second block within wgetch... Looks like related to mouse event handling, even though mouse events were not enabled...: (top-gdb) bt #0 0x000000373bcea9c0 in __poll_nocancel () at ../sysdeps/unix/syscall-template.S:81 #1 0x00007f62bc49d43d in _nc_timed_wait (sp=0x18f7730, mode=3, milliseconds=1000, timeleft=0x0) at ../../ncurses/tty/lib_twait.c:265 #2 0x00007f62bc6bf484 in check_mouse_activity (sp=0x18f7730, delay=1000) at ../../ncurses/base/lib_getch.c:145 #3 0x00007f62bc6c00c8 in kgetch (sp=0x18f7730) at ../../ncurses/base/lib_getch.c:734 #4 0x00007f62bc6bfbec in _nc_wgetch (win=0x193e6a0, result=0x7fffdf59f2c8, use_meta=1) at ../../ncurses/base/lib_getch.c:513 #5 0x00007f62bc6bfe6e in wgetch (win=0x193e6a0) at ../../ncurses/base/lib_getch.c:643 #6 0x0000000000516d88 in tui_getc (fp=0x373bfb9640 <_IO_2_1_stdin_>) at /home/pedro/gdb/mygit/src/gdb/tui/tui-io.c:662 #7 0x000000000078385f in rl_read_key () at /home/pedro/gdb/mygit/src/readline/input.c:448 #8 0x000000000076b5a8 in _rl_subseq_getchar (key=27) at /home/pedro/gdb/mygit/src/readline/readline.c:658 #9 0x000000000076b5fb in _rl_dispatch_callback (cxt=0x1661190) at /home/pedro/gdb/mygit/src/readline/readline.c:680 #10 0x0000000000783d73 in rl_callback_read_char () at /home/pedro/gdb/mygit/src/readline/callback.c:170 #11 0x0000000000618ba9 in rl_callback_read_char_wrapper (client_data=0x0) at /home/pedro/gdb/mygit/src/gdb/event-top.c:167 #12 0x0000000000618f7f in stdin_event_handler (error=0, client_data=0x0) at /home/pedro/gdb/mygit/src/gdb/event-top.c:373 #13 0x0000000000617b72 in handle_file_event (data=...) at /home/pedro/gdb/mygit/src/gdb/event-loop.c:763 #14 0x0000000000617059 in process_event () at /home/pedro/gdb/mygit/src/gdb/event-loop.c:340 #15 0x0000000000617120 in gdb_do_one_event () at /home/pedro/gdb/mygit/src/gdb/event-loop.c:404 #16 0x0000000000617170 in start_event_loop () at /home/pedro/gdb/mygit/src/gdb/event-loop.c:429 If such delays/blocks can't be eliminated due to buggy ncurses, or something missing in the APIs, then it looks like the only way to fix this would be to move the wgetch call to a separate thread, like, we'd create a pipe, and put one end in the event loop as stdin source instead of the real stdin, and then the separate thread would push the results of wgetch into this pipe... Thanks, Pedro Alves