diff mbox series

pthread/tst-cancel28: Fix barrier re-init race condition

Message ID 20210925080206.3213821-1-shorne@gmail.com
State Committed
Commit 9874ca536b1d0662b1cea46af3ce09a4d42aeb32
Delegated to: Adhemerval Zanella Netto
Headers
Series pthread/tst-cancel28: Fix barrier re-init race condition |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent
dj/TryBot-32bit success Build for i686

Commit Message

Stafford Horne Sept. 25, 2021, 8:02 a.m. UTC 
  When running this test on the OpenRISC port I am working on this test
fails with a timeout.  The test passes when being straced or debugged.
Looking at the code there seems to be a race condition in that:

  1 main thread: calls xpthread_cancel
  2 sub thread : receives cancel signal
  3 sub thread : cleanup routine waits on barrier
  4 main thread: re-inits barrier
  5 main thread: waits on barrier

After getting to 5 the main thread and sub thread wait forever as the 2
barriers are no longer the same.

Removing the barrier re-init seems to fix this issue.  Also, the barrier
does not need to be reinitialized as that is done by default.

Cc: Adhemerval Zanella <adhemerval.zanella@linaro.org>
---
 sysdeps/pthread/tst-cancel28.c | 1 -
 1 file changed, 1 deletion(-)

Comments

Stafford Horne Sept. 27, 2021, 8:42 p.m. UTC | #1 
ping

On Sat, Sep 25, 2021 at 5:02 PM Stafford Horne <shorne@gmail.com> wrote:
>
> When running this test on the OpenRISC port I am working on this test
> fails with a timeout.  The test passes when being straced or debugged.
> Looking at the code there seems to be a race condition in that:
>
>   1 main thread: calls xpthread_cancel
>   2 sub thread : receives cancel signal
>   3 sub thread : cleanup routine waits on barrier
>   4 main thread: re-inits barrier
>   5 main thread: waits on barrier
>
> After getting to 5 the main thread and sub thread wait forever as the 2
> barriers are no longer the same.
>
> Removing the barrier re-init seems to fix this issue.  Also, the barrier
> does not need to be reinitialized as that is done by default.
>
> Cc: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> ---
>  sysdeps/pthread/tst-cancel28.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/sysdeps/pthread/tst-cancel28.c b/sysdeps/pthread/tst-cancel28.c
> index 627cbc8160..9286c159b3 100644
> --- a/sysdeps/pthread/tst-cancel28.c
> +++ b/sysdeps/pthread/tst-cancel28.c
> @@ -69,7 +69,6 @@ do_test (void)
>
>    xpthread_cancel (timer_thread);
>
> -  xpthread_barrier_init (&barrier, NULL, 2);
>    xpthread_barrier_wait (&barrier);
>
>    return 0;
> --
> 2.31.1
>
Adhemerval Zanella Sept. 28, 2021, 1:47 p.m. UTC | #2 
On 25/09/2021 05:02, Stafford Horne wrote:
> When running this test on the OpenRISC port I am working on this test
> fails with a timeout.  The test passes when being straced or debugged.
> Looking at the code there seems to be a race condition in that:
> 
>   1 main thread: calls xpthread_cancel
>   2 sub thread : receives cancel signal
>   3 sub thread : cleanup routine waits on barrier
>   4 main thread: re-inits barrier
>   5 main thread: waits on barrier
> 
> After getting to 5 the main thread and sub thread wait forever as the 2
> barriers are no longer the same.
> 
> Removing the barrier re-init seems to fix this issue.  Also, the barrier
> does not need to be reinitialized as that is done by default.
> 
> Cc: Adhemerval Zanella <adhemerval.zanella@linaro.org>

LGTM, thanks.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  sysdeps/pthread/tst-cancel28.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/sysdeps/pthread/tst-cancel28.c b/sysdeps/pthread/tst-cancel28.c
> index 627cbc8160..9286c159b3 100644
> --- a/sysdeps/pthread/tst-cancel28.c
> +++ b/sysdeps/pthread/tst-cancel28.c
> @@ -69,7 +69,6 @@ do_test (void)
>  
>    xpthread_cancel (timer_thread);
>  
> -  xpthread_barrier_init (&barrier, NULL, 2);
>    xpthread_barrier_wait (&barrier);
>  
>    return 0;
>
diff mbox series

Patch

diff --git a/sysdeps/pthread/tst-cancel28.c b/sysdeps/pthread/tst-cancel28.c
index 627cbc8160..9286c159b3 100644
--- a/sysdeps/pthread/tst-cancel28.c
+++ b/sysdeps/pthread/tst-cancel28.c
@@ -69,7 +69,6 @@  do_test (void)
 
   xpthread_cancel (timer_thread);
 
-  xpthread_barrier_init (&barrier, NULL, 2);
   xpthread_barrier_wait (&barrier);
 
   return 0;