[Xcb] More pthread stubs.

[M Joonas Pihlaja]

On Wed, 10 Mar 2010, Jamey Sharp wrote:

> 2010/3/10 Rémi Denis-Courmont <remi at remlab.net>:

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

I'm hoping for "not very much".  As a reference point, HP's equivalent 
of pthread-stubs returns zero for nearly every function it provides a 
stub for except the thread local variable functions which actually 
have working implementations, pthread_self() (returns 1), 
pthread_equal() (checks that the args are equal), pthread_create() 
(returns ENOSYS), and pthread_attr_init() (also ENOSYS)

http://docs.hp.com/en/1896/pthreads.html

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

Personally I'm not a huge fan of providing checking since we'd need to 
actually save some state somewhere, and also we'd need to make some 
assumptions about the system's static mutex initializers.  While 
pthreads itself only supports PTHREAD_MUTEX_INITIALIZER, apps use 
non-portable initializers like PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP. 
I suppose we could autoconf our way out of this though. *sigh*

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

It appears that the main motivation is to let the application separate 
the concerns of drawing to a surface and having a thread take 
exclusive ownership of the resources underlying a "device" backing a 
cairo surface.

There are two use cases which together necessitate a recursive mutex:

1) A thread performs a cairo call without explicitly taking the device 
lock so the cairo call itself must then take the device lock.  This is 
required to keep the API simple so that non-device aware users of 
cairo can Just Work(tm).

2) A thread wants to take exclusive ownership of the device's 
resources while also making cairo calls.  The cairo calls again try to 
take the device lock as per case 1), but since the thread has already 
locked the mutex it must be recursive.

If we only needed to support either one of those use cases, then the 
mutex wouldn't need to be recursive.  We could say that the user MUST 
take the device lock before multithreaded rendering, or NEVER does. 
The former option is just plain annoying for the user, and the latter 
doesn't let the application establish a single critical region of 
ownership where they can perform cairo rendering and twiddle the 
device at the same time.

I may have gotten the details wrong since this is pretty new code in 
cairo which I'm not intimately familiar with, but as far as I can tell 
this is the justification for the recursivity of the lock.

> 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.
[snip]

Okay I can do that.  I have to say I find Lua to be much more readable 
than m4 though.

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

Could pthread-stubs gain a policy which would allow more stubs to be 
provided without apps needing to prod for each new stub (and wait for 
a subsequent release of pthread-stubs) with a specific use case?  
Something like the one you outlined last fall, and could go something 
like this: "Setters are okay.  Getters not so much.  Correctly 
functioning single threaded apps which don't rely on actual threads 
should work as long as long as they don't try to invoke the stubs with 
invalid arguments."

Cheers,

Joonas