From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32392 invoked by alias); 28 Aug 2014 15:59: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 32376 invoked by uid 89); 28 Aug 2014 15:59:55 -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 15:59:54 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s7SFxpjd030116 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Thu, 28 Aug 2014 11:59:51 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s7SFxoAl004802; Thu, 28 Aug 2014 11:59:50 -0400 Message-ID: <53FF51F5.80506@redhat.com> Date: Thu, 28 Aug 2014 15:59: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> <53FF1102.8050709@redhat.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2014-08/txt/msg00621.txt.bz2 On 08/28/2014 03:13 PM, 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. Right. TBC, I noticed/found this while ramping up for reviewing this patch, reading the ncurses code and trying to make sense of the whole ncurses/tui integration and your patch. I'm not _that_ familiar with this code, although I'm probably one of the most familiar nowadays... My main motivation for pointing this out was to brainstorm. I had no idea if there was a canned solution for this, which your or someone might know about. :-) I wasn't really sure of the complexity of the solution until I spent a few more hours this morning working through what it would take and write out those steps list... > Yeah same here. I can't seem to find the magical invocation that > actually disables this timeout. Thanks for poking. >> 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... > > I am not sure that it's possible to eliminate the internal timeout > completely so it seems to me that your thread-based solution may be > necessary to fix this. But is it worth the complexity to fix this > seemingly obscure issue? Yeah, we certainly shouldn't let the perfect be the enemy of the good. I'd think using vi-mode keybindings would need it, but according to PR11383 those don't work anyway. The thread-based solution would make your patch unnecessary, but then again, that's easy to revert if a thread-based solution ends up written. I think we understand the problems and the effort required for the potential solutions enough to be able to firmly say: yes, let's proceed with your solution. Thanks, Pedro Alves