Re: [PATCH] was Re: [PATCH 2/3] [delete] Not-so-harmless spurious call to `wait4`

[Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>]

On 2021-04-08 4:25 p.m., Dominique Quatravaux wrote:> 
> 
>> Le 8 avr. 2021 à 21:26, Simon Marchi <simon.marchi@polymtl.ca <mailto:simon.marchi@polymtl.ca>> a écrit :
>>
>>   Can you please explain in the commit message what harm
>> that wait4 call does and why we want to remove it?
> 
> In trying to make a better case in the commit message, I found that swapping the two patches around made more sense. Here is an updated, rebased, single patch (working under the assumption that the other two will be merged first):
> 
> From 5c3756e9eb0342b1a5a23bcb54d5b8317743ce0d Mon Sep 17 00:00:00 2001
> From: Dominique Quatravaux <dominique.quatravaux@epfl.ch <mailto:dominique.quatravaux@epfl.ch>>
> Date: Thu, 8 Apr 2021 21:35:57 +0200
> Subject: [PATCH] [delete] not-so-harmless spurious call to `wait4`
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> As seen in https://sourceware.org/bugzilla/show_bug.cgi?id=24069 <https://sourceware.org/bugzilla/show_bug.cgi?id=24069> this
> code will typically wait4() a second time on the same process that was
> already wait4()'d a few lines above. While this used to be
> harmless/idempotent (when we assumed that the process already exited),
> this now causes a deadlock in the WIFSTOPPED case.
> 
> The early (~2019) history of bug #24069 cautiously suggests to use
> WNOHANG instead of outright deleting the call. However, tests on the
> current version of Darwin (Big Sur) demonstrate that gdb runs just
> fine without a redundant call to wait4(), as would be expected.
> Notwithstanding the debatable value of conserving bug compatibility
> with an OS release that is more than a decade old, there is scant
> evidence of what that double-wait4() was supposed to achieve in the
> first place

Thanks, this looks good.  If we had more contributors for macOS,
especially some that cared about old macOS versions, we could aim at
supporting many versions back.  But given the current state, it's
perfectly reasonable to aim at making GDB work well for the latest
version.

 - A cursory investigation with `git blame` pinpoints
> commits bb00b29d7802 and a80b95ba67e2 from the 2008-2009 era, but
> fails to answer the “why” question conclusively.

Yes, the commits that predate the usage of git don't have the patch
message / rationale that people sent along with the patch, it just
wasn't recorded in CVS.  You can always dig into into the mailing
list.

a80b95ba67e2: https://pi.simark.ca/gdb-patches/F1826DD8-CC0F-4FF1-BC47-3F2ACBB42909@adacore.com/
bb00b29d7802: https://pi.simark.ca/gdb-patches/20090319141746.GA81236@ulanbator.act-europe.fr/

Although there isn't more details about the bit you are removing.

So let's just wait until we hear back from the FSF about your copyright
assignment, and then we can merge all of this.

Note that we currently require contributions to include ChangeLog
entries:

    https://sourceware.org/gdb/wiki/ContributionChecklist#Properly_Formatted_GNU_ChangeLog

I don't mind for this time, the patches are small enough that I can
write them when merging.  We are also discussing whether we keep using
those, so it may or may not apply in the future.

Simon