From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id HrO6GfE5HGARBwAAWB0awg (envelope-from ) for ; Thu, 04 Feb 2021 13:16:17 -0500 Received: by simark.ca (Postfix, from userid 112) id 59DD61EFCB; Thu, 4 Feb 2021 13:16:17 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-1.1 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 0C4E81EF7A for ; Thu, 4 Feb 2021 13:16:16 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 5546439AF1AE; Thu, 4 Feb 2021 18:16:15 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 5546439AF1AE DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1612462575; bh=srO3iMYuKsYBUetv4cnj2L+sgF1npXQaHtyYQChr0bU=; h=Subject:To:References:Date:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=KmuPrt/bLR4ZIsHKbG2/qIswvt5RCTxy5kc5q6gKjkiKy7f3BmRBElogQb51ToZ14 0BqD6gmqZbxgUWcb37Rn/rkOSOaK/Ds9rb+HcfMJJYG8KmL6RvWNyzH7ErYvKsaUIH 2BYxDroena8m4LXsuh/xftu+Nw6N+fVwuT7W4Qyk= Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id DCC0B3857800 for ; Thu, 4 Feb 2021 18:16:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org DCC0B3857800 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 114IFxsq012353 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 4 Feb 2021 13:16:03 -0500 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 114IFxsq012353 Received: from [10.0.0.11] (192-222-157-6.qc.cable.ebox.net [192.222.157.6]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id CE85C1EF7A; Thu, 4 Feb 2021 13:15:58 -0500 (EST) Subject: Re: [PATCH 1/4] gdb: make async event handlers clear themselves To: Pedro Alves , Simon Marchi , gdb-patches@sourceware.org References: <20201130165251.830482-1-simon.marchi@efficios.com> <20201130165251.830482-2-simon.marchi@efficios.com> <0fe68d72-6530-cb97-a40a-e68a8f230702@palves.net> Message-ID: <291ae5d2-e157-24ee-beb2-b4a3304bf8b2@polymtl.ca> Date: Thu, 4 Feb 2021 13:15:58 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.1 MIME-Version: 1.0 In-Reply-To: <0fe68d72-6530-cb97-a40a-e68a8f230702@palves.net> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Thu, 4 Feb 2021 18:15:59 +0000 X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Simon Marchi via Gdb-patches Reply-To: Simon Marchi Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" On 2021-02-04 12:42 p.m., Pedro Alves wrote: > On 30/11/20 16:52, Simon Marchi via Gdb-patches wrote: >> The `ready` flag of async event handlers is cleared by the async event >> handler system right before invoking the associated callback, in >> check_async_event_handlers. >> >> This is not ideal with how the infrun subsystem consumes events: all >> targets' async event handler callbacks essentially just invoke >> `inferior_event_handler`, which eventually calls `fetch_inferior_event` >> and `do_target_wait`. `do_target_wait` picks an inferior at random, >> and thus a target at random (it could be the target whose `ready` flag >> was cleared, or not), and pulls one event from it. >> >> So it's possible that: >> >> - the async event handler for a target A is called >> - we end up consuming an event for target B >> - all threads of target B are stopped, target_async(0) is called on it, >> so its async event handler is cleared (e.g. >> record_btrace_target::async) >> >> As a result, target A still has events to report while its async event >> handler is left unmarked, so these events are not consumed. To counter >> this, at the end of their async event handler callbacks, targets check >> if they still have something to report and re-mark their async event >> handler (e.g. remote_async_inferior_event_handler). >> >> The linux_nat target does not suffer from this because it doesn't use an >> async event handler at the moment. It only uses a pipe registered with >> the event loop. It is written to in the SIGCHLD handler (and in other >> spots that want to get target wait method called) and read from in >> the target's wait method. So if linux_nat happened to be target A in >> the example above, the pipe would just stay readable, and the event loop >> would wake up again, until linux_nat's wait method is finally called and >> consumes the contents of the pipe. >> >> I think it would be nicer if targets using async_event_handler worked in >> a similar way, where the flag would stay set until the target's wait >> method is actually called. As a first step towards that, this patch >> moves the responsibility of clearing the ready flags of async event >> handlers to the invoked callback. >> >> All async event handler callbacks are modified to clear their ready flag >> before doing anything else. So in practice, nothing changes with this >> patch. It's only the responsibility of clearing the flag that is >> shifted toward the callee. >> >> gdb/ChangeLog: >> >> * async-event.h (async_event_handler_func): Add documentation. >> * async-event.c (check_async_event_handlers): Don't clear >> async_event_handler ready flag. >> * infrun.c (infrun_async_inferior_event_handler): Clear ready >> flag. >> * record-btrace.c (record_btrace_handle_async_inferior_event): >> Likewise. >> * record-full.c (record_full_async_inferior_event_handler): >> Likewise. >> * remote-notif.c (remote_async_get_pending_events_handler): >> Likewise. >> * remote.c (remote_async_inferior_event_handler): Likewise. > > (Watch out for spaces/tabs in the ChangeLog.) Fixed. > I like this. OK. Pushed, thanks. Simon