Re: [PATCH 19/25] Don't resume new threads if scheduler-locking is in effect

[Pedro Alves <pedro@palves.net>]

On 2022-07-11 4:44 p.m., Eli Zaretskii wrote:
>> Cc: gdb-patches@sourceware.org
>> From: Pedro Alves <pedro@palves.net>
>> Date: Mon, 11 Jul 2022 15:20:03 +0100
>>
>>>> +* If supported by the target, when scheduler-locking is in effect, new
>>>> +  threads created by the resumed thread are held stopped.
>>>                         ^^^^^^^^^^^^^^^^^^
>>> "a resumed thread", I guess?
>>
>> When scheduler locking is in effect, execution commands only resume one thread,
>> so "the" seems more correct to me.
> 
> I don't think so, because which thread that is, is unknown to the
> reader at this point.
> 
> As a compromise, I suggest to say "new threads created by resumed
> threads".
> 
>> I wrote the sentence in the manual first, before copying it here, and in the manual,
>> the text describing scheduler locking, above my new sentence says for example:
>>
>>   "If @code{on}, then only the current thread may run when the inferior is resumed."
> 
> That's fine, but I don't see how it implies anything for the NEWS
> entry.  Besides, the m,annual has context that NEWS not necessarily does.

You had said "Same here, " for the manual part, and I was explaining that
for the manual it makes sense to use "the", and that I just copied it over
to NEWS.

> 
>> I notice now that the whole "set scheduler-locking" description doesn't even
>> say that not all targets support scheduler locking.  Hmm.  How about I just
>> remove the "If supported by the target" part.  We can just consider that targets
>> that don't behave that way yet should be fixed (there's a new testcase that
>> will fail for such targets), while older GDBservers just can't ever be fixed,
>> but such is life of software progress.
> 
> That's okay, but it takes care of a different wording issue.
> 
>> How about this, then, given the explanations above?  Is it OK?
>>
>> diff --git c/gdb/NEWS w/gdb/NEWS
>> index 1178a37017e..94553f18608 100644
>> --- c/gdb/NEWS
>> +++ w/gdb/NEWS
>> @@ -3,6 +3,9 @@
>>  
>>  *** Changes since GDB 12
>>  
>> +* When scheduler-locking is in effect, new threads created by the
>> +  resumed thread are held stopped.
>> +
>>  * "info breakpoints" now displays enabled breakpoint locations of
>>    disabled breakpoints as in the "y-" state.  For example:
>>  
>> diff --git c/gdb/doc/gdb.texinfo w/gdb/doc/gdb.texinfo
>> index 7a4e337d15b..fc297218696 100644
>> --- c/gdb/doc/gdb.texinfo
>> +++ w/gdb/doc/gdb.texinfo
>> @@ -6953,6 +6953,9 @@ current thread away from the thread that you are debugging.  The
>>  @code{replay} mode behaves like @code{off} in record mode and like
>>  @code{on} in replay mode.
>>  
>> +When scheduler-locking is in effect, new threads created by the
>> +resumed thread are held stopped.
> 
> This still has the same "the resumed thread" issue.
> 

Sure, because I had explained why I thought it was still better.

Let's treat NEWS and manual differently, then.  This version adds the missing context
in the NEWS entry.  WDYT?

diff --git c/gdb/NEWS w/gdb/NEWS
index 1178a37017e..d474ed21350 100644
--- c/gdb/NEWS
+++ w/gdb/NEWS
@@ -3,6 +3,13 @@
 
 *** Changes since GDB 12
 
+* Scheduler-locking and new threads
+
+  When scheduler-locking is in effect, only the current thread may run
+  when the inferior is resumed.  However, previously, new threads
+  created by the resumed thread would still be able to run free.  Now,
+  they are held stopped.
+
 * "info breakpoints" now displays enabled breakpoint locations of
   disabled breakpoints as in the "y-" state.  For example:
 
diff --git c/gdb/doc/gdb.texinfo w/gdb/doc/gdb.texinfo
index 7a4e337d15b..fc297218696 100644
--- c/gdb/doc/gdb.texinfo
+++ w/gdb/doc/gdb.texinfo
@@ -6953,6 +6953,9 @@ current thread away from the thread that you are debugging.  The
 @code{replay} mode behaves like @code{off} in record mode and like
 @code{on} in replay mode.
 
+When scheduler-locking is in effect, new threads created by the
+resumed thread are held stopped.
+
 @item show scheduler-locking
 Display the current scheduler locking mode.
 @end table