[Xcb] More pthread stubs.

[Jamey Sharp]

2010/3/10 Rémi Denis-Courmont <remi at remlab.net>:
> On Thu, 4 Mar 2010 11:39:57 +0200 (EET), M Joonas Pihlaja
> <jpihlaja at cc.helsinki.fi> wrote:
>> Recently cairo started using recursive mutexes and we found that both
>> pthread-stubs and glibc are missing stubs for creating and setting
>> mutex attributes.  We moved to directly linking to pthreads then but
>> doing it correctly is such a can of worms that it's just easier to add
>> the stubs which we use.  While doing that I've fleshed out some of the
>> other (hopefully uncontroversial) missing stubs while trying to stay
>> within the boundaries described by Jamey Sharp in a post last fall[1]
>> (No getters. Only setters, attributes, and noop-locks.)
>
> In principle, the attribute setters should check that the value is valid
> and return EINVAL if not. In that perspective, at least
> pthread_condattr_setclock() and pthread_mutexattr_settype() shouldn't
> blindly return zero. I have not checked the other ones.

I wonder. How much do we care about validating that code linked
against pthread-stubs is correctly using the API?

For example, our pthread_cond_timedwait just calls abort(), so any
change through pthread_condattr_setclock is not observable to the
application. Since the caller can't tell if we're doing it wrong,
maybe we should just pretend that any value is OK. If it meant
something to the caller, by golly, we'll take their word for it.

The alternative seems to be trying to match the system's -lpthread
behavior, anywhere that's feasible. Which sounds like a lot of work to
match error returns from functions that are unlikely to be called with
bad values and whose return statuses are rarely checked anyway. But
perhaps somebody can explain why this is important?

> Also pthread_mutex_lock() should probably to check against recursive
> locking if you add pthread_mutexattr_settype(), meaning you add
> error-checking mutexes.

As you say, Rémi, this is the exception. If somebody asks for an
error-checking mutex, we'd probably better check for self-deadlock. I
guess.

My main reaction to the proposed patches is that I'm sad to have to
start generating a real library with actual code in it on glibc
systems. I'd still like to see a response to my post on the cairo list
about this, explaining why that library actually needs recursive
mutexes. But I'm not going to push back on this just because it was
nifty to convince people to install empty packages. ;-)

I'd very much like to take the first commit in the series, "Add a tool
to create stubs.c from a list of functions," but oh please, don't
write it in Lua. Such a tool should just be part of the build, not
something hand-run where list.txt can get out of sync with stubs.c,
and that means it needs a language commonly available in build
environments. You've demonstrated that you know how to write
mostly-equivalent m4, which would be a perfectly acceptable choice.
Then remove stubs.c from version control and ensure it's generated
during build.

The rest of the commits are just a matter of debating which parts of
the POSIX thread API we should support and why; and I think that
discussion needs justification from callers like cairo about why they
care. Assuming we want all those stubs, I think these patches
correctly implement them.

Jamey

p.s. Julien, as always, thanks for keeping patches from becoming forgotten. :-)