Hi, Doug, I think I'v got your meaning. Thanks for explaining this.
On Wed, Jan 20, 2010 at 11:01 AM, Doug Judd <[email protected]> wrote: > Hi Steve, > > Thanks for reviewing the code. The calls to > m_impl->retry_cond.wait(lock...) will relinquish the lock before going to > sleep. From the boost condition variable documentation: > > "Waiting in a condition variable is always associated with a mutex. The > mutex must be locked prior to waiting on the condition. When waiting on the > condition variable, the thread unlocks the mutex and waits atomically." > > Does that make sense, or am I not understanding the problem? > > - Doug > > On Tue, Jan 19, 2010 at 6:21 PM, Steve Chu <[email protected]> wrote: >> >> Hi, all, >> >> I'v review the hypertable code recently, some code confuses me, I >> think it may be buggy. >> >> Here's the ConnectionManager thread runner code(pu branch): >> >> void ConnectionManager::operator()() { >> ScopedLock lock(m_impl->mutex); >> ConnectionStatePtr conn_state; >> >> while (!m_impl->shutdown) { >> >> while (m_impl->retry_queue.empty()) { >> m_impl->retry_cond.wait(lock); >> if (m_impl->shutdown) >> break; >> } >> >> if (m_impl->shutdown) >> break; >> >> conn_state = m_impl->retry_queue.top(); >> >> if (conn_state->decomissioned) { >> m_impl->retry_queue.pop(); >> continue; >> } >> >> if (!conn_state->connected) { >> { >> ScopedLock conn_lock(conn_state->mutex); >> boost::xtime now; >> boost::xtime_get(&now, boost::TIME_UTC); >> >> if (xtime_cmp(conn_state->next_retry, now) <= 0) { >> m_impl->retry_queue.pop(); >> send_connect_request(conn_state.get()); >> continue; >> } >> } >> m_impl->retry_cond.timed_wait(lock, conn_state->next_retry); >> } >> else >> m_impl->retry_queue.pop(); >> } >> } >> >> I think the issue is if 'm_impl->shutdown' is not true, the >> 'ScopedLock lock(m_impl->mutex);' can not be unlocked. >> I'm not sure, but seems this will affect >> ConnectionManager::add/remove, if thread is running, can not >> add/remove any more. I haven't tested it, only from the code logic >> view. Did you guy find this problem in production release? >> >> -- >> Best Regards, >> >> Steve Chu >> http://stvchu.org >> >> -- >> You received this message because you are subscribed to the Google Groups >> "Hypertable Development" group. >> To post to this group, send email to [email protected]. >> To unsubscribe from this group, send email to >> [email protected]. >> For more options, visit this group at >> http://groups.google.com/group/hypertable-dev?hl=en. >> >> >> > > > -- > You received this message because you are subscribed to the Google Groups > "Hypertable Development" group. > To post to this group, send email to [email protected]. > To unsubscribe from this group, send email to > [email protected]. > For more options, visit this group at > http://groups.google.com/group/hypertable-dev?hl=en. > > -- Best Regards, Steve Chu http://stvchu.org
-- You received this message because you are subscribed to the Google Groups "Hypertable Development" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/hypertable-dev?hl=en.
