add logging and assertions. There's a race condition in cref

recycling and may be one in tpool socket checking-out.
This commit is contained in:
Eric House 2010-09-20 22:11:13 -07:00
parent b6ce74be1c
commit 4ad735124c
3 changed files with 17 additions and 13 deletions

View file

@ -204,6 +204,7 @@ class SafeCref {
bool Forward( HostID src, HostID dest, unsigned char* buf, int buflen ) { bool Forward( HostID src, HostID dest, unsigned char* buf, int buflen ) {
if ( IsValid() ) { if ( IsValid() ) {
assert( 0 != m_cref->GetCookieID() );
m_cref->_Forward( src, dest, buf, buflen ); m_cref->_Forward( src, dest, buf, buflen );
return true; return true;
} else { } else {
@ -212,6 +213,7 @@ class SafeCref {
} }
bool Connect( int socket, int nPlayersH, int nPlayersS, int seed ) { bool Connect( int socket, int nPlayersH, int nPlayersS, int seed ) {
if ( IsValid() ) { if ( IsValid() ) {
assert( 0 != m_cref->GetCookieID() );
return m_cref->_Connect( socket, nPlayersH, nPlayersS, seed ); return m_cref->_Connect( socket, nPlayersH, nPlayersS, seed );
} else { } else {
return false; return false;
@ -220,6 +222,7 @@ class SafeCref {
bool Reconnect( int socket, HostID srcID, int nPlayersH, int nPlayersS, bool Reconnect( int socket, HostID srcID, int nPlayersH, int nPlayersS,
int seed ) { int seed ) {
if ( IsValid() ) { if ( IsValid() ) {
assert( 0 != m_cref->GetCookieID() );
m_cref->_Reconnect( socket, srcID, nPlayersH, nPlayersS, seed ); m_cref->_Reconnect( socket, srcID, nPlayersH, nPlayersS, seed );
return true; return true;
} else { } else {
@ -228,11 +231,13 @@ class SafeCref {
} }
void Disconnect(int socket, HostID hostID ) { void Disconnect(int socket, HostID hostID ) {
if ( IsValid() ) { if ( IsValid() ) {
assert( 0 != m_cref->GetCookieID() );
m_cref->_Disconnect( socket, hostID ); m_cref->_Disconnect( socket, hostID );
} }
} }
bool HandleAck(HostID hostID ) { bool HandleAck(HostID hostID ) {
if ( IsValid() ) { if ( IsValid() ) {
assert( 0 != m_cref->GetCookieID() );
m_cref->_HandleAck( hostID ); m_cref->_HandleAck( hostID );
return true; return true;
} else { } else {
@ -241,11 +246,13 @@ class SafeCref {
} }
void Shutdown() { void Shutdown() {
if ( IsValid() ) { if ( IsValid() ) {
assert( 0 != m_cref->GetCookieID() );
m_cref->_Shutdown(); m_cref->_Shutdown();
} }
} }
void Remove( int socket ) { void Remove( int socket ) {
if ( IsValid() ) { if ( IsValid() ) {
assert( 0 != m_cref->GetCookieID() );
m_cref->_Remove( socket ); m_cref->_Remove( socket );
} }
} }
@ -253,6 +260,7 @@ class SafeCref {
#ifdef RELAY_HEARTBEAT #ifdef RELAY_HEARTBEAT
bool HandleHeartbeat( HostID id, int socket ) { bool HandleHeartbeat( HostID id, int socket ) {
if ( IsValid() ) { if ( IsValid() ) {
assert( 0 != m_cref->GetCookieID() );
m_cref->_HandleHeartbeat( id, socket ); m_cref->_HandleHeartbeat( id, socket );
return true; return true;
} else { } else {
@ -261,6 +269,7 @@ class SafeCref {
} }
void CheckHeartbeats( time_t now ) { void CheckHeartbeats( time_t now ) {
if ( IsValid() ) { if ( IsValid() ) {
assert( 0 != m_cref->GetCookieID() );
m_cref->_CheckHeartbeats( now ); m_cref->_CheckHeartbeats( now );
} }
} }

View file

@ -228,12 +228,13 @@ XWThreadPool::real_tpool_main()
} }
if ( m_timeToDie ) { if ( m_timeToDie ) {
logf( XW_LOGINFO, "%s: unlocking b/c m_timeToDie set", __func__ );
pthread_mutex_unlock( &m_queueMutex ); pthread_mutex_unlock( &m_queueMutex );
break; break;
} }
QueuePr pr; QueuePr pr;
grab_elem_locked( socket, &pr ); grab_elem_locked( &pr );
pthread_mutex_unlock( &m_queueMutex ); pthread_mutex_unlock( &m_queueMutex );
@ -408,30 +409,24 @@ XWThreadPool::enqueue( int socket, QAction act )
} }
void void
XWThreadPool::grab_elem_locked( int curSock, QueuePr* prp ) XWThreadPool::grab_elem_locked( QueuePr* prp )
{ {
logf( XW_LOGINFO, "%s()", __func__ );
bool found = false; bool found = false;
prp->m_socket = -1; prp->m_socket = -1;
deque<QueuePr>::iterator iter; deque<QueuePr>::iterator iter;
for ( iter = m_queue.begin(); !found && iter != m_queue.end(); ++iter ) { for ( iter = m_queue.begin(); !found && iter != m_queue.end(); ++iter ) {
int socket = iter->m_socket; int socket = iter->m_socket;
if ( socket == curSock if ( m_sockets_in_use.end() == m_sockets_in_use.find( socket ) ) {
|| m_sockets_in_use.end() == m_sockets_in_use.find( socket ) ) {
*prp = *iter; *prp = *iter;
m_queue.erase( iter ); m_queue.erase( iter ); /* double-free! */
m_sockets_in_use.insert( socket ); m_sockets_in_use.insert( socket );
found = true; found = true;
} }
} }
/* I think once an event isn't "found" here there's a chance of events
sitting in the queue without the threads knowing to go after them. So
IFF this happens need to deal with it or at least confirm that there's
no chance of starvation */
// assert( found ); THIS IS FIRING
logf( XW_LOGINFO, "%s()=>%d", __func__, found ); logf( XW_LOGINFO, "%s()=>%d", __func__, found );
} } /* grab_elem_locked */
void void
XWThreadPool::release_socket_locked( int socket ) XWThreadPool::release_socket_locked( int socket )

View file

@ -61,7 +61,7 @@ class XWThreadPool {
void enqueue( int socket, QAction act = Q_READ ); void enqueue( int socket, QAction act = Q_READ );
void release_socket_locked( int socket ); void release_socket_locked( int socket );
void grab_elem_locked( int curSock, QueuePr* qpp ); void grab_elem_locked( QueuePr* qpp );
bool get_process_packet( int socket ); bool get_process_packet( int socket );
void interrupt_poll(); void interrupt_poll();