diff mbox

[v2,2/3] PR remote/19496, interrupted syscall in forking-threads-plus-bkpt

Message ID 1458152750-25119-1-git-send-email-donb@codesourcery.com
State New
Headers show

Commit Message

Don Breazeal March 16, 2016, 6:25 p.m. UTC
On 3/15/2016 5:55 AM, Pedro Alves wrote:
> Hi Don,
> 
> On 02/11/2016 12:28 AM, Don Breazeal wrote:
> 
---snip---
> 
> Use with_timeout_factor instead so that the timeout is properly restored,
> and put it around the problematic test, only, instead of basically
> around the whole test case.  I think that'll be the "inferior 1 exited"
> test?

Thanks Pedro.  This is done in the patch below.  OK to push?
--Don

This patch addresses "fork:Interrupted system call" (or wait:) failures
in gdb.threads/forking-threads-plus-breakpoint.exp.

The test program spawns ten threads, each of which do ten fork/waitpid
sequences.  The cause of the problem was that when one of the fork
children exited before the corresponding fork parent could initiate its
waitpid for that child, a SIGCHLD and/or SIGSTOP was delivered and
interrupted a fork or waitpid in another thread.

The fix was to wrap the system calls in a loop to retry the call if
it was interrupted, like:

do
  {
    pid = fork ();
  }
while (pid == -1 && errno == EINTR);

Since this is a Linux-only test I figure it is OK to use errno and EINTR.
I tried a number of alternative fixes using SIG_IGN, SA_RESTART,
pthread_sigblock, and bsd_signal, but none of these worked as well.

Tested on Nios II Linux target with x86 Linux host.

gdb/testsuite/ChangeLog:
2016-03-16  Don Breazeal  <donb@codesourcery.com>

	* gdb.threads/forking-threads-plus-breakpoint.c (thread_forks):
	Retry fork and waitpid on interrupted system call errors.
	* gdb.threads/forking-threads-plus-breakpoint.exp: (do_test):
	Use with_timeout_factor to increase timeout to 90.

---
 .../gdb.threads/forking-threads-plus-breakpoint.c  |   14 +++++++++-
 .../forking-threads-plus-breakpoint.exp            |   27 +++++++++++---------
 2 files changed, 27 insertions(+), 14 deletions(-)

Comments

Pedro Alves March 16, 2016, 6:33 p.m. UTC | #1
On 03/16/2016 06:25 PM, Don Breazeal wrote:

> --- a/gdb/testsuite/gdb.threads/forking-threads-plus-breakpoint.exp
> +++ b/gdb/testsuite/gdb.threads/forking-threads-plus-breakpoint.exp
> @@ -72,6 +72,7 @@ proc do_test { cond_bp_target detach_on_fork displaced } {
>       global decimal gdb_prompt
>       global linenum
>       global is_remote_target
> +    global timeout

Don't need this.

>
>       set saved_gdbflags $GDBFLAGS
>       set GDBFLAGS [concat $GDBFLAGS " -ex \"set non-stop on\""]
> @@ -115,18 +116,20 @@ proc do_test { cond_bp_target detach_on_fork displaced } {
>       set fork_count 0
>       set ok 0
>
> -    set test "inferior 1 exited"
> -    gdb_test_multiple "" $test {
> -	-re "Inferior 1 \(\[^\r\n\]+\) exited normally" {
> -	    set ok 1
> -	    pass $test
> -	}
> -	-re "Inferior $decimal \(\[^\r\n\]+\) exited normally" {
> -	    incr fork_count
> -	    if {$fork_count <= 100} {
> -		exp_continue
> -	    } else {
> -		fail "$test (too many forks)"
> +    with_timeout_factor 90 {

Eeeek.  :-)  This is a factor, not absolute time.

The default is 10 seconds, so a factor of 9 or 10
should suffice.  I'd say make it 10, just because it is
round and matches the number of threads.

Otherwise OK.

Thanks,
Pedro Alves
Don Breazeal March 16, 2016, 10:18 p.m. UTC | #2
On 3/16/2016 11:33 AM, Pedro Alves wrote:
> On 03/16/2016 06:25 PM, Don Breazeal wrote:
> 
>> --- a/gdb/testsuite/gdb.threads/forking-threads-plus-breakpoint.exp
>> +++ b/gdb/testsuite/gdb.threads/forking-threads-plus-breakpoint.exp
>> @@ -72,6 +72,7 @@ proc do_test { cond_bp_target detach_on_fork displaced } {
>>       global decimal gdb_prompt
>>       global linenum
>>       global is_remote_target
>> +    global timeout
> 
> Don't need this.
> 
>>
>>       set saved_gdbflags $GDBFLAGS
>>       set GDBFLAGS [concat $GDBFLAGS " -ex \"set non-stop on\""]
>> @@ -115,18 +116,20 @@ proc do_test { cond_bp_target detach_on_fork displaced } {
>>       set fork_count 0
>>       set ok 0
>>
>> -    set test "inferior 1 exited"
>> -    gdb_test_multiple "" $test {
>> -	-re "Inferior 1 \(\[^\r\n\]+\) exited normally" {
>> -	    set ok 1
>> -	    pass $test
>> -	}
>> -	-re "Inferior $decimal \(\[^\r\n\]+\) exited normally" {
>> -	    incr fork_count
>> -	    if {$fork_count <= 100} {
>> -		exp_continue
>> -	    } else {
>> -		fail "$test (too many forks)"
>> +    with_timeout_factor 90 {
> 
> Eeeek.  :-)  This is a factor, not absolute time.
> 
> The default is 10 seconds, so a factor of 9 or 10
> should suffice.  I'd say make it 10, just because it is
> round and matches the number of threads.
> 
> Otherwise OK.
> 
> Thanks,
> Pedro Alves
> 
Thanks Pedro.  This is now pushed, with changes.
--Don
diff mbox

Patch

diff --git a/gdb/testsuite/gdb.threads/forking-threads-plus-breakpoint.c b/gdb/testsuite/gdb.threads/forking-threads-plus-breakpoint.c
index fc64d93..c169e18 100644
--- a/gdb/testsuite/gdb.threads/forking-threads-plus-breakpoint.c
+++ b/gdb/testsuite/gdb.threads/forking-threads-plus-breakpoint.c
@@ -22,6 +22,7 @@ 
 #include <sys/types.h>
 #include <sys/wait.h>
 #include <stdlib.h>
+#include <errno.h>
 
 /* Number of threads.  Each thread continuously spawns a fork and wait
    for it.  If we have another thread continuously start a step over,
@@ -49,14 +50,23 @@  thread_forks (void *arg)
     {
       pid_t pid;
 
-      pid = fork ();
+      do
+	{
+	  pid = fork ();
+	}
+      while (pid == -1 && errno == EINTR);
 
       if (pid > 0)
 	{
 	  int status;
 
 	  /* Parent.  */
-	  pid = waitpid (pid, &status, 0);
+	  do
+	    {
+	      pid = waitpid (pid, &status, 0);
+	    }
+	  while (pid == -1 && errno == EINTR);
+
 	  if (pid == -1)
 	    {
 	      perror ("wait");
diff --git a/gdb/testsuite/gdb.threads/forking-threads-plus-breakpoint.exp b/gdb/testsuite/gdb.threads/forking-threads-plus-breakpoint.exp
index 3d8b308..9c700bf 100644
--- a/gdb/testsuite/gdb.threads/forking-threads-plus-breakpoint.exp
+++ b/gdb/testsuite/gdb.threads/forking-threads-plus-breakpoint.exp
@@ -72,6 +72,7 @@  proc do_test { cond_bp_target detach_on_fork displaced } {
     global decimal gdb_prompt
     global linenum
     global is_remote_target
+    global timeout
 
     set saved_gdbflags $GDBFLAGS
     set GDBFLAGS [concat $GDBFLAGS " -ex \"set non-stop on\""]
@@ -115,18 +116,20 @@  proc do_test { cond_bp_target detach_on_fork displaced } {
     set fork_count 0
     set ok 0
 
-    set test "inferior 1 exited"
-    gdb_test_multiple "" $test {
-	-re "Inferior 1 \(\[^\r\n\]+\) exited normally" {
-	    set ok 1
-	    pass $test
-	}
-	-re "Inferior $decimal \(\[^\r\n\]+\) exited normally" {
-	    incr fork_count
-	    if {$fork_count <= 100} {
-		exp_continue
-	    } else {
-		fail "$test (too many forks)"
+    with_timeout_factor 90 {
+        set test "inferior 1 exited"
+        gdb_test_multiple "" $test {
+	    -re "Inferior 1 \(\[^\r\n\]+\) exited normally" {
+	        set ok 1
+	        pass $test
+	    }
+	    -re "Inferior $decimal \(\[^\r\n\]+\) exited normally" {
+	        incr fork_count
+	        if {$fork_count <= 100} {
+		    exp_continue
+	        } else {
+		    fail "$test (too many forks)"
+	        }
 	    }
 	}
     }