From patchwork Fri Mar 21 11:26:12 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 204 Return-Path: X-Original-To: siddhesh@wilcox.dreamhost.com Delivered-To: siddhesh@wilcox.dreamhost.com Received: from homiemail-mx22.g.dreamhost.com (caibbdcaabij.dreamhost.com [208.113.200.189]) by wilcox.dreamhost.com (Postfix) with ESMTP id 05F883600F8 for ; Fri, 21 Mar 2014 04:26:22 -0700 (PDT) Received: by homiemail-mx22.g.dreamhost.com (Postfix, from userid 14314964) id 3AC544F8F11A; Fri, 21 Mar 2014 04:26:22 -0700 (PDT) X-Original-To: gdb@patchwork.siddhesh.in Delivered-To: x14314964@homiemail-mx22.g.dreamhost.com Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by homiemail-mx22.g.dreamhost.com (Postfix) with ESMTPS id 0B0504F8F11A for ; Fri, 21 Mar 2014 04:26:22 -0700 (PDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:message-id:date:from:mime-version:to:cc :subject:references:in-reply-to:content-type :content-transfer-encoding; q=dns; s=default; b=WfeICuFf9NtItlDY dUYZ/+q2zWpInuDc2UEd7UUwqgJ/3Lti8sVdYsT9NoTB46u8oIb2gRw6DeHB4BKC E0cStZo3PnatEsFW/mifoRqb5BhDwqagu+5HT+sbF+xutf5ZfuIUvELTcvZIcxbl QYjJHvUgKOsObDu0l3edW4IEsjU= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:message-id:date:from:mime-version:to:cc :subject:references:in-reply-to:content-type :content-transfer-encoding; s=default; bh=I6o0bX4uUBRB+0lGgFQyLJ YRyzs=; b=ErAJNuXXJydh3f4gCDP7i8M86RN+O2ecdgBBAb1h2UGzGkWfazaB8r IDlAF4u250VjYtMYVCHFhUFrlNVVdtKqpsrtuNwahV9F3psvomHJvucAU5LI6JR7 KWkhgnQY1HpExs9BjVbpbWwC+rdLs2SjlUfE0TmzmuvFpeMLzekwg= Received: (qmail 11276 invoked by alias); 21 Mar 2014 11:26:20 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 11265 invoked by uid 89); 21 Mar 2014 11:26:19 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL, BAYES_00, SPF_HELO_PASS, SPF_PASS, T_RP_MATCHES_RCVD 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 ESMTP; Fri, 21 Mar 2014 11:26:17 +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 s2LBQFuS012896 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 21 Mar 2014 07:26:15 -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 s2LBQDjY029720; Fri, 21 Mar 2014 07:26:14 -0400 Message-ID: <532C21D4.2090809@redhat.com> Date: Fri, 21 Mar 2014 11:26:12 +0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7 MIME-Version: 1.0 To: Doug Evans CC: gdb-patches Subject: [PUSHED] normal_stop: Extend and clarify comment. (was: Re: No thread context switch in non-stop mode???) References: <53272539.2020302@redhat.com> <21287.35647.738364.880400@ruffy.mtv.corp.google.com> In-Reply-To: <21287.35647.738364.880400@ruffy.mtv.corp.google.com> X-DH-Original-To: gdb@patchwork.siddhesh.in On 03/17/2014 11:54 PM, Doug Evans wrote: > Pedro Alves writes: > > > The above comment is a bit misleading since that code doesn't actually > > > change anything, it just prints a message notifying the user that the > > > current thread has changed. > > > > Hmm, WDYM by prints a message? > > I mean it prints a message, this message: > > printf_filtered (_("[Switching to %s]\n"), > target_pid_to_str (inferior_ptid)); > > This comment: > > /* In non-stop mode, we don't want GDB to switch threads behind the > user's back, to avoid races where the user is typing a command to > apply to thread x, but GDB switches to thread y before the user > finishes entering the command. */ > > is misleading (to me) because while the wording is correct the > place where it lives has nothing to do with implementing that. > As I'm reading the comment I'm expecting the following code to make sure GDB > doesn't switch threads behind the user's back, and since the following > code is not doing that, and is just printing the "Switching to" message > (given certain conditions (!non_stop && ...)), > I'm left wondering if there's a bug. > Could be missing something of course. Ah, I see what you mean now. > > > If it weren't for the cleanup, > > For clarity's sake, which cleanup? I've applied the patch below. Let's extend it further if still not clear. BTW, I think we should actually change the predicate that controls whether GDB switches threads from non_stop, to "whether the user has the prompt". IOW, fg vs bg command. Or YOW, sync_execution. But, we'd need to consider MI frontends that enable all-stop+async somehow, as those wouldn't be expecting such a change... Thanks. ------- [PATCH] normal_stop: Extend and clarify comment. Explain better why we skip saying "Switching to ..." in non-stop mode. gdb/ 2014-03-21 Pedro Alves * infrun.c (normal_stop): Extend comment. --- gdb/ChangeLog | 4 ++++ gdb/infrun.c | 16 ++++++++++------ 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index ba2ca31..0aec7b9 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,7 @@ +2014-03-21 Pedro Alves + + * infrun.c (normal_stop): Extend comment. + 2014-03-21 Hui Zhu Pedro Alves diff --git a/gdb/infrun.c b/gdb/infrun.c index d7de7d1..8f9e820 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -6059,18 +6059,22 @@ normal_stop (void) && last.kind != TARGET_WAITKIND_NO_RESUMED) make_cleanup (finish_thread_state_cleanup, &inferior_ptid); - /* In non-stop mode, we don't want GDB to switch threads behind the - user's back, to avoid races where the user is typing a command to - apply to thread x, but GDB switches to thread y before the user - finishes entering the command. */ - /* As with the notification of thread events, we want to delay notifying the user that we've switched thread context until the inferior actually stops. There's no point in saying anything if the inferior has exited. Note that SIGNALLED here means "exited with a signal", not - "received a signal". */ + "received a signal". + + Also skip saying anything in non-stop mode. In that mode, as we + don't want GDB to switch threads behind the user's back, to avoid + races where the user is typing a command to apply to thread x, + but GDB switches to thread y before the user finishes entering + the command, fetch_inferior_event installs a cleanup to restore + the current thread back to the thread the user had selected right + after this event is handled, so we're not really switching, only + informing of a stop. */ if (!non_stop && !ptid_equal (previous_inferior_ptid, inferior_ptid) && target_has_execution