diff --git a/xwords4/common/comms.c b/xwords4/common/comms.c index e843be9d7..6fd65cb32 100644 --- a/xwords4/common/comms.c +++ b/xwords4/common/comms.c @@ -1,6 +1,6 @@ /* -*- compile-command: "cd ../linux && make MEMDEBUG=TRUE"; -*- */ /* - * Copyright 2001-2009 by Eric House (xwords@eehouse.org). All rights + * Copyright 2001-2011 by Eric House (xwords@eehouse.org). All rights * reserved. * * This program is free software; you can redistribute it and/or @@ -236,6 +236,7 @@ XWREASON2Str( XWREASON reason ) CASE_STR(XWRELAY_ERROR_DUP_ROOM); CASE_STR(XWRELAY_ERROR_TOO_MANY); CASE_STR(XWRELAY_ERROR_DELETED); + CASE_STR(XWRELAY_ERROR_NORECONN); CASE_STR(XWRELAY_ERROR_LASTERR); default: XP_ASSERT(0); @@ -360,7 +361,7 @@ reset_internal( CommsCtxt* comms, XP_Bool isServer, } #endif LOG_RETURN_VOID(); -} /* comms_reset */ +} /* reset_internal */ void comms_reset( CommsCtxt* comms, XP_Bool isServer, @@ -1247,7 +1248,7 @@ relayPreProcess( CommsCtxt* comms, XWStreamCtxt* stream, XWHostID* senderID ) XP_Bool consumed = XP_TRUE; XWHostID destID, srcID; CookieID cookieID; - XP_U8 relayErr; + XWREASON relayErr; /* nothing for us to do here if not using relay */ XWRELAY_Cmd cmd = stream_getU8( stream ); @@ -1355,10 +1356,16 @@ relayPreProcess( CommsCtxt* comms, XWStreamCtxt* stream, XWHostID* senderID ) case XWRELAY_CONNECTDENIED: /* socket will get closed by relay */ relayErr = stream_getU8( stream ); + XP_LOGF( "%s: got reason: %s", __func__, XWREASON2Str( relayErr ) ); set_relay_state( comms, COMMS_RELAYSTATE_DENIED ); - util_userError( comms->util, ERR_RELAY_BASE + relayErr ); - /* requires action, not just notification */ - (*comms->procs.rerror)( comms->procs.closure, relayErr ); + + if ( XWRELAY_ERROR_NORECONN == relayErr ) { + init_relay( comms, comms->r.nPlayersHere, comms->r.nPlayersTotal ); + } else { + util_userError( comms->util, ERR_RELAY_BASE + relayErr ); + /* requires action, not just notification */ + (*comms->procs.rerror)( comms->procs.closure, relayErr ); + } break; /* fallthru */ diff --git a/xwords4/relay/Makefile b/xwords4/relay/Makefile index 4c6f49f71..a8b10ab2b 100644 --- a/xwords4/relay/Makefile +++ b/xwords4/relay/Makefile @@ -22,7 +22,6 @@ SRC = \ configs.cpp \ cref.cpp \ crefmgr.cpp \ - ctrl.cpp \ dbmgr.cpp \ http.cpp \ lstnrmgr.cpp \ diff --git a/xwords4/relay/cidlock.cpp b/xwords4/relay/cidlock.cpp index cb63ed3d7..3e8b1cb3f 100644 --- a/xwords4/relay/cidlock.cpp +++ b/xwords4/relay/cidlock.cpp @@ -112,10 +112,9 @@ CidLock::ClaimSocket( int sock ) return info; } -void -CidLock::Associate( const CookieRef* cref, int socket ) +bool +CidLock::Associate_locked( const CookieRef* cref, int socket ) { - MutexLock ml( &m_infos_mutex ); map< CookieID, CidInfo*>::iterator iter = m_infos.begin(); while ( iter != m_infos.end() ) { if ( cref == iter->second->GetRef() ) { @@ -124,16 +123,30 @@ CidLock::Associate( const CookieRef* cref, int socket ) } ++iter; } + bool isNew = m_sockets.find( socket ) == m_sockets.end(); + if ( isNew ) { + m_sockets.insert( socket ); + } + return isNew; +} + +bool +CidLock::Associate( const CookieRef* cref, int socket ) +{ + MutexLock ml( &m_infos_mutex ); + return Associate_locked( cref, socket ); } void CidLock::DisAssociate( const CookieRef* cref, int socket ) { - Associate( cref, 0 ); + MutexLock ml( &m_infos_mutex ); + Associate_locked( cref, 0 ); + m_sockets.erase( socket ); } void -CidLock::Relinquish( CidInfo* claim ) +CidLock::Relinquish( CidInfo* claim, bool drop ) { CookieID cid = claim->GetCid(); logf( XW_LOGINFO, "%s(%d)", __func__, cid ); @@ -141,7 +154,12 @@ CidLock::Relinquish( CidInfo* claim ) MutexLock ml( &m_infos_mutex ); map< CookieID, CidInfo*>::iterator iter = m_infos.find( cid ); assert( iter != m_infos.end() ); - iter->second->SetOwner( 0 ); + if ( drop ) { + delete iter->second; + m_infos.erase( iter ); + } else { + iter->second->SetOwner( 0 ); + } print_claimed(); pthread_cond_signal( &m_infos_condvar ); logf( XW_LOGINFO, "%s(%d): DONE", __func__, cid ); diff --git a/xwords4/relay/cidlock.h b/xwords4/relay/cidlock.h index cadacd51f..42ffe8a34 100644 --- a/xwords4/relay/cidlock.h +++ b/xwords4/relay/cidlock.h @@ -23,6 +23,7 @@ #define _CIDLOCK_H_ #include +#include #include "xwrelay.h" #include "cref.h" @@ -61,22 +62,24 @@ class CidLock { } return s_instance; } + ~CidLock(); CidInfo* Claim( void ) { return Claim(0); } CidInfo* Claim( CookieID cid ); CidInfo* ClaimSocket( int sock ); - void Relinquish( CidInfo* claim ); + void Relinquish( CidInfo* claim, bool drop ); - void Associate( const CookieRef* cref, int socket ); + bool Associate( const CookieRef* cref, int socket ); /* return true if new association */ void DisAssociate( const CookieRef* cref, int socket ); private: CidLock(); - ~CidLock(); void print_claimed(); + bool Associate_locked( const CookieRef* cref, int socket ); static CidLock* s_instance; - map< CookieID, CidInfo*> m_infos; + set m_sockets; + map< CookieID, CidInfo* > m_infos; pthread_mutex_t m_infos_mutex; pthread_cond_t m_infos_condvar; int m_nextCID; diff --git a/xwords4/relay/cref.cpp b/xwords4/relay/cref.cpp index 30d590061..14ecffbb4 100644 --- a/xwords4/relay/cref.cpp +++ b/xwords4/relay/cref.cpp @@ -47,30 +47,30 @@ using namespace std; * SocketsIterator class *****************************************************************************/ -SocketsIterator::SocketsIterator( SocketMap::iterator iter, - SocketMap::iterator end, - pthread_mutex_t* mutex ) - : m_iter( iter ) - , m_end( end ) - , m_mutex( mutex ) -{ -} +/* SocketsIterator::SocketsIterator( SocketMap::iterator iter, */ +/* SocketMap::iterator end, */ +/* pthread_mutex_t* mutex ) */ +/* : m_iter( iter ) */ +/* , m_end( end ) */ +/* , m_mutex( mutex ) */ +/* { */ +/* } */ -SocketsIterator::~SocketsIterator() -{ - pthread_mutex_unlock( m_mutex ); -} +/* SocketsIterator::~SocketsIterator() */ +/* { */ +/* pthread_mutex_unlock( m_mutex ); */ +/* } */ -int -SocketsIterator::Next() -{ - int socket = 0; - if ( m_iter != m_end ) { - socket = m_iter->first; - ++m_iter; - } - return socket; -} +/* int */ +/* SocketsIterator::Next() */ +/* { */ +/* int socket = 0; */ +/* if ( m_iter != m_end ) { */ +/* socket = m_iter->first; */ +/* ++m_iter; */ +/* } */ +/* return socket; */ +/* } */ /***************************************************************************** * CookieRef class @@ -209,20 +209,28 @@ CookieRef::_Connect( int socket, int nPlayersH, int nPlayersS, int seed, return connected; } -void +bool CookieRef::_Reconnect( int socket, HostID hid, int nPlayersH, int nPlayersS, int seed, bool gameDead ) { - if ( AlreadyHere( hid, seed, socket ) ) { - logf( XW_LOGINFO, "dropping reconnection because already here" ); + bool spotTaken = false; + bool alreadyHere = AlreadyHere( hid, seed, socket, &spotTaken ); + if ( spotTaken ) { + logf( XW_LOGINFO, "%s: failing because spot taken", __func__ ); } else { - (void)CRefMgr::Get()->Associate( socket, this ); - pushReconnectEvent( socket, hid, nPlayersH, nPlayersS, seed ); + if ( alreadyHere ) { + logf( XW_LOGINFO, "%s: dropping because already here", + __func__ ); + } else { + (void)CRefMgr::Get()->Associate( socket, this ); + pushReconnectEvent( socket, hid, nPlayersH, nPlayersS, seed ); + } + if ( gameDead ) { + pushGameDead( socket ); + } + handleEvents(); } - if ( gameDead ) { - pushGameDead( socket ); - } - handleEvents(); + return !spotTaken; } void @@ -326,7 +334,8 @@ CookieRef::AlreadyHere( unsigned short seed, int socket, HostID* prevHostID ) } bool -CookieRef::AlreadyHere( HostID hid, unsigned short seed, int socket ) +CookieRef::AlreadyHere( HostID hid, unsigned short seed, int socket, + bool* spotTaken ) { logf( XW_LOGINFO, "%s(hid=%d,seed=%x,socket=%d)", __func__, hid, seed, socket ); @@ -335,8 +344,9 @@ CookieRef::AlreadyHere( HostID hid, unsigned short seed, int socket ) vector::iterator iter; for ( iter = m_sockets.begin(); iter != m_sockets.end(); ++iter ) { if ( iter->m_hostID == hid ) { - assert( seed == iter->m_seed ); - if ( socket == iter->m_socket ) { + if ( seed != iter->m_seed ) { + *spotTaken = true; + } else if ( socket == iter->m_socket ) { here = true; /* dup packet */ } else { logf( XW_LOGINFO, "%s: hids match; nuking existing record" @@ -386,6 +396,7 @@ CookieRef::removeSocket( int socket ) iter->m_hostID ); DBMgr::Get()->RmDeviceByHid( ConnName(), iter->m_hostID ); m_nPlayersHere -= iter->m_nPlayersH; + cancelAckTimer( iter->m_hostID ); } m_sockets.erase(iter); --count; @@ -918,7 +929,7 @@ void CookieRef::setAckTimer( HostID hid ) { ASSERT_LOCKED(); - logf( XW_LOGINFO, "%s(%d)", __func__, hid ); + logf( XW_LOGINFO, "%s(hid=%d)", __func__, hid ); assert( hid >= HOST_ID_SERVER ); assert( hid <= 4 ); @@ -940,12 +951,11 @@ void CookieRef::cancelAckTimer( HostID hid ) { ASSERT_LOCKED(); - logf( XW_LOGINFO, "%s(%d)", __func__, hid ); + logf( XW_LOGINFO, "%s(hid=%d)", __func__, hid ); assert( hid >= HOST_ID_SERVER ); assert( hid <= 4 ); - --hid; - m_timers[hid].m_this = NULL; + m_timers[hid-1].m_this = NULL; // TimerMgr::GetTimerMgr()->ClearTimer( s_checkAck, this ); } @@ -1088,19 +1098,19 @@ CookieRef::notifyGameDead( int socket ) send_with_length( socket, buf, sizeof(buf), true ); } -void -CookieRef::moveSockets( void ) -{ - ASSERT_LOCKED(); +/* void */ +/* CookieRef::moveSockets( void ) */ +/* { */ +/* ASSERT_LOCKED(); */ - vector sockets; - vector::iterator iter; - for ( iter = m_sockets.begin(); iter != m_sockets.end(); ++iter ) { - sockets.push_back( iter->m_socket ); - } +/* vector sockets; */ +/* vector::iterator iter; */ +/* for ( iter = m_sockets.begin(); iter != m_sockets.end(); ++iter ) { */ +/* sockets.push_back( iter->m_socket ); */ +/* } */ - CRefMgr::Get()->MoveSockets( sockets, this ); -} +/* CRefMgr::Get()->MoveSockets( sockets, this ); */ +/* } */ void CookieRef::sendAllHere( bool initial ) @@ -1326,9 +1336,9 @@ CookieRef::printSeeds( const char* caller ) char buf[64] = {0}; vector::iterator iter; for ( iter = m_sockets.begin(); iter != m_sockets.end(); ++iter ) { - len += snprintf( &buf[len], sizeof(buf)-len, "%.4x/%d/%c ", - iter->m_seed, iter->m_socket, - iter->m_ackPending?'a':'A' ); + len += snprintf( &buf[len], sizeof(buf)-len, "[%d]%.4x(%d)/%d/%c ", + iter->m_hostID, iter->m_seed, iter->m_seed, + iter->m_socket, iter->m_ackPending?'a':'A' ); } logf( XW_LOGINFO, "seeds/sockets/ack'd after %s(): %s", caller, buf ); } diff --git a/xwords4/relay/cref.h b/xwords4/relay/cref.h index 4c5224c3c..dc0685b58 100644 --- a/xwords4/relay/cref.h +++ b/xwords4/relay/cref.h @@ -103,7 +103,7 @@ class CookieRef { /* connect case */ bool AlreadyHere( unsigned short seed, int socket, HostID* prevHostID ); /* reconnect case */ - bool AlreadyHere( HostID hid, unsigned short seed, int socket ); + bool AlreadyHere( HostID hid, unsigned short seed, int socket, bool* spotTaken ); /* for console */ void _PrintCookieInfo( string& out ); @@ -118,7 +118,7 @@ class CookieRef { bool _Connect( int socket, int nPlayersH, int nPlayersS, int seed, bool seenSeed ); - void _Reconnect( int socket, HostID srcID, int nPlayersH, int nPlayersS, + bool _Reconnect( int socket, HostID srcID, int nPlayersH, int nPlayersS, int seed, bool gameDead ); void _HandleAck( HostID hostID ); void _Disconnect(int socket, HostID hostID ); diff --git a/xwords4/relay/crefmgr.cpp b/xwords4/relay/crefmgr.cpp index d6b889edf..165721a97 100644 --- a/xwords4/relay/crefmgr.cpp +++ b/xwords4/relay/crefmgr.cpp @@ -59,7 +59,7 @@ CRefMgr::CRefMgr() , m_startTime(time(NULL)) { /* should be using pthread_once() here */ - pthread_mutex_init( &m_SocketStuffMutex, NULL ); + /* pthread_mutex_init( &m_SocketStuffMutex, NULL ); */ pthread_mutex_init( &m_roomsFilledMutex, NULL ); pthread_mutex_init( &m_freeList_mutex, NULL ); pthread_rwlock_init( &m_cookieMapRWLock, NULL ); @@ -72,15 +72,16 @@ CRefMgr::~CRefMgr() assert( this == s_instance ); delete m_db; + delete m_cidlock; pthread_mutex_destroy( &m_freeList_mutex ); pthread_rwlock_destroy( &m_cookieMapRWLock ); - SocketMap::iterator iter; - for ( iter = m_SocketStuff.begin(); iter != m_SocketStuff.end(); ++iter ) { - SocketStuff* stuff = iter->second; - delete stuff; - } + /* SocketMap::iterator iter; */ + /* for ( iter = m_SocketStuff.begin(); iter != m_SocketStuff.end(); ++iter ) { */ + /* SocketStuff* stuff = iter->second; */ + /* delete stuff; */ + /* } */ s_instance = NULL; } @@ -235,9 +236,6 @@ CRefMgr::getMakeCookieRef( const char* cookie, HostID hid, int socket, bool makePublic, bool* seenSeed ) { CidInfo* cinfo; - CookieID cid; - char connNameBuf[MAX_CONNNAME_LEN+1] = {0}; - int alreadyHere = 0; /* We have a cookie from a new connection or from a reconnect. This may be the first time it's been seen, or there may be a game currently in @@ -246,32 +244,54 @@ CRefMgr::getMakeCookieRef( const char* cookie, HostID hid, int socket, a new one. Pass the connName which will be used if set, but if not set we'll be generating another later when the game is complete. */ + for ( ; ; ) { + /* What's this for loop thing. It's to fix a race condition. One + thread has "claim" on cid , which is in the DB. Another comes + into this function and looks it up in the DB, retrieving , but + progress is blocked inside getCookieRef_impl which calls Claim(). + The first thread winds up removing from the DB and deleting its + cref before calling Relinquish so that when Claim() returns there's + no cref. So we test for that case and retry. */ - *seenSeed = m_db->SeenSeed( cookie, seed, langCode, nPlayersT, - wantsPublic, connNameBuf, - sizeof(connNameBuf), &alreadyHere, &cid ); - if ( !*seenSeed ) { - cid = m_db->FindOpen( cookie, langCode, nPlayersT, nPlayersH, - wantsPublic, connNameBuf, sizeof(connNameBuf), - &alreadyHere ); - } - if ( cid > 0 ) { - cinfo = getCookieRef_impl( cid ); - } else { - cinfo = m_cidlock->Claim(); - cid = cinfo->GetCid(); - CookieRef* cref = AddNew( cookie, connNameBuf, cid, langCode, - nPlayersT, alreadyHere ); - cinfo->SetRef( cref ); - if ( !connNameBuf[0] ) { /* didn't exist in DB */ - m_db->AddNew( cookie, cref->ConnName(), cid, langCode, nPlayersT, - wantsPublic || makePublic ); - } else { - m_db->AddCID( connNameBuf, cid ); + CookieID cid; + char connNameBuf[MAX_CONNNAME_LEN+1] = {0}; + int alreadyHere = 0; + + *seenSeed = m_db->SeenSeed( cookie, seed, langCode, nPlayersT, + wantsPublic, connNameBuf, + sizeof(connNameBuf), &alreadyHere, &cid ); + if ( !*seenSeed ) { + cid = m_db->FindOpen( cookie, langCode, nPlayersT, nPlayersH, + wantsPublic, connNameBuf, sizeof(connNameBuf), + &alreadyHere ); } - } + if ( cid > 0 ) { + cinfo = m_cidlock->Claim( cid ); + if ( NULL == cinfo->GetRef() ) { + m_cidlock->Relinquish( cinfo, true ); + continue; + } + } else { + cinfo = m_cidlock->Claim(); + cid = cinfo->GetCid(); + CookieRef* cref = AddNew( cookie, connNameBuf, cid, langCode, + nPlayersT, alreadyHere ); + cinfo->SetRef( cref ); + if ( !connNameBuf[0] ) { /* didn't exist in DB */ + m_db->AddNew( cookie, cref->ConnName(), cid, langCode, nPlayersT, + wantsPublic || makePublic ); + } else { + if ( !m_db->AddCID( connNameBuf, cid ) ) { + m_cidlock->Relinquish( cinfo, true ); + continue; + } + } + } + break; + } + assert( cinfo->GetRef() ); return cinfo; } /* getMakeCookieRef */ @@ -285,37 +305,48 @@ CRefMgr::getMakeCookieRef( const char* connName, const char* cookie, CookieRef* cref = NULL; CidInfo* cinfo; - /* fetch these from DB */ - char curCookie[MAX_INVITE_LEN+1]; - int curLangCode; - int nPlayersT = 0; - int nAlreadyHere = 0; + for ( ; ; ) { /* for: see comment above */ + /* fetch these from DB */ + char curCookie[MAX_INVITE_LEN+1]; + int curLangCode; + int nPlayersT = 0; + int nAlreadyHere = 0; - CookieID cid = m_db->FindGame( connName, curCookie, sizeof(curCookie), - &curLangCode, &nPlayersT, &nAlreadyHere, - isDead ); - if ( 0 != cid ) { /* already open */ - cinfo = getCookieRef_impl( cid ); - } else { - CookieID cid; - /* The entry may not even be in the DB, e.g. if it got deleted. - Deal with that possibility by taking the caller's word for it. */ - cinfo = m_cidlock->Claim(); - cid = cinfo->GetCid(); - - if ( nPlayersT == 0 ) { /* wasn't in the DB */ - m_db->AddNew( cookie, connName, cid, langCode, nPlayersS, isPublic ); - curLangCode = langCode; - nPlayersT = nPlayersS; + CookieID cid = m_db->FindGame( connName, curCookie, sizeof(curCookie), + &curLangCode, &nPlayersT, &nAlreadyHere, + isDead ); + if ( 0 != cid ) { /* already open */ + cinfo = m_cidlock->Claim( cid ); + if ( NULL == cinfo->GetRef() ) { + m_cidlock->Relinquish( cinfo, true ); + continue; + } } else { - cookie = curCookie; - } + CookieID cid; + /* The entry may not even be in the DB, e.g. if it got deleted. + Deal with that possibility by taking the caller's word for it. */ + cinfo = m_cidlock->Claim(); + cid = cinfo->GetCid(); - cref = AddNew( cookie, connName, cid, curLangCode, nPlayersT, - nAlreadyHere ); - cinfo->SetRef( cref ); - m_db->AddCID( connName, cid ); - } + if ( nPlayersT == 0 ) { /* wasn't in the DB */ + m_db->AddNew( cookie, connName, cid, langCode, nPlayersS, isPublic ); + curLangCode = langCode; + nPlayersT = nPlayersS; + } else { + if ( !m_db->AddCID( connName, cid ) ) { + m_cidlock->Relinquish( cinfo, true ); + continue; + } + cookie = curCookie; + } + + cref = AddNew( cookie, connName, cid, curLangCode, nPlayersT, + nAlreadyHere ); + cinfo->SetRef( cref ); + } + break; + } /* for */ + assert( cinfo->GetRef() ); return cinfo; } /* getMakeCookieRef */ @@ -329,86 +360,97 @@ CRefMgr::getMakeCookieRef( const char* const connName, bool* isDead ) int nPlayersT = 0; int nAlreadyHere = 0; - CookieID cid = m_db->FindGame( connName, curCookie, sizeof(curCookie), - &curLangCode, &nPlayersT, &nAlreadyHere, - isDead ); - if ( 0 != cid ) { /* already open */ - cinfo = getCookieRef_impl( cid ); - } else { - if ( nPlayersT == 0 ) { /* wasn't in the DB */ - /* do nothing; insufficient info to fake it */ + for ( ; ; ) { /* for: see comment above */ + CookieID cid = m_db->FindGame( connName, curCookie, sizeof(curCookie), + &curLangCode, &nPlayersT, &nAlreadyHere, + isDead ); + if ( 0 != cid ) { /* already open */ + cinfo = m_cidlock->Claim( cid ); + if ( NULL == cinfo->GetRef() ) { + m_cidlock->Relinquish( cinfo, true ); + continue; + } } else { - cinfo = m_cidlock->Claim(); - cref = AddNew( curCookie, connName, cinfo->GetCid(), curLangCode, - nPlayersT, nAlreadyHere ); - cinfo->SetRef( cref ); - m_db->AddCID( connName, cinfo->GetCid() ); + if ( nPlayersT == 0 ) { /* wasn't in the DB */ + /* do nothing; insufficient info to fake it */ + } else { + cinfo = m_cidlock->Claim(); + if ( !m_db->AddCID( connName, cinfo->GetCid() ) ) { + m_cidlock->Relinquish( cinfo, true ); + continue; + } + cref = AddNew( curCookie, connName, cinfo->GetCid(), curLangCode, + nPlayersT, nAlreadyHere ); + cinfo->SetRef( cref ); + } } + break; } + assert( cinfo->GetRef() ); return cinfo; } bool CRefMgr::Associate( int socket, CookieRef* cref ) { - m_cidlock->Associate( cref, socket ); - MutexLock ml( &m_SocketStuffMutex ); - return Associate_locked( socket, cref ); + return m_cidlock->Associate( cref, socket ); + /* MutexLock ml( &m_SocketStuffMutex ); */ + /* return Associate_locked( socket, cref ); */ } -bool -CRefMgr::Associate_locked( int socket, CookieRef* cref ) -{ - bool isNew = false; - SocketMap::iterator iter = m_SocketStuff.find( socket ); - /* This isn't enough. Must provide a way to reuse sockets should a - genuinely different connection appear. Now maybe we already remove - this reference when a socket is closed. Test this! Or assert - something here. Bottom line: need to swallow repeated/duplicate - connect messages from same host. */ - if ( iter == m_SocketStuff.end() ) { - SocketStuff* stuff = new SocketStuff( cref ); - m_SocketStuff.insert( pair< int, SocketStuff* >( socket, stuff ) ); - isNew = true; - } else { - logf( XW_LOGERROR, "Already have cref/threadID pair for socket %d; " - "error???", socket ); - } - return isNew; -} +/* bool */ +/* CRefMgr::Associate_locked( int socket, CookieRef* cref ) */ +/* { */ +/* bool isNew = false; */ +/* SocketMap::iterator iter = m_SocketStuff.find( socket ); */ +/* /\* This isn't enough. Must provide a way to reuse sockets should a */ +/* genuinely different connection appear. Now maybe we already remove */ +/* this reference when a socket is closed. Test this! Or assert */ +/* something here. Bottom line: need to swallow repeated/duplicate */ +/* connect messages from same host. *\/ */ +/* if ( iter == m_SocketStuff.end() ) { */ +/* SocketStuff* stuff = new SocketStuff( cref ); */ +/* m_SocketStuff.insert( pair< int, SocketStuff* >( socket, stuff ) ); */ +/* isNew = true; */ +/* } else { */ +/* logf( XW_LOGERROR, "Already have cref/threadID pair for socket %d; " */ +/* "error???", socket ); */ +/* } */ +/* return isNew; */ +/* } */ -void -CRefMgr::Disassociate_locked( int socket, CookieRef* cref ) -{ - m_cidlock->DisAssociate( cref, socket ); - SocketMap::iterator iter = m_SocketStuff.find( socket ); - if ( iter == m_SocketStuff.end() ) { - logf( XW_LOGERROR, "can't find SocketStuff for socket %d", socket ); - } else { - SocketStuff* stuff = iter->second; - assert( cref == NULL || stuff->m_cref == cref ); - delete stuff; - m_SocketStuff.erase( iter ); - } -} +/* void */ +/* CRefMgr::Disassociate_locked( int socket, CookieRef* cref ) */ +/* { */ +/* SocketMap::iterator iter = m_SocketStuff.find( socket ); */ +/* if ( iter == m_SocketStuff.end() ) { */ +/* logf( XW_LOGERROR, "can't find SocketStuff for socket %d", socket ); */ +/* } else { */ +/* SocketStuff* stuff = iter->second; */ +/* assert( cref == NULL || stuff->m_cref == cref ); */ +/* delete stuff; */ +/* m_SocketStuff.erase( iter ); */ +/* } */ +/* } */ void CRefMgr::Disassociate( int socket, CookieRef* cref ) { - MutexLock ml( &m_SocketStuffMutex ); - Disassociate_locked( socket, cref ); + m_cidlock->DisAssociate( cref, socket ); + /* MutexLock ml( &m_SocketStuffMutex ); */ + /* Disassociate_locked( socket, cref ); */ } -void -CRefMgr::MoveSockets( vector sockets, CookieRef* cref ) -{ - MutexLock ml( &m_SocketStuffMutex ); - vector::iterator iter; - for ( iter = sockets.begin(); iter != sockets.end(); ++iter ) { - Disassociate_locked( *iter, NULL ); - Associate_locked( *iter, cref ); - } -} +/* void */ +/* CRefMgr::MoveSockets( vector sockets, CookieRef* cref ) */ +/* { */ +/* MutexLock ml( &m_SocketStuffMutex ); */ +/* vector::iterator iter; */ +/* for ( iter = sockets.begin(); iter != sockets.end(); ++iter ) { */ +/* Disassociate_locked( *iter, NULL ); */ +/* Associate_locked( *iter, cref ); */ +/* } */ +/* } */ #if 0 pthread_mutex_t* @@ -457,19 +499,20 @@ CRefMgr::PrintSocketInfo( int socket, string& out ) } } -/* static */ SocketsIterator -CRefMgr::MakeSocketsIterator() -{ - pthread_mutex_lock( &m_SocketStuffMutex ); - SocketsIterator iter( m_SocketStuff.begin(), m_SocketStuff.end(), - &m_SocketStuffMutex ); - return iter; -} +/* /\* static *\/ SocketsIterator */ +/* CRefMgr::MakeSocketsIterator() */ +/* { */ +/* assert( 0 ); /\* called? *\/ */ +/* pthread_mutex_lock( &m_SocketStuffMutex ); */ +/* SocketsIterator iter( m_SocketStuff.begin(), m_SocketStuff.end(), */ +/* &m_SocketStuffMutex ); */ +/* return iter; */ +/* } */ CidInfo* -CRefMgr::getCookieRef( CookieID cookieID ) +CRefMgr::getCookieRef( CookieID cid ) { - return getCookieRef_impl( cookieID ); + return m_cidlock->Claim( cid ); } /* getCookieRef */ CidInfo* @@ -485,6 +528,7 @@ CRefMgr::getCookieRef( int socket ) /* } */ /* } */ + assert( NULL == cinfo || NULL != cinfo->GetRef() ); return cinfo; } /* getCookieRef */ @@ -498,7 +542,7 @@ CRefMgr::heartbeatProc( void* closure ) #endif CookieRef* -CRefMgr::AddNew( const char* cookie, const char* connName, CookieID id, +CRefMgr::AddNew( const char* cookie, const char* connName, CookieID cid, int langCode, int nPlayers, int nAlreadyHere ) { if ( 0 == connName[0] ) { @@ -506,17 +550,17 @@ CRefMgr::AddNew( const char* cookie, const char* connName, CookieID id, } /* PENDING: should this return a locked cref? */ logf( XW_LOGINFO, "%s( cookie=%s, connName=%s, cid=%d)", __func__, - cookie, connName, id ); + cookie, connName, cid ); CookieRef* ref = getFromFreeList(); RWWriteLock rwl( &m_cookieMapRWLock ); - logf( XW_LOGINFO, "making new cref: %d", id ); + logf( XW_LOGINFO, "making new cref: %d", cid ); if ( !!ref ) { - ref->ReInit( cookie, connName, id, langCode, nPlayers, nAlreadyHere ); + ref->ReInit( cookie, connName, cid, langCode, nPlayers, nAlreadyHere ); } else { - ref = new CookieRef( cookie, connName, id, langCode, nPlayers, + ref = new CookieRef( cookie, connName, cid, langCode, nPlayers, nAlreadyHere ); } @@ -551,6 +595,8 @@ CRefMgr::Recycle_locked( CookieRef* cref ) cref->Unlock(); + sleep(2); + /* don't grab this lock until after releasing cref's lock; otherwise deadlock happens. */ RWWriteLock rwl( &m_cookieMapRWLock ); @@ -592,25 +638,25 @@ CRefMgr::Recycle( const char* connName ) Recycle( id ); } /* Delete */ -CidInfo* -CRefMgr::getCookieRef_impl( CookieID cid ) -{ - CidInfo* info = m_cidlock->Claim( cid ); - /* CookieRef* ref = NULL; */ - /* RWReadLock rwl( &m_cookieMapRWLock ); */ +/* CidInfo* */ +/* CRefMgr::getCookieRef_impl( CookieID cid ) */ +/* { */ +/* CidInfo* info = m_cidlock->Claim( cid ); */ +/* /\* CookieRef* ref = NULL; *\/ */ +/* /\* RWReadLock rwl( &m_cookieMapRWLock ); *\/ */ - /* CookieMap::iterator iter = m_cookieMap.find( cid ); */ - /* while ( iter != m_cookieMap.end() ) { */ - /* CookieRef* second = iter->second; */ - /* if ( second->GetCookieID() == cid ) { */ - /* ref = second; */ - /* break; */ - /* } */ - /* ++iter; */ - /* } */ - /* info->SetRef( ref ); */ - return info; -} +/* /\* CookieMap::iterator iter = m_cookieMap.find( cid ); *\/ */ +/* /\* while ( iter != m_cookieMap.end() ) { *\/ */ +/* /\* CookieRef* second = iter->second; *\/ */ +/* /\* if ( second->GetCookieID() == cid ) { *\/ */ +/* /\* ref = second; *\/ */ +/* /\* break; *\/ */ +/* /\* } *\/ */ +/* /\* ++iter; *\/ */ +/* /\* } *\/ */ +/* /\* info->SetRef( ref ); *\/ */ +/* return info; */ +/* } */ #ifdef RELAY_HEARTBEAT void @@ -776,14 +822,16 @@ SafeCref::SafeCref( int socket ) SafeCref::~SafeCref() { if ( m_cinfo != NULL ) { + bool recycle = false; if ( m_locked ) { CookieRef* cref = m_cinfo->GetRef(); - if ( cref->ShouldDie() ) { + recycle = cref->ShouldDie(); + if ( recycle ) { m_mgr->Recycle_locked( cref ); } else { cref->Unlock(); } } - m_mgr->m_cidlock->Relinquish( m_cinfo ); + m_mgr->m_cidlock->Relinquish( m_cinfo, recycle ); } } diff --git a/xwords4/relay/crefmgr.h b/xwords4/relay/crefmgr.h index a2ebcd795..75b739578 100644 --- a/xwords4/relay/crefmgr.h +++ b/xwords4/relay/crefmgr.h @@ -31,20 +31,6 @@ typedef map CookieMap; class CookieMapIterator; -class SocketStuff; -typedef map< int, SocketStuff* > SocketMap; - -class SocketsIterator { - public: - SocketsIterator( SocketMap::iterator iter, SocketMap::iterator end, - pthread_mutex_t* mutex ); - ~SocketsIterator(); - int Next(); - private: - SocketMap::iterator m_iter; - SocketMap::iterator m_end; - pthread_mutex_t* m_mutex; /* locked */ -}; class CrefInfo { public: @@ -97,8 +83,8 @@ class CRefMgr { CookieID CookieIdForName( const char* name ); /* For use from ctrl only!!!! */ - void LockAll() { pthread_rwlock_wrlock( &m_cookieMapRWLock ); } - void UnlockAll() { pthread_rwlock_unlock( &m_cookieMapRWLock ); } + /* void LockAll() { pthread_rwlock_wrlock( &m_cookieMapRWLock ); } */ + /* void UnlockAll() { pthread_rwlock_unlock( &m_cookieMapRWLock ); } */ /* Track sockets independent of cookie refs */ bool Associate( int socket, CookieRef* cref ); @@ -109,7 +95,7 @@ class CRefMgr { pthread_mutex_t* GetWriteMutexForSocket( int socket ); void RemoveSocketRefs( int socket ); void PrintSocketInfo( int socket, string& out ); - SocketsIterator MakeSocketsIterator(); + /* SocketsIterator MakeSocketsIterator(); */ void IncrementFullCount( void ); int GetNumRoomsFilled( void ); @@ -176,9 +162,6 @@ class CRefMgr { pthread_rwlock_t m_cookieMapRWLock; CookieMap m_cookieMap; - pthread_mutex_t m_SocketStuffMutex; - SocketMap m_SocketStuff; - time_t m_startTime; string m_ports; @@ -235,9 +218,8 @@ class SafeCref { if ( IsValid() ) { CookieRef* cref = m_cinfo->GetRef(); assert( 0 != cref->GetCookieID() ); - cref->_Reconnect( socket, srcID, nPlayersH, nPlayersS, - seed, m_dead ); - return true; + return cref->_Reconnect( socket, srcID, nPlayersH, nPlayersS, + seed, m_dead ); } else { return false; } diff --git a/xwords4/relay/dbmgr.cpp b/xwords4/relay/dbmgr.cpp index 1415f2037..4cae3620c 100644 --- a/xwords4/relay/dbmgr.cpp +++ b/xwords4/relay/dbmgr.cpp @@ -341,16 +341,18 @@ DBMgr::HaveDevice( const char* connName, HostID hid, int seed ) return found; } -void +bool DBMgr::AddCID( const char* const connName, CookieID cid ) { const char* fmt = "UPDATE " GAMES_TABLE " SET cid = %d " - " WHERE connName = '%s'"; + " WHERE connName = '%s' AND cid IS NULL"; char query[256]; snprintf( query, sizeof(query), fmt, cid, connName ); logf( XW_LOGINFO, "%s: query: %s", __func__, query ); - execSql( query ); + bool result = execSql( query ); + logf( XW_LOGINFO, "%s(cid=%d)=>%d", __func__, cid, result ); + return result; } void diff --git a/xwords4/relay/dbmgr.h b/xwords4/relay/dbmgr.h index b7afa41f2..733d778f4 100644 --- a/xwords4/relay/dbmgr.h +++ b/xwords4/relay/dbmgr.h @@ -60,7 +60,7 @@ class DBMgr { bool RmDeviceByHid( const char* const connName, HostID id ); void RmDeviceBySeed( const char* const connName, unsigned short seed ); bool HaveDevice( const char* const connName, HostID id, int seed ); - void AddCID( const char* const connName, CookieID cid ); + bool AddCID( const char* const connName, CookieID cid ); void ClearCID( const char* connName ); void RecordSent( const char* const connName, HostID hid, int nBytes ); void GetPlayerCounts( const char* const connName, int* nTotal, diff --git a/xwords4/relay/states.cpp b/xwords4/relay/states.cpp index 75c0f5066..dbc435b23 100644 --- a/xwords4/relay/states.cpp +++ b/xwords4/relay/states.cpp @@ -95,9 +95,8 @@ static StateTable g_stateTable[] = { /* { XWS_CHK_ALLHERE, XWE_SOMEMISSING, XWA_NONE, XWS_WAITMORE }, */ -/* { XWS_ALLCONND, XWE_DISCONN, XWA_DISCONNECT, XWS_MISSING }, */ -/* { XWS_WAITMORE, XWE_DISCONN, XWA_DISCONNECT, XWS_WAITMORE }, */ -/* { XWS_MISSING, XWE_DISCONN, XWA_DISCONNECT, XWS_MISSING }, */ +{ XWS_ALLCONND, XWE_DISCONN, XWA_DISCONNECT, XWS_WAITMORE }, +{ XWS_WAITMORE, XWE_DISCONN, XWA_DISCONNECT, XWS_WAITMORE }, /* EMPTY means have messages to send but no connections. Time out and free memory after a while. BUT: don't I really want to keep these forever and diff --git a/xwords4/relay/xwrelay.cpp b/xwords4/relay/xwrelay.cpp index 58ad5c65d..ba8e3cdca 100644 --- a/xwords4/relay/xwrelay.cpp +++ b/xwords4/relay/xwrelay.cpp @@ -368,9 +368,7 @@ processReconnect( unsigned char* bufp, int bufLen, int socket ) unsigned char flags = *bufp++; XWREASON err = flagsOK( flags ); - if ( err != XWRELAY_ERROR_NONE ) { - denyConnection( socket, err ); - } else { + if ( err == XWRELAY_ERROR_NONE ) { char cookie[MAX_INVITE_LEN+1]; char connName[MAX_CONNNAME_LEN+1] = {0}; HostID srcID; @@ -396,12 +394,18 @@ processReconnect( unsigned char* bufp, int bufLen, int socket ) wantsPublic, makePublic ); success = scr.Reconnect( socket, srcID, nPlayersH, nPlayersT, gameSeed ); - } - - if ( !success ) { - denyConnection( socket, XWRELAY_ERROR_BADPROTO ); + if ( !success ) { + err = XWRELAY_ERROR_NORECONN; + } + } else { + err = XWRELAY_ERROR_BADPROTO; } } + + if ( err != XWRELAY_ERROR_NONE ) { + denyConnection( socket, err ); + } + return success; } /* processReconnect */ @@ -626,7 +630,7 @@ shutdown() delete tPool; - stop_ctrl_threads(); + //stop_ctrl_threads(); g_listeners.RemoveAll(); close( g_control ); @@ -1180,7 +1184,8 @@ main( int argc, char** argv ) } } if ( FD_ISSET( g_control, &rfds ) ) { - run_ctrl_thread( g_control ); + assert(0); // not working; don't use until fixed + // run_ctrl_thread( g_control ); --retval; } #ifdef DO_HTTP diff --git a/xwords4/relay/xwrelay.h b/xwords4/relay/xwrelay.h index 1b726b04d..6ae23a522 100644 --- a/xwords4/relay/xwrelay.h +++ b/xwords4/relay/xwrelay.h @@ -140,6 +140,7 @@ enum { ,XWRELAY_ERROR_DUP_ROOM ,XWRELAY_ERROR_TOO_MANY ,XWRELAY_ERROR_DELETED + ,XWRELAY_ERROR_NORECONN /* you can't reconnect; reset and try CONNECTING again */ ,XWRELAY_ERROR_LASTERR } #ifndef CANT_DO_TYPEDEF