Re: [PATCH v2] Release the GIL while running a gdb command or expression

[Pedro Alves <palves at redhat dot com>]

On 10/10/2018 09:22 PM, Tom Tromey wrote:

> index 0000000000..92d103a963
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-gil-mthread.c
> @@ -0,0 +1,30 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2018 Free Software Foundation, Inc.

Fedora's local version has copyright 2014, so this should
be 2014-2018.

> +#include <stdio.h>
> +#include <unistd.h>
> +
> +int
> +main (void)
> +{
> +  int i;
> +  for (i = 0; i < 10; i++)
> +    {
> +      sleep (1); /* break-here */
> +      printf ("Sleeping %d\n", i);
> +    }
> +}
> diff --git a/gdb/testsuite/gdb.python/py-gil-mthread.exp b/gdb/testsuite/gdb.python/py-gil-mthread.exp
> new file mode 100644
> index 0000000000..6a65346a8d

Please add a describing comment mentioning what the
testcase is about.  The testcase isn't 

> +
> +standard_testfile .c .py
> +set executable $testfile
> +
> +if { [prepare_for_testing $testfile.exp $executable $srcfile] } {
> +    return -1
> +}
> +
> +# Skip all tests if Python scripting is not enabled.
> +if { [skip_python_tests] } { continue }
> +
> +if ![runto_main] {
> +    return -1
> +}
> +

The test relies on stdio, so there should be a gdb_skip_stdio_test
or gdb,noinferiorio check here somewhere.

> +gdb_breakpoint $srcfile:[gdb_get_line_number "break-here"] temporary
> +gdb_continue_to_breakpoint "break-here" ".* break-here .*"
> +
> +set test "response"
> +set timeout 60

Do we need to change the timeout?  

Should this use with_timeout_factor?

> +set sleeping_last -1
> +set hello_last 0
> +set minimal 5
> +gdb_test_multiple "python exec (open ('$srcdir/$subdir/$srcfile2').read ())" $test {

I don't think this would work with remote host testing.
AFAIC, python tests usually "gdb_remote_download host"
the python script.

> +    -re "Error: unable to start thread\r\n" {
> +	fail $test
> +    }
> +    -re "Sleeping (\[0-9\]+)\r\n" {

This is inferior output, so should be expected on
$inferior_spawn_id.   Maybe you can tweak the
test to not need this, like run to a breakpoint.
No idea whether that makes sense.

Please double check the testcase works against
--target_board=native-gdbserver/native-extended-gdbserver.

Thanks,
Pedro Alves

> +	set n $expect_out(1, string)
> +	if { $sleeping_last + 1 != $n } {
> +	    fail $test
> +	} else {
> +	    set sleeping_last $n
> +	    if { $sleeping_last >= $minimal && $hello_last >= $minimal } {
> +		pass $test
> +	    } else {
> +		exp_continue
> +	    }
> +	}
> +    }
> +    -re "Hello \\( (\[0-9\]+) \\)\r\n" {
> +	set n $expect_out(1,string)
> +	if { $hello_last + 1 != $n } {
> +	    fail $test
> +	} else {
> +	    set hello_last $n
> +	    if { $sleeping_last >= $minimal && $hello_last >= $minimal } {
> +		pass $test
> +	    } else {
> +		exp_continue
> +	    }
> +	}
> +    }
> +}