I remember this specific issue coming up before (i.e. the pthread library 
doesn't strictly obey timeouts and may actually return early) -- though I can't 
remember exactly what we did to address it. It may have only come up internally 
at Facebook...

Aside from turning off -DNDEBUG I might recommend actually commenting out this 
assert statement and placing a comment next to it explaining that the 
underlying thread implementation doesn't actually properly support these 
semantics. If possible we should #ifdef the assert() for just the platforms 
where we know the timeout handling to be an issue -- generally a failure of 
this assert() could represent a major problem.

-----Original Message-----
From: Erik Frey [mailto:[EMAIL PROTECTED] 
Sent: Wednesday, November 26, 2008 9:59 AM
To: [email protected]
Subject: bad assert() in Monitor.cpp

I just had a very long night because of line 69 in
lib/cpp/src/concurrency/Monitor.cpp:

assert(Util::currentTime() >= (now + timeout));

There are a couple of problems with this and I thought I'd check here before 
submitting a jira:
* Release version of thrift library isn't being compiled with -DNDEBUG
* On certain kernels, waiting with a very long timeout (e.g. many minutes or 
hours) returns just -before- the expected time, hitting the assert and abruptly 
bringing down the whole process.  May be a timer resolution issue, not sure.

What do you guys suggest?  Whether -DNDEBUG is used or not, I'm not sure thrift 
release is an appropriate place to unit test pthreads.

Erik

Reply via email to