On Mon, Sep 09, 2019 at 04:54:29PM +0200, Szabolcs Nagy wrote:
> * 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.

Also, pthread_create should not pull in cancellation. Aside from being
unnecessary amounts of code that increases lots of costs in static
linking (for example, cancellable syscall paths have to be used),
there's no reason to use cancellation for something like this where
it's not trying to work with arbitrary application code, just a fixed
piece of code that admits explicit negotiation of how to continue.

> 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.

That's the side issue I noted which my third patch fixes.

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

Yes, sorry for not posting it sooner. Attached are the drafts that I
plan to push soon. (If you see something wrong and they've already
been pushed, just let me know and I'll fix it.) Patch 2 is the one
that addresses the issue reported here.

Rich