From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6940 invoked by alias); 28 Aug 2014 17:41:57 -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 6931 invoked by uid 89); 28 Aug 2014 17:41:57 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.6 required=5.0 tests=AWL,BAYES_00,COMPENSATION,RCVD_IN_DNSWL_LOW autolearn=no version=3.3.2 X-HELO: mail-pa0-f44.google.com Received: from mail-pa0-f44.google.com (HELO mail-pa0-f44.google.com) (209.85.220.44) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Thu, 28 Aug 2014 17:41:55 +0000 Received: by mail-pa0-f44.google.com with SMTP id rd3so3527477pab.3 for ; Thu, 28 Aug 2014 10:41:54 -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=zWnEMEqNctyztPj+ab56S9fIMrUuIkZiJrwcDqbJ0qc=; b=UjdOpm41qEzTTwEvVnWV+FP/XYy6Tkx5ZozYGFv2inSWUEr1kkOSJadbDmMVPBq89Q CUDNjjB3haFY/PfztNNowGEmd2rJYfcSiLgbMlBOYqt1bCIlYOcNwxMhjJxgvu862+1q 6CaMm6Jhzn2r8Jo78pGmPRdtmAccYpDdTZmJlLnCSLL4hGMA3AJFBBmYkeSJxByQaEow 9KZWVYwE13uHcrISDeBX4zB/xm4TSW3J+WFSc81AbWfMKE+LgeQRTkQxTHTrxxchgVhy HunTHLV0w5c2ZWjhrXrL1u/AzKIHWoShUKP7FU/rZRQPoGYcMZghRSQtdJkEYH1fEc3z shqg== X-Gm-Message-State: ALoCoQlHJU7WQo/826OD/uo1yDjvILXqZ4JAyDWLoBu2PpcvI8MX6ugXA205544JZ2NeTfJlvm7H MIME-Version: 1.0 X-Received: by 10.70.87.199 with SMTP id ba7mr7971258pdb.121.1409247713957; Thu, 28 Aug 2014 10:41:53 -0700 (PDT) Received: by 10.70.5.229 with HTTP; Thu, 28 Aug 2014 10:41:53 -0700 (PDT) In-Reply-To: References: <1408740286-29326-1-git-send-email-patrick@parcs.ath.cx> <53FE0FF9.9010008@redhat.com> <53FF1102.8050709@redhat.com> Date: Thu, 28 Aug 2014 17:41: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/msg00625.txt.bz2 On Thu, Aug 28, 2014 at 10:13 AM, Patrick Palka wrote: > 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. > > Is the internal timeout a big deal though? The handling of the target > event just gets temporarily delayed, not missed entirely, right? And > isn't this timeout only experienced when one presses ESC (which has no > use in TUI) and/or attempts to manually type a function key sequence? > I'm not sure why anyone would do that. > >> >> 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. > > Not sure it's possible to make wgetch() behave this way. From what I > can tell, wgetch() will always return the key from its fifo if there's > one available -- it won't check whether the fifo contents + a new key > from stdin will make a complete sequence. It won't even read from > stdin if there's a key in the fifo. > >> (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...: > > Yeah same here. I can't seem to find the magical invocation that > actually disables this timeout. So it looks like the (non-standard) ncurses variable ESCDELAY is what controls the timeout: wgetch() sets the timeout to ESCDELAY milliseconds. So do you suppose that we should set this variable