Re: [PATCH v2 5/6] [gdb/testsuite] Extend gdb.testsuite/lock.exp

[Keith Seitz <keiths@redhat.com>]

Hi,

I think this series looks good, I just have one question.

On 9/29/26 4:09 AM, Tom de Vries wrote:
> diff --git a/gdb/testsuite/lib/gdb-utils.exp b/gdb/testsuite/lib/gdb-utils.exp
> index 1277558170d..9ccde20634e 100644
> --- a/gdb/testsuite/lib/gdb-utils.exp
> +++ b/gdb/testsuite/lib/gdb-utils.exp
> @@ -180,10 +180,15 @@ proc version_compare { l1 op l2 } {
>       return 1
>   }
>   
> -# Acquire lock file LOCKFILE.  Tries forever until the lock file is
> -# successfully created.
> -
> -proc lock_file_acquire {lockfile} {
> +# Acquire lock file LOCKFILE.
> +# If the lock file:
> +# - doesn't exist, create it and return a file handle/file name pair.
> +# - does exist, and:
> +#   - ON_RETRY is empty, wait a bit and try again.
> +#   - ON_RETRY is non-empty, call it.  If it returns 0, don't retry and
> +#     return {} to indicate failure.
> +
> +proc lock_file_acquire {lockfile {on_retry ""}} {
>       verbose -log "acquiring lock file: $::subdir/${::gdb_test_file_name}.exp"
>   
>       while {![info exists fh]} {
> @@ -191,7 +196,11 @@ proc lock_file_acquire {lockfile} {
>   	    set fh [open $lockfile {WRONLY CREAT EXCL}]
>   	} trap "POSIX EEXIST" {} {
>   	    # Lock in use.  Ignore and try again.  Propagate all other errors.
> -	    after 10
> +	    if {$on_retry == ""} {
> +		after 10
> +	    } elseif {![$on_retry]} {
> +		return {}
> +	    }
>   	}
>       }
>   

The case where ON_RETRY is NOT empty and returns non-zero is not
particularly well documented, but I assume it is the same as ON_RETRY
being empty. That is, both cases should loop.

However, in the empty case, there is a 10ms delay. In the non-empty
case, there is no (default) delay. I see that the on_retry proc in
lock.exp does do a delay.

It seems it would be easy to introduce a really tight spin?

In any case, a note to document this responsibility might be in
order.

For the series:
Reviewed-By: Keith Seitz <keiths@redhat.com>

Keith