Re: [PATCH 0/4] Testsuite update

Re: [PATCH 0/4] Testsuite update

[Jon Turney]

[-- Attachment #1: Type: text/plain, Size: 997 bytes --]

On 07/08/2023 09:55, Corinna Vinschen wrote:
> On Aug  4 13:47, Jon Turney wrote:
>> This gets us down to no permanent failures in the testsuite in CI.

There is an intermittent failure in kill01, which I need to do something 
about before turning on taking notice of the testsuite result in CI.

Effectively, all this does is:

   pid = fork();
   if (pid == 0) {
     pause();
   } else {
     kill(pid, SIGKILL);
     waitpid(pid, &status, 0);
   }

This is quite easy to demonstrate with 'winsup.api/ltp/kill01 -i 1000', 
which repeats the test, succeeding until it hangs.

Looking at the strace output, I would guess it's some race condition 
where a child process isn't yet in a position to receive a signal 
immediately after fork() returns in the parent (so the signal is 
dropped, and the parent blocks indefinitely in waitpid waiting for the 
child to terminate)

I'm not sure if that's fixable (or worth effort), so maybe just adding a 
small delay in the test is the thing to do... :)

[-- Attachment #2: 0001-Cygwin-testsuite-Add-a-small-delay-in-kill01.patch --]
[-- Type: text/plain, Size: 848 bytes --]

From 3b7d7ae0f3c29de4d8e7ff0d4487bc6f7913dc86 Mon Sep 17 00:00:00 2001
From: Jon Turney <jon.turney@dronecode.org.uk>
Date: Tue, 8 Aug 2023 16:38:20 +0100
Subject: [PATCH] Cygwin: testsuite: Add a small delay in kill01

Avoid transient failures by adding a small delay after fork()-ing to
allow the child to get into a state where it can recieve signals.
---
 winsup/testsuite/winsup.api/ltp/kill01.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/winsup/testsuite/winsup.api/ltp/kill01.c b/winsup/testsuite/winsup.api/ltp/kill01.c
index 042899173..58053eeb8 100644
--- a/winsup/testsuite/winsup.api/ltp/kill01.c
+++ b/winsup/testsuite/winsup.api/ltp/kill01.c
@@ -102,6 +102,7 @@ main(int ac, char **av)
 			/*NOTREACHED*/
 			exit(exno);
 		} else {
+			Sleep(1);
 			TEST(kill(pid, TEST_SIG));
 			waitpid(pid, &status, 0);
 		}
-- 
2.39.0

Re: [PATCH 0/4] Testsuite update

[Corinna Vinschen]

On Aug  8 17:02, Jon Turney via Cygwin-apps wrote:
> On 07/08/2023 09:55, Corinna Vinschen wrote:
> > On Aug  4 13:47, Jon Turney wrote:
> > > This gets us down to no permanent failures in the testsuite in CI.
> 
> There is an intermittent failure in kill01, which I need to do something
> about before turning on taking notice of the testsuite result in CI.
> 
> Effectively, all this does is:
> 
>   pid = fork();
>   if (pid == 0) {
>     pause();
>   } else {
>     kill(pid, SIGKILL);
>     waitpid(pid, &status, 0);
>   }
> 
> This is quite easy to demonstrate with 'winsup.api/ltp/kill01 -i 1000',
> which repeats the test, succeeding until it hangs.
> 
> Looking at the strace output, I would guess it's some race condition where a
> child process isn't yet in a position to receive a signal immediately after
> fork() returns in the parent (so the signal is dropped, and the parent
> blocks indefinitely in waitpid waiting for the child to terminate)
> 
> I'm not sure if that's fixable (or worth effort), so maybe just adding a
> small delay in the test is the thing to do... :)

It might be worth looking into it to make this more reliable, but the
patch is fine for the time being, of course.


Thanks,
Corinna

Re: [PATCH 0/4] Testsuite update

[Jon Turney]

On 08/08/2023 19:19, Corinna Vinschen via Cygwin-apps wrote:
> On Aug  8 17:02, Jon Turney via Cygwin-apps wrote:
>> On 07/08/2023 09:55, Corinna Vinschen wrote:
>>> On Aug  4 13:47, Jon Turney wrote:
>>>> This gets us down to no permanent failures in the testsuite in CI.
>>
>> There is an intermittent failure in kill01, which I need to do something
>> about before turning on taking notice of the testsuite result in CI.
>>
>> Effectively, all this does is:
>>
>>    pid = fork();
>>    if (pid == 0) {
>>      pause();
>>    } else {
>>      kill(pid, SIGKILL);
>>      waitpid(pid, &status, 0);
>>    }
>>
>> This is quite easy to demonstrate with 'winsup.api/ltp/kill01 -i 1000',
>> which repeats the test, succeeding until it hangs.
>>
>> Looking at the strace output, I would guess it's some race condition where a
>> child process isn't yet in a position to receive a signal immediately after
>> fork() returns in the parent (so the signal is dropped, and the parent
>> blocks indefinitely in waitpid waiting for the child to terminate)
>>
>> I'm not sure if that's fixable (or worth effort), so maybe just adding a
>> small delay in the test is the thing to do... :)
> 
> It might be worth looking into it to make this more reliable, but the
> patch is fine for the time being, of course.

Thanks.

Sorry for sending this to the wrong list.