Re: cvsup broken on amd64?

On Mon, Oct 3, 2011 at 11:30 PM, Jilles Tjoelker <jilles_at_stack.nl> wrote:
> On Mon, Oct 03, 2011 at 06:15:41PM +0200, Maxime Henrion wrote:
>> Knowing all that, what's happening seems quite clear. If
>> fixups_close() is called while there was still fixup requests pending,
>> those should be processed by the detailer thread before it returns.
>> Subsequent fixups_get() call should continue returning pending items,
>> until there are no more entries in the queue _and_ the queue is
>> closed.
>
>> So, line 144 in fixups.c (in fixups_get()):
>
>>         if (f->closed) {
>
>> should actually be:
>
>>         if (f->closed && f->size == 0) {
>
> That looks sensible.
>
>> The fact that this could even happen smells a bit weird to me though;
>> it means the detailer thread didn't get a chance to run between some
>> item was added to the queue (fixups_put() signals the detailer thread
>> via pthread_cond_signal()), and that it only runs now that the updater
>> queue has been closed. I wouldn't go as far as saying this is a bug,
>> but is it even valid for the pthread library to "coalesce" two
>> pthread_cond_signal() calls into one when the thread didn't get to run
>> since the first call was made? If so, then the above fix is perfectly
>> valid. If not, there is a deeper bug in the threading library, or in
>> the CVS mode code which I didn't write (but that seems far-fetched).
>
> The pthread library is free to "coalesce" pthread_cond_signal() calls
> like that. This is because a thread awakened by pthread_cond_signal()
> (or any other event) is not guaranteed to start running immediately and
> pthread_cond_signal() does nothing if there is no thread to wake up.

Great, that's a relief. I knew the pthread library was free to wake a
thread up even if it hadn't been signaled, which is why one always has
to call pthread_cond_wait() inside of a while() loop checking for the
condition, but wasn't sure about that particular scenario, so I'm glad
to hear a confirmation. Thanks!

Cheers,
Maxime