Re: [PATCH 1/2] [gdb/testsuite] Fix race in gdb.threads/leader-exit.exp

[Tom de Vries <tdevries@suse.de>]

On 7/24/26 7:49 PM, Keith Seitz wrote:
> Hi,
> 
> On 7/23/26 4:40 AM, Tom de Vries wrote:
>>
>> The problem is that after the pthread_join is executed:
>> ...
>>    i = pthread_join (main_thread, NULL);
>>    ...
>>
>>    sleep (10);  /* break-here */
>> ...
>> there's a race between:
>> - the breakpoint at break-here triggering, and
>> - the "Thread exited" message for the main thread being reported.
> 
> While I cannot reproduce this on my local hardware, your analysis
> is sound.
> 
>> Fix this by:
>> - adding a loop before the break location
>> - waiting in the loop until the "Thread exited" message is seen
>> - sending ^C to get a prompt
>> - setting a variable to exit the loop
>>
>> Likewise in gdb.threads/non-ldr-exc-2.exp.
> 
> Sounds like a plan!
> 
>> diff --git a/gdb/testsuite/gdb.threads/leader-exit.exp b/gdb/ 
>> testsuite/gdb.threads/leader-exit.exp
>> index b5e6f558ac7..22f27d2d6e8 100644
>> --- a/gdb/testsuite/gdb.threads/leader-exit.exp
>> +++ b/gdb/testsuite/gdb.threads/leader-exit.exp
>> @@ -30,6 +30,28 @@ if {![runto_main]} {
>>       return
>>   }
>> +# Wait for the "Thread exited" message.
>> +set re_thread_exited {\[Thread [^\r\n]+ exited\]}
>> +set saw_thread_exited 0
>> +gdb_test_multiple "continue" "continue to thread exited" {
>> +    -re $re_thread_exited {
>> +    set saw_thread_exited 1
>> +    # Get a prompt.
>> +    send_gdb "\003"
>> +    exp_continue
>> +    }
>> +    -re -wrap "" {
>> +    pass $gdb_test_name
>> +    }
>> +}
>> +gdb_assert {$saw_thread_exited} "thread exited"
>> +if {!$saw_thread_exited} {
>> +    return
>> +}
>> +
>> +# Let the inferior to exit the wait_for_exit loop.
> 
> This comment got a bit jumbled. Superfluous "to"?
> 
>> +gdb_test_no_output "set var wait_for_exit = 0"
>> +
>>   gdb_breakpoint [gdb_get_line_number "break-here"]
>>   gdb_continue_to_breakpoint "break-here" ".* break-here .*"
>> diff --git a/gdb/testsuite/gdb.threads/non-ldr-exc-2.exp b/gdb/ 
>> testsuite/gdb.threads/non-ldr-exc-2.exp
>> index ead262cc2b5..dfc34f42afe 100644
>> --- a/gdb/testsuite/gdb.threads/non-ldr-exc-2.exp
>> +++ b/gdb/testsuite/gdb.threads/non-ldr-exc-2.exp
>> @@ -37,6 +37,28 @@ proc do_test { lock_sched nonstop } {
>>       return -1
>>       }
>> +    # Wait for the "Thread exited" message.
>> +    set re_thread_exited {\[Thread [^\r\n]+ exited\]}
>> +    set saw_thread_exited 0
>> +    gdb_test_multiple "continue" "continue to thread exited" {
>> +    -re $re_thread_exited {
>> +        set saw_thread_exited 1
>> +        # Get a prompt.
>> +        send_gdb "\003"
>> +        exp_continue
>> +    }
>> +    -re -wrap "" {
>> +        pass $gdb_test_name
>> +    }
>> +    }
>> +    gdb_assert {$saw_thread_exited} "thread exited"
>> +    if {!$saw_thread_exited} {
>> +    return
>> +    }
>> +
>> +    # Let the inferior to exit the wait_for_exit loop.
> 
> Same nit in this comment.
> 

Hi Keith,

fixed both nits.

> Question: Since this test file now relies on sending the target
> an interrupt, does it also need the same "require" on gdb,nointerrupts
> that leader-exit.exp has?
> 

Thanks for spotting that.  I've added this, and pushed.

Thanks for the reviews,
- Tom

> Reviewed-By: Keith Seitz <keiths@redhat.com>
> 
> Keith
> 
>> +    gdb_test_no_output "set var wait_for_exit = 0"
>> +
>>       gdb_breakpoint [gdb_get_line_number "break-here"]
>>       gdb_continue_to_breakpoint "break-here" ".* break-here .*"
>