From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10765 invoked by alias); 28 Aug 2014 13:10:48 -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 10736 invoked by uid 89); 28 Aug 2014 13:10:48 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL,BAYES_00,COMPENSATION,RCVD_IN_DNSWL_LOW autolearn=no version=3.3.2 X-HELO: mail-pa0-f50.google.com Received: from mail-pa0-f50.google.com (HELO mail-pa0-f50.google.com) (209.85.220.50) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Thu, 28 Aug 2014 13:10:46 +0000 Received: by mail-pa0-f50.google.com with SMTP id kq14so2515818pab.37 for ; Thu, 28 Aug 2014 06:10:43 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=05EZVkD8aJD7uXprA8uDo+T0xqJHvWd92lUTBU4A/3U=; b=Mq95i8SexoSsxPE4DcNRqkrdpF0VuUrASsgSpJsGIIwpo0TTwf3YZ2HmA4N/4sYOgW 8LvyCBvFQap67Hg3U/bZpqTVYiWTEU+oI2/dE4q05e3jNJrQxA+fRXodnwMlnjOYBzHR MhUZha6Cm/ykOfyOKJ4aNrEOKuDx04svekZFVYI9V4ZAL5zz3mRxpAgTEx9RssdGtE7B yjGvmz2kR5NayFJSLjUSXGxXIKZk1LYH/jjHPiYw4wnNyoYFzRrMl4bGsJRmYZF/3l1Y W4UXksCuKg+J/FVT8DOF5ZKPsU9UNIGGzKfbCV2txDXZvYIXpfIEQRQIsCvvywOhN51W chMg== X-Gm-Message-State: ALoCoQmGnoUQfcsKSMojUSpER4r60EMa4RrkQuajRv2yaCciQoJRIwFQsQZSWKY0qT3UqM1qyfKj MIME-Version: 1.0 X-Received: by 10.70.87.199 with SMTP id ba7mr5575185pdb.121.1409231443397; Thu, 28 Aug 2014 06:10:43 -0700 (PDT) Received: by 10.70.5.229 with HTTP; Thu, 28 Aug 2014 06:10:43 -0700 (PDT) In-Reply-To: <53FF1102.8050709@redhat.com> References: <1408740286-29326-1-git-send-email-patrick@parcs.ath.cx> <53FE0FF9.9010008@redhat.com> <53FF1102.8050709@redhat.com> Date: Thu, 28 Aug 2014 13:10:00 -0000 Message-ID: Subject: Re: [PATCH] Fix the processing of Meta-key commands in TUI From: Patrick Palka To: Pedro Alves Cc: gdb-patches@sourceware.org Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2014-08/txt/msg00606.txt.bz2 On Thu, Aug 28, 2014 at 7:22 AM, Pedro Alves wrote: > 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 >