From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2198 invoked by alias); 10 Oct 2018 08:33:11 -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 2184 invoked by uid 89); 10 Oct 2018 08:33:11 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-0.9 required=5.0 tests=BAYES_00,KAM_LAZY_DOMAIN_SECURITY,RCVD_IN_DNSWL_NONE autolearn=no version=3.3.2 spammy=initiated, HX-Received:sk:d16-v6m, H*RU:209.85.221.65, unsure X-HELO: mail-wr1-f65.google.com Received: from mail-wr1-f65.google.com (HELO mail-wr1-f65.google.com) (209.85.221.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 10 Oct 2018 08:33:09 +0000 Received: by mail-wr1-f65.google.com with SMTP id 61-v6so4675040wrb.6 for ; Wed, 10 Oct 2018 01:33:09 -0700 (PDT) Return-Path: Received: from ?IPv6:2a02:c7f:ae6a:ed00:4685:ff:fe66:9f4? ([2a02:c7f:ae6a:ed00:4685:ff:fe66:9f4]) by smtp.gmail.com with ESMTPSA id z16-v6sm13911611wmc.0.2018.10.10.01.33.05 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 10 Oct 2018 01:33:06 -0700 (PDT) Subject: Re: [RFC] Release the GIL while running a gdb command or expression To: Tom Tromey Cc: gdb-patches@sourceware.org References: <20180915040732.6718-1-tom@tromey.com> <807f284a-e227-37ed-c197-170a7f2abe40@redhat.com> <874ldugb5e.fsf@tromey.com> From: Phil Muldoon Message-ID: <6503980f-f5ab-ecab-37a5-993d9daec226@redhat.com> Date: Wed, 10 Oct 2018 08:33:00 -0000 MIME-Version: 1.0 In-Reply-To: <874ldugb5e.fsf@tromey.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2018-10/txt/msg00243.txt.bz2 On 09/10/2018 20:42, Tom Tromey wrote: >>>>>> "Phil" == Phil Muldoon writes: > > Phil> We've had a similar patch in the Fedora RPM for a while. It's been on > Phil> my list to upstream for a bit. Initially I was a bit reluctant > Phil> because I hadn't audited all the Python reentry points in GDB to make > Phil> sure we reacquired the GIL before interacting with Python. I realize > Phil> now this was not a real concern (reviews would catch this and if one > Phil> did slip by it would be fixed as a bug.) > > Yes, I think it would result in a crash. > > Phil> The only real difference in the patch approach was to make the GIL > Phil> release an option over a default. > > I don't think this is necessary, mostly because I can't think of when it > would be desirable not to release the GIL; but also because when writing > Python one doesn't normally have to worry about the GIL -- it's not > really a Python-visible feature, nor should it be, since implementations > like PyPY don't have it. It's not so much an implementation detail that should be exposed to the user but rather a change in behavior around gdb.execute. Given that now, with this patch, we always release the Python GIL during the execution of a GDB command via gdb.execute, any other Python thread that was previously blocked by the GIL is now unblocked, and it may appear to those threads that the Python thread that initiated the gdb.execute has returned from that command when it may not have (this is especially so in cases where a GDB command takes seconds to complete a command). Also, any other Python threads that wish to interact with GDB will have to wait until the GDB event loop returns to a state where it is accepting input (at least I think this is true). This may break some scripts out there. Are these scripts relying on what we now classify as a bug or is there is a reasonable expectation, on the users' behalf, that a script could rely on GDB's previous GIL blocking behavior? I'm not advocating we should have a release_gil parameter, I'm just unsure of the expectations of users and scripts out there, and that if we don't provide a mechanism to optionally block the GIL, it will cause disruption to any established scripts out there. I suppose the solution is to either provide a GIL blocking parameter or to thoroughly document this new behavior in the manual. What do you think? > Phil> As for a test, we also have a test included. It does not appear to be > Phil> racy for our purposes. I also include it for consideration. This > Phil> snippet is just for initial consideration and will make any changes > Phil> needed to include it in the patch. > > I can try it. If it works, whose name should I put on the ChangeLog? I wrote the GIL patch for Fedora so I suppose mine. The test itself may need some modification in that it can be optimized and it may also need some format tweaks. Cheers Phil