[Xcb] Bug#496715: Bug#496715: Bug#496715: libpthread-stubs: Should provide more pthread_* functions

[Jamey Sharp]

On Thu, Oct 8, 2009 at 4:09 AM, Julien Danjou <julien at danjou.info> wrote:
> At 1253709699 time_t, Samuel Thibault wrote:
>> Here is an updated patch. Autoreconf needed of course.
>
> Can someone review this and say yes/no/wtf? :)

Sorry! I didn't notice this bug. Thanks for the reminder, Julien.

I'm not yet saying "no" or "wtf" :-) but I'd like to see justification
for these patches. "Because glibc does it" is not that convincing.
Most convincing would be a pointer to a library that should work in
both single-threaded and multi-threaded programs, and needs these
functions when running multi-threaded, but where these stubs are
appropriate for single-threaded use.

It isn't obvious to me that most of the proposed additions are useful,
which is why real use cases are important. Here are some specific
concerns, as well as which parts of the proposal I approve of as-is.

Since we don't provide pthread_create, functions operating on
pthread_attr_t don't seem useful. Why would a library that doesn't
care about threads ever use them?

By the same token, though, pthread_condattr_init and _destroy are a
good idea to include. I'd support a patch for that.

For either kind of attribute object, it's hard for me to believe that
returning 0 from the getter functions is a good idea. That's supposed
to indicate success, but the out-parameters won't have been set, so
the caller will get garbage. I'd be inclined to not provide any
getters.

I have no objection to providing setter functions though. The tricky
part is that we probably ought to provide *precisely* the collection
of setters that the platform's libpthread provides. We certainly
should never provide any that the platform does *not* have.

In hindsight, I'm concerned about the fact that we provide
pthread_cond_wait as a stub returning 0. If it's ever called in a
single-threaded application, that function should probably call
abort(). The real implementation would be guaranteed to hang in that
circumstance, after all. In XCB, we need this to be available as a
stub, but I can prove that it won't be called unless there are really
multiple threads.

So adding pthread_cond_timedwait now has the same issue, except that I
suppose a crazy person could use it to sleep for a specified amount of
time. In that case, though, immediately returning 0 would be the wrong
implementation. I'd rather not provide any implementation until
there's a real library with a sane use case for it.

Providing a pthread_exit that just calls exit seems fine judging by
the POSIX spec, except that the spec says it should call exit(0), not
exit(EXIT_SUCCESS). Just because EXIT_SUCCESS==0 on every sane
platform doesn't mean we should assume it, right?

The pthread_getschedparam and pthread_setschedparam functions don't
seem usable in a single-threaded program to me, but if they are they
probably shouldn't be implemented as no-ops returning success. I could
imagine two possibilities: either return ESRCH, because the thread
argument must be invalid, or delegate to
sched_setscheduler/sched_getscheduler with a pid of 0, and return
-errno on failure.

Since pthread_setcancelstate and pthread_setcanceltype are both
getters and setters, I'm not sure what a correct stub implementation
should do. I'd like to see a real use case for this too.

Samuel, thanks for providing these patches. I just need to see some
good justification for them.

Jamey