Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20140831160630.GZ12888@brightrain.aerifal.cx>
Date: Sun, 31 Aug 2014 12:06:30 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: [PATCH 7/8] add the thrd_xxxxxx functions

On Sun, Aug 31, 2014 at 05:07:22PM +0200, Jens Gustedt wrote:
> > > > > > Also, since it doesn't depend on anything in pthread_create.c,
> > > > > 
> > > > > It does, __THRD_C11 :)
> > > > 
> > > > How about naming it to __ATTRP_C11_THREAD and putting it in
> > > > pthread_impl.h then?
> > > 
> > > ok, I'll do that
> > > 
> > > But hopefully we didn't overlook any possible use pattern that would
> > > drag in different versions of the tsd symbols. I am not too
> > > comfortable with that.
> > 
> > What do you mean here? I don't follow.
> 
> When trying to separate the two implementations, for a while I
> struggled with the fact that the game with the dummy functions
> enforces that pthread_exit and pthread_create must be in the same
> TU. I found that difficult to deal with, not knowing the code and the
> interaction between the different TU too well.
> 
> (There might be an independent possibility of improvement in
> readability and maintainability in separating pthread_create and
> pthread_exit more cleanly.)

pthread_create inherently pulls in pthread_exit because the start
function calls the latter. The other direction is not inherent -- for
example, a single-threaded program could in principle exit via a
direct call to pthread_exit or cancellation of pthread_self().

Regarding the tsd weak symbols, the logic is slightly complicated. The
main thread's tsd pointer can be initialized during the first
pthread_create call, or by pthread_key_create; it merely needs to be
valid before any call to pthread_setspecific or pthread_getspecific.
I'd kind of like to get rid of the initialization path in
pthread_create, but the one in pthread_key_create cannot find the main
thread unless the main thread is the only thread in existence when
it's called.

In any case, my quick analysis of the weak symbols leads me to believe
it's fine to split the ones used by pthread_exit and pthread_create
into separate TUs, but I'm still not convinced this is terribly useful
to do.

> > > But I just discovered another such incentive :) You were right, that
> > > the error handling for thrd_create was not correct for C11, but it
> > > wasn't my fault :) POSIX (and thus __pthread_create) basically maps
> > > all errors to EAGAIN, where C11 requires us to distinguish ENOMEM from
> > > other failures.
> > > 
> > > Thus I had to integrate this difference into __pthread_create, which
> > > was not difficult, but which intrudes even a little bit more into the
> > > existing code.
> > 
> > I think POSIX's EAGAIN is fully equivalent to C11's thrd_nomem: both
> > should reflect any condition where the thread could not be created due
> > to a resource exhaustion type failure. While you could argue that
> > thrd_nomem should only indicate failure of the mmap, not the clone, I
> > think this would be a useless distinction to callers (both of them are
> > fundamentally allocation operations) and then you'd be forced to use
> > thrd_error for clone failures, whereas I would think thrd_error should
> > be reserved for either erroneous usage (but that's UB anyway) or more
> > permanent failures (like lack of thread support on the OS).
> 
> Having read up a bit, now, I don't think so, for C threads this
> mapping is not correct.  clone has several different error returns
> that the actual code correctly maps to EAGAIN, but among them it also
> has ENOMEM.
> 
> So we have possible ENOMEM coming from clone and from mmap, and a
> bunch of other obscure errors that should go to thrd_error.

Like what? I see no possible errors except EAGAIN and ENOMEM. The only
others listed in the man page are EINVAL and EPERM and they pertain to
invalid arguments that aren't being used by pthread_create.

> > > > I'm aware that you can't cast the int* to void**, but you can still
> > > > implement the function as a trivial wrapper that doesn't introduce any
> > > > duplication of internal logic for cleaning up an exited thread:
> > > > 
> > > > int thrd_join(thrd_t t, int *res)
> > > > {
> > > > 	void *pthread_res;
> > > > 	__pthread_join(t, &pthread_res);
> > > > 	if (res) *res = (int)(intptr_t)pthread_res;
> > > > 	return thrd_success;
> > > > }
> > > 
> > > dunno, doesn't look much simpler to me and drags in one more TU into C
> > > thread applications
> > 
> > What's simpler is that this version does not:
> > 
> > - Need pthread_impl.h
> > - Have knowledge of the mechanism for waiting for thread exit.
> > - Have knowledge of how to obtain the exit code out of thread struct.
> > - Have knowledge of thread deallocation mechanism.
> 
> Ok, I'll move that separated implementation to patch 8, and use your
> version for patch 7.

OK.

Rich

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.