Re: CWaiter is broken in SPTK [was: Problem with starting CPPSERV]

["Ilya A. Volynets-Evenbakh" <ilya@total-knowledge.com>]

For the time being, in UU project, yes. But I think
we need to release 3.4.1 as soon as you fix XML issue.

Alexey Parshin wrote:
> Ok. So, we are Ok at the moment. :)
>
> 2007/3/13, Ilya A. Volynets-Evenbakh <ilya@total-knowledge.com>:
>>
>> Eh? Latest version (rev 266) is already fixed by me.
>> And if you think your change (rev 230) is "correct", then
>> you probably forgot the lengthly discussion on this subject
>> we had back when I wrote the original ;-)
>> The proper part of that patch is freeing of the wait condition
>> variable (my bad ;-). Lock/unlock, however, should be done by
>> the _caller_. Which, BTW, means that you have to make sure
>> you do that, in any code that calls this function.
>>
>> Alexey Parshin wrote:
>> > Interesting. The latest version is using the 'correct' way. Can you
>> > try to
>> > figure it out? If you can't - just remove that mutex lock/unlock in
>> > SVN ..
>> >
>> > 2007/3/13, Ilya A. Volynets-Evenbakh <ilya@total-knowledge.com>:
>> >>
>> >> OK. I had to look at the bug. It is, indeed breakage caused by
>> >> SPTK upgrade.
>> >>
>> >> Specific change in question is this:
>> >>
>> >>
>> http://gateway.total-knowledge.com/websvn/diff.php?repname=SPTK&path=%2Ftrunk%2Fsrc%2Futils%2FCWaiter.cpp&rev=230&sc=1
>>
>> >>
>> >>
>> >> Attached patch reverses the breakage part. I'll commit it some time
>> >> soon.
>> >> To be specific: it is expected that CWaiter::waitForSignal() caller
>> >> locks the
>> >> waiter. Otherwise the whole paradigm makes no sense.
>> >>
>> >>
>> >> --
>> >> Ilya A. Volynets-Evenbakh
>> >> Total Knowledge. CTO
>> >> http://www.total-knowledge.com
>> >>
>> >>
>> >> Index: src/utils/CWaiter.cpp
>> >> ===================================================================
>> >> --- src/utils/CWaiter.cpp       (revision 265)
>> >> +++ src/utils/CWaiter.cpp       (working copy)
>> >> @@ -64,24 +64,22 @@
>> >> }
>> >>
>> >> int CWaiter::waitForSignal(int timeout) {
>> >> -    int rc = 0;
>> >> #ifndef _WIN32
>> >> -    pthread_mutex_lock(&m_waiter);
>> >>      if (timeout > 0) {
>> >>          int   secs  = timeout / 1000;
>> >>          int   msecs = timeout % 1000;
>> >>          struct timespec   abstime = { time(NULL) + secs, msecs *
>> >> 1000L };
>> >>          int rc =
>> >> pthread_cond_timedwait(&m_condition,&m_waiter,&abstime);
>> >> -        rc = -1;
>> >> +       if(rc != 0)
>> >> +           return -1;
>> >>      } else
>> >>          pthread_cond_wait(&m_condition,&m_waiter);
>> >>
>> >> -    pthread_mutex_unlock(&m_waiter);
>> >> #else
>> >>      WaitForSingleObject(m_waiter,timeout);
>> >> #endif
>> >>
>> >> -    return rc;
>> >> +    return 0;
>> >> }
>> >>
>> >> void CWaiter::sendSignalNoLock() {
>> >>
>> >>
>> >
>> >
>>
>> -- 
>> Ilya A. Volynets-Evenbakh
>> Total Knowledge. CTO
>> http://www.total-knowledge.com
>>
>>
>
>

-- 
Ilya A. Volynets-Evenbakh
Total Knowledge. CTO
http://www.total-knowledge.com