From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31321 invoked by alias); 1 Jul 2008 13:38:20 -0000 Received: (qmail 31305 invoked by uid 22791); 1 Jul 2008 13:38:18 -0000 X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.31) with ESMTP; Tue, 01 Jul 2008 13:37:48 +0000 Received: (qmail 6502 invoked from network); 1 Jul 2008 13:37:46 -0000 Received: from unknown (HELO orlando.local) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 1 Jul 2008 13:37:46 -0000 From: Pedro Alves To: Daniel Jacobowitz Subject: Re: [non-stop] 10/10 split user/internal threads Date: Tue, 01 Jul 2008 13:38:00 -0000 User-Agent: KMail/1.9.9 Cc: gdb-patches@sourceware.org References: <200806152209.45049.pedro@codesourcery.com> <200806300108.49392.pedro@codesourcery.com> <20080701005128.GA10432@caradoc.them.org> In-Reply-To: <20080701005128.GA10432@caradoc.them.org> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200807011437.45222.pedro@codesourcery.com> X-IsSubscribed: yes 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 X-SW-Source: 2008-07/txt/msg00004.txt.bz2 A Tuesday 01 July 2008 01:51:28, Daniel Jacobowitz wrote: > On Mon, Jun 30, 2008 at 01:08:48AM +0100, Pedro Alves wrote: > > - In non-stop mode, the current thread may exit while the > > user has it selected. Switching current thread, and restoring > > it with a cleanup is problematic in non-stop mode. > > The target interface is async, the inferior program is running - but > GDB retains a single thread of control. So unless the inferior runs > while the cleanup is live, there's no problem here. I suppose it > could be trouble if you enter the expression evaluator, though? Sure it can run while the cleanup is live. It could already be running when the cleanup was created. Remember that the selected thread may be running at any time. If the selected thread was not running, but it is set running while the cleanup is live, and we enter the expression evaluator, I don't think there's a problem currently, as returning from infcalls tries to restore the originally selected frame. The other case the current thread may be set running is: 01: foo3() 02: { 03: } 04: 05: foo2() 06: { 07: foo3(); 08: } 09: 10: foo1() 11: { 12: foo2(); 13: } 14: 15: foo_other_thread() 16: { 17: } (gdb) info threads * 1 Thread at foo2 line 7 (gdb) bt #0 0xffffe410 in foo2 () #1 0xf7e4aff6 in foo1 () #2 0xf7e8634c in thread_entry () (gdb) up #1 0xf7e4aff6 in foo1 () (gdb) break foo_other_thread Breakpoint 1 ... (gdb) commands >thread 1 >step& >end The user loses the selected frame in thread 1, because when the cleanup is ran, the thread is running/stepping. Since the thread stepped into another frame, we could claim that since the originally selected frame is still live, it should still be selected. But, it can also be claimed that, though luck, the thread was set running, you lose the selected frame. This doesn't seem like an important enough case to care about. Agree? > > - GDB has finished handling the event, so should restore to the > > user selected thread and frame. But, which is it now? > > thread 1, or thread 3? In all-stop mode? In non-stop mode? > > IMO it's thread 3 in all-stop and thread 1 in non-stop. This is a > weird user-visible artifact, though. > Ok, that's my opinion too. > > With all the above in mind, I thought of: > > > > - calling the { selected thread and frame } thing a "context". > > - having a context stack > > (The top level context being what the user/frontend > > considers current.) > > > > Instead of temporarilly switching to another thread and frame; > > execute commands in it, and restoring the selected thread and > > frame, we do: > > > > - push a new context on the context stack > > - switch the selected thread and frame in this > > new now top level context > > - execute command(s) > > which can switch thread and frame at will > > - pop context > > > > - Whenever a thread exit is detected, we go through all > > contexts and if it was the selected thread in it, invalidate > > it. > > Isn't this exactly the same as using cleanups, except that there is a > way to invalidate saved cleanups? > Yeah, conceptually different, but the end result is close. In fact, ... > If so, then it seems to me there is a simpler and less intrusive > solution. Put a reference count in the thread structure and increment > it when you push a cleanup. I think Vladimir added a hook that will > let us drop the reference even if the cleanup is discarded. > ... I like this. There's one issue that I'd like to point out. That is, is a thread exits, and it was inferior_ptid, we can't just switch inferior_ptid to null_ptid as I could with split user/internal threads split. A *lot* of things break. The issue arises with the OS quickly reusing ptids: - inferior_ptid (ptid1) exits, we can't delete it from the thread list yet (things break, e.g. context switching..., but many other things break in target code.) - We mark it with THREAD_EXITED, but leave it there. - target notifies new thread with ptid1. There's already such a thread in the list, and it's also the current thread -- inferior_ptid. - To add this new thread to the list, we must get rid of the old exited thread. Conceptually, this new thread although it has the target identifier, it should have a new GDB thread id. So, we could say that in this case, the "non-stop doesn't change threads" premise breaks. But, then again, the user couldn't do anything to the exited thread anyway, and it can only exit is it is running, in which case the user also couldn't do most things with it. Maybe we can just live with this exception. > I don't want to proliferate cleanup-like mechanisms if we can avoid > it, the ones we have are confusing enough already :-) > > > > > +/* Only safe to use this cleanup on a stopped thread, and > > > > + inferior_ptid isn't ran before running the cleanup. */ > > > > + > > > > struct current_thread_cleanup > > > > > > Not sure what you mean by this comment. > > > > I've changed the comment to this: > > > > /* It is only safe to use this cleanup iff inferior_ptid is alive and > > stopped, and, by if by the time it is ran, inferior_ptid represents > > the same thread, it is alive and it is stopped. */ > > > > Does it make it clearer? > > Does it in "by the time it is" refer to the cleanup? And then the > "it"s on the last line to "the same thread"? Pronouns are so > confusing. Oh, sorry. Yes, that's correct. > I didn't look at the incremental patch, sorry - let's finish talking > about the problem first. No problem. I like the refcounting idea. Let me work on it, and see if there's any other problem. -- Pedro Alves