Re: Subject: [PATCH] pthread: Fix bug that pthread_create may cause priority inversion

[Szabolcs Nagy <nsz@port70.net>]

* zhaohang (F) <zhaohang14@huawei.com> [2019-09-09 13:57:36 +0000]:
> diff --git a/src/thread/pthread_create.c b/src/thread/pthread_create.c
> index 7d4dc2e..ae08c0f 100644
> --- a/src/thread/pthread_create.c
> +++ b/src/thread/pthread_create.c
> @@ -181,15 +181,8 @@ static int start(void *p)
>  {
>         struct start_args *args = p;
>         if (args->attr) {
> -               pthread_t self = __pthread_self();
> -               int ret = -__syscall(SYS_sched_setscheduler, self->tid,
> -                       args->attr->_a_policy, &args->attr->_a_prio);
> -               if (a_swap(args->perr, ret)==-2)
> -                       __wake(args->perr, 1, 1);
> -               if (ret) {
> -                       self->detach_state = DT_DETACHED;
> -                       __pthread_exit(0);
> -               }
> +               if (a_cas(args->perr, -1, -2) == -1)
> +                       __wait(args->perr, 0, -2, 1);
>         }
>         __syscall(SYS_rt_sigprocmask, SIG_SETMASK, &args->sig_mask, 0, _NSIG/8);
>         __pthread_exit(args->start_func(args->start_arg));
> @@ -367,10 +360,14 @@ int __pthread_create(pthread_t *restrict res, const pthread_attr_t *restrict att
>         }
> 
>         if (attr._a_sched) {
> -               if (a_cas(&err, -1, -2)==-1)
> -                       __wait(&err, 0, -2, 1);
> -               ret = err;
> -               if (ret) return ret;
> +               ret = -__syscall(SYS_sched_setscheduler, new->tid, attr._a_policy, &attr._a_prio);
> +               if (ret) {
> +                       new->detach_state = DT_DETACHED;
> +                       pthread_cancel(new);
> +                       return ret;

the child has the cancel signal blocked so it will never act on the signal.

but even if that's fixed, the detached child may not get scheduled to
handle the signal for a long time and will take up stack/tid resources.

i think Rich already has a solution that will deal with these issues.

> +               }
> +               if (a_swap(&err, 0) == -2)
> +                       __wake(&err, 1, 1);
>         }
> 
>         *res = new;