Fix bug where device quitting one game to start another with same

cookie winds up connecting to the old game -- by flagging a cref as
full and no longer accepting connections without connNames even when
in the MISSING state.  Required a protocol changes so that devices get
their connNames as early as possible.
This commit is contained in:
ehouse 2009-09-14 01:55:03 +00:00
parent 7eeede45c3
commit 6603949b0e
5 changed files with 64 additions and 75 deletions

View file

@ -129,7 +129,6 @@ struct CommsCtxt {
XP_U16 nPlayersHere; XP_U16 nPlayersHere;
XP_U16 nPlayersTotal; XP_U16 nPlayersTotal;
XP_Bool connecting; XP_Bool connecting;
XP_Bool connNameValid; /* this can probably go. PENDING */
} r; } r;
XP_Bool isServer; XP_Bool isServer;
@ -234,7 +233,6 @@ init_relay( CommsCtxt* comms, XP_U16 nPlayersHere, XP_U16 nPlayersTotal )
comms->r.nPlayersTotal = nPlayersTotal; comms->r.nPlayersTotal = nPlayersTotal;
comms->r.cookieID = COOKIE_ID_NONE; comms->r.cookieID = COOKIE_ID_NONE;
comms->r.connName[0] = '\0'; comms->r.connName[0] = '\0';
comms->r.connNameValid = XP_FALSE;
} }
#endif #endif
@ -459,11 +457,8 @@ comms_makeFromStream( MPFORMAL XWStreamCtxt* stream, XW_UtilCtxt* util,
comms->nextChannelNo = stream_getU16( stream ); comms->nextChannelNo = stream_getU16( stream );
if ( addr.conType == COMMS_CONN_RELAY ) { if ( addr.conType == COMMS_CONN_RELAY ) {
comms->r.myHostID = stream_getU8( stream ); comms->r.myHostID = stream_getU8( stream );
comms->r.connNameValid = stream_getU8( stream ); stringFromStreamHere( stream, comms->r.connName,
if ( comms->r.connNameValid ) { sizeof(comms->r.connName) );
stringFromStreamHere( stream, comms->r.connName,
sizeof(comms->r.connName) );
}
} }
#ifdef DEBUG #ifdef DEBUG
@ -636,10 +631,7 @@ comms_writeToStream( const CommsCtxt* comms, XWStreamCtxt* stream )
stream_putU16( stream, comms->nextChannelNo ); stream_putU16( stream, comms->nextChannelNo );
if ( comms->addr.conType == COMMS_CONN_RELAY ) { if ( comms->addr.conType == COMMS_CONN_RELAY ) {
stream_putU8( stream, comms->r.myHostID ); stream_putU8( stream, comms->r.myHostID );
stream_putU8( stream, comms->r.connNameValid ); stringToStream( stream, comms->r.connName );
if ( comms->r.connNameValid ) {
stringToStream( stream, comms->r.connName );
}
} }
#ifdef DEBUG #ifdef DEBUG
@ -1064,7 +1056,6 @@ relayPreProcess( CommsCtxt* comms, XWStreamCtxt* stream, XWHostID* senderID )
XWHostID destID, srcID; XWHostID destID, srcID;
CookieID cookieID; CookieID cookieID;
XP_U8 relayErr; XP_U8 relayErr;
XP_Bool hasName;
/* nothing for us to do here if not using relay */ /* nothing for us to do here if not using relay */
XWRELAY_Cmd cmd = stream_getU8( stream ); XWRELAY_Cmd cmd = stream_getU8( stream );
@ -1078,6 +1069,11 @@ relayPreProcess( CommsCtxt* comms, XWStreamCtxt* stream, XWHostID* senderID )
comms->r.cookieID = stream_getU16( stream ); comms->r.cookieID = stream_getU16( stream );
XP_LOGF( "set cookieID = %d", comms->r.cookieID ); XP_LOGF( "set cookieID = %d", comms->r.cookieID );
setHeartbeatTimer( comms ); setHeartbeatTimer( comms );
if ( XWRELAY_CONNECT_RESP == cmd ) {
XP_ASSERT( comms->r.connName[0] == '\0' );
stringFromStreamHere( stream, comms->r.connName,
sizeof(comms->r.connName) );
}
break; break;
case XWRELAY_ALLHERE: case XWRELAY_ALLHERE:
@ -1087,14 +1083,6 @@ relayPreProcess( CommsCtxt* comms, XWStreamCtxt* stream, XWHostID* senderID )
|| comms->r.myHostID == srcID ); || comms->r.myHostID == srcID );
comms->r.myHostID = srcID; comms->r.myHostID = srcID;
XP_LOGF( "set hostid: %x", comms->r.myHostID ); XP_LOGF( "set hostid: %x", comms->r.myHostID );
hasName = stream_getU8( stream );
if ( hasName ) {
stringFromStreamHere( stream, comms->r.connName,
sizeof(comms->r.connName) );
XP_LOGF( "read connName: %s", comms->r.connName );
} else {
XP_ASSERT( comms->r.connName[0] != '\0' );
}
/* We're [re-]connected now. Send any pending messages. This may /* We're [re-]connected now. Send any pending messages. This may
need to be done later since we're inside the platform's socket need to be done later since we're inside the platform's socket
@ -1114,10 +1102,6 @@ relayPreProcess( CommsCtxt* comms, XWStreamCtxt* stream, XWHostID* senderID )
XP_LOGF( "%s: rejecting data message", __func__ ); XP_LOGF( "%s: rejecting data message", __func__ );
} else { } else {
*senderID = srcID; *senderID = srcID;
if ( !comms->r.connNameValid ) {
XP_LOGF( "%s: setting connNameValid", __func__ );
comms->r.connNameValid = XP_TRUE;
}
} }
break; break;
@ -1799,7 +1783,7 @@ relayConnect( CommsCtxt* comms )
LOG_FUNC(); LOG_FUNC();
if ( comms->addr.conType == COMMS_CONN_RELAY && !comms->r.connecting ) { if ( comms->addr.conType == COMMS_CONN_RELAY && !comms->r.connecting ) {
comms->r.connecting = XP_TRUE; comms->r.connecting = XP_TRUE;
success = send_via_relay( comms, comms->r.connNameValid? success = send_via_relay( comms, comms->r.connName[0]?
XWRELAY_GAME_RECONNECT : XWRELAY_GAME_CONNECT, XWRELAY_GAME_RECONNECT : XWRELAY_GAME_CONNECT,
comms->r.myHostID, NULL, 0 ); comms->r.myHostID, NULL, 0 );
comms->r.connecting = XP_FALSE; comms->r.connecting = XP_FALSE;

View file

@ -39,7 +39,6 @@
#include "timermgr.h" #include "timermgr.h"
#include "configs.h" #include "configs.h"
#include "crefmgr.h" #include "crefmgr.h"
#include "permid.h"
using namespace std; using namespace std;
@ -93,6 +92,7 @@ CookieRef::ReInit( const char* cookie, const char* connName, CookieID id )
m_nPlayersHere = 0; m_nPlayersHere = 0;
m_locking_thread = 0; m_locking_thread = 0;
m_starttime = uptime(); m_starttime = uptime();
m_gameFull = false;
RelayConfigs::GetConfigs()->GetValueFor( "HEARTBEAT", &m_heatbeat ); RelayConfigs::GetConfigs()->GetValueFor( "HEARTBEAT", &m_heatbeat );
logf( XW_LOGINFO, "initing cref for cookie %s, connName %s", logf( XW_LOGINFO, "initing cref for cookie %s, connName %s",
@ -171,6 +171,12 @@ void
CookieRef::_Connect( int socket, HostID hid, int nPlayersH, int nPlayersT ) CookieRef::_Connect( int socket, HostID hid, int nPlayersH, int nPlayersT )
{ {
if ( CRefMgr::Get()->Associate( socket, this ) ) { if ( CRefMgr::Get()->Associate( socket, this ) ) {
if ( hid == HOST_ID_NONE ) {
hid = nextHostID();
logf( XW_LOGINFO, "assigned host id: %x", hid );
} else {
logf( XW_LOGINFO, "NOT assigned host id; why?" );
}
pushConnectEvent( socket, hid, nPlayersH, nPlayersT ); pushConnectEvent( socket, hid, nPlayersH, nPlayersT );
handleEvents(); handleEvents();
} else { } else {
@ -241,13 +247,16 @@ CookieRef::NeverFullyConnected()
} }
bool bool
CookieRef::AcceptingReconnections( HostID hid, const char* cookie, CookieRef::GameOpen( HostID hid, const char* cookie,
int nPlayersH ) int nPlayersH, bool isNew )
{ {
bool accept = false; bool accept = false;
/* First, do we have room. Second, are we missing this guy? */ /* First, do we have room. Second, are we missing this guy? */
if ( m_curState != XWS_INITED if ( isNew && m_gameFull ) {
/* do nothing; reject */
logf( XW_LOGINFO, "reject: game for %s is full", cookie );
} else if ( m_curState != XWS_INITED
&& m_curState != XWS_CONNECTING && m_curState != XWS_CONNECTING
&& m_curState != XWS_MISSING ) { && m_curState != XWS_MISSING ) {
/* do nothing; reject */ /* do nothing; reject */
@ -274,7 +283,7 @@ CookieRef::AcceptingReconnections( HostID hid, const char* cookie,
} }
return accept; return accept;
} /* AcceptingReconnections */ } /* GameOpen */
void void
CookieRef::notifyDisconn( const CRefEvent* evt ) CookieRef::notifyDisconn( const CRefEvent* evt )
@ -551,9 +560,8 @@ CookieRef::handleEvents()
case XWA_SENDALLHERE: case XWA_SENDALLHERE:
case XWA_SNDALLHERE_2: case XWA_SNDALLHERE_2:
cancelAllConnectedTimer(); cancelAllConnectedTimer();
assignConnName();
assignHostIds(); assignHostIds();
sendAllHere( takeAction == XWA_SENDALLHERE ); sendAllHere( );
break; break;
case XWA_REJECT: case XWA_REJECT:
@ -605,7 +613,7 @@ CookieRef::increasePlayerCounts( const CRefEvent* evt )
int nPlayersT = evt->u.con.nPlayersT; int nPlayersT = evt->u.con.nPlayersT;
HostID hid = evt->u.con.srcID; HostID hid = evt->u.con.srcID;
logf( XW_LOGINFO, "increasePlayerCounts: hid=%d, nPlayersH=%d, " logf( XW_LOGINFO, "%s: hid=%d, nPlayersH=%d, ", __func__,
"nPlayersT=%d", hid, nPlayersH, nPlayersT ); "nPlayersT=%d", hid, nPlayersH, nPlayersT );
if ( hid == HOST_ID_SERVER ) { if ( hid == HOST_ID_SERVER ) {
@ -617,12 +625,16 @@ CookieRef::increasePlayerCounts( const CRefEvent* evt )
} }
m_nPlayersHere += nPlayersH; m_nPlayersHere += nPlayersH;
logf( XW_LOGVERBOSE1, "increasePlayerCounts: here=%d; total=%d", logf( XW_LOGVERBOSE1, "%s: here=%d; total=%d", __func__,
m_nPlayersHere, m_nPlayersSought ); m_nPlayersHere, m_nPlayersSought );
CRefEvent newevt; CRefEvent newevt;
newevt.type = (m_nPlayersHere == m_nPlayersSought) ? if ( m_nPlayersHere == m_nPlayersSought ) {
XWE_ALLHERE : XWE_SOMEMISSING; newevt.type = XWE_ALLHERE;
m_gameFull = true;
} else {
newevt.type = XWE_SOMEMISSING;
}
m_eventQueue.push_back( newevt ); m_eventQueue.push_back( newevt );
} /* increasePlayerCounts */ } /* increasePlayerCounts */
@ -715,7 +727,7 @@ CookieRef::sendResponse( const CRefEvent* evt, bool initial )
unsigned char buf[1 + /* cmd */ unsigned char buf[1 + /* cmd */
sizeof(short) + /* heartbeat */ sizeof(short) + /* heartbeat */
sizeof(CookieID) + sizeof(CookieID) +
1 /* hostID */ 1 + MAX_CONNNAME_LEN
]; ];
unsigned char* bufp = buf; unsigned char* bufp = buf;
@ -724,6 +736,16 @@ CookieRef::sendResponse( const CRefEvent* evt, bool initial )
putNetShort( &bufp, GetHeartbeat() ); putNetShort( &bufp, GetHeartbeat() );
putNetShort( &bufp, GetCookieID() ); putNetShort( &bufp, GetCookieID() );
if ( initial ) {
const char* connName = ConnName();
assert( !!connName && connName[0] );
int len = strlen( connName );
assert( len < MAX_CONNNAME_LEN );
*bufp++ = (char)len;
memcpy( bufp, connName, len );
bufp += len;
}
send_with_length( socket, buf, bufp - buf, true ); send_with_length( socket, buf, bufp - buf, true );
logf( XW_LOGVERBOSE0, "sent %s", cmdToStr( XWRELAY_Cmd(buf[0]) ) ); logf( XW_LOGVERBOSE0, "sent %s", cmdToStr( XWRELAY_Cmd(buf[0]) ) );
} /* sendResponse */ } /* sendResponse */
@ -793,25 +815,14 @@ CookieRef::notifyOthers( int socket, XWRelayMsg msg, XWREASON why )
} /* notifyOthers */ } /* notifyOthers */
void void
CookieRef::sendAllHere( bool includeName ) CookieRef::sendAllHere( void )
{ {
unsigned char buf[1 + 1 + MAX_CONNNAME_LEN]; unsigned char buf[1 + 1];
unsigned char* bufp = buf; unsigned char* bufp = buf;
unsigned char* idLoc; unsigned char* idLoc;
*bufp++ = XWRELAY_ALLHERE; *bufp++ = XWRELAY_ALLHERE;
idLoc = bufp++; /* space for hostId, remembering address */ idLoc = bufp++; /* space for hostId, remembering address */
*bufp++ = includeName? 1 : 0;
if ( includeName ) {
const char* connName = ConnName();
assert( !!connName && connName[0] );
int len = strlen( connName );
assert( len < MAX_CONNNAME_LEN );
*bufp++ = (char)len;
memcpy( bufp, connName, len );
bufp += len;
}
ASSERT_LOCKED(); ASSERT_LOCKED();
vector<HostRec>::iterator iter = m_sockets.begin(); vector<HostRec>::iterator iter = m_sockets.begin();
@ -819,24 +830,11 @@ CookieRef::sendAllHere( bool includeName )
logf( XW_LOGINFO, "%s: sending to hostid %d", __func__, logf( XW_LOGINFO, "%s: sending to hostid %d", __func__,
iter->m_hostID ); iter->m_hostID );
*idLoc = iter->m_hostID; /* write in this target's hostId */ *idLoc = iter->m_hostID; /* write in this target's hostId */
send_with_length( iter->m_socket, buf, bufp-buf, send_with_length( iter->m_socket, buf, bufp-buf, true );
true );
++iter; ++iter;
} }
} /* sendAllHere */ } /* sendAllHere */
void
CookieRef::assignConnName( void )
{
if ( !ConnName()[0] ) {
m_connName = PermID::GetNextUniqueID();
logf( XW_LOGINFO, "%s: assigning name: %s", __func__, ConnName() );
assert( GetCookieID() != 0 );
} else {
logf( XW_LOGINFO, "%s: has name: %s", __func__, ConnName() );
}
} /* assignConnName */
void void
CookieRef::assignHostIds( void ) CookieRef::assignHostIds( void )
{ {

View file

@ -88,8 +88,7 @@ class CookieRef {
int SocketForHost( HostID dest ); int SocketForHost( HostID dest );
bool NeverFullyConnected(); bool NeverFullyConnected();
bool AcceptingReconnections( HostID hid, const char* cookie, bool GameOpen( HostID hid, const char* cookie, int nPlayersH, bool isNew );
int nPlayersH );
/* for console */ /* for console */
void _PrintCookieInfo( string& out ); void _PrintCookieInfo( string& out );
@ -194,7 +193,7 @@ class CookieRef {
void noteHeartbeat(const CRefEvent* evt); void noteHeartbeat(const CRefEvent* evt);
void notifyDisconn(const CRefEvent* evt); void notifyDisconn(const CRefEvent* evt);
void removeSocket( int socket ); void removeSocket( int socket );
void sendAllHere( bool includeName ); void sendAllHere( void );
void assignConnName( void ); void assignConnName( void );
void assignHostIds( void ); void assignHostIds( void );
@ -208,7 +207,8 @@ class CookieRef {
static void s_checkAllConnected( void* closure ); static void s_checkAllConnected( void* closure );
vector<HostRec> m_sockets; vector<HostRec> m_sockets;
bool m_gameFull; /* once we've filled up, no more *new*
connections ever */
int m_heatbeat; /* might change per carrier or something. */ int m_heatbeat; /* might change per carrier or something. */
string m_cookie; /* cookie used for initial connections */ string m_cookie; /* cookie used for initial connections */
string m_connName; /* globally unique name */ string m_connName; /* globally unique name */

View file

@ -30,6 +30,7 @@
#include "mlock.h" #include "mlock.h"
#include "configs.h" #include "configs.h"
#include "timermgr.h" #include "timermgr.h"
#include "permid.h"
class SocketStuff { class SocketStuff {
public: public:
@ -132,8 +133,8 @@ CRefMgr::FindOpenGameFor( const char* cookie, const char* connName,
if ( 0 == strcmp( cref->ConnName(), connName ) ) { if ( 0 == strcmp( cref->ConnName(), connName ) ) {
if ( cref->Lock() ) { if ( cref->Lock() ) {
assert( !cookie || 0 == strcmp( cookie, cref->Cookie() ) ); assert( !cookie || 0 == strcmp( cookie, cref->Cookie() ) );
if ( cref->AcceptingReconnections( hid, cookie, if ( cref->GameOpen( hid, cookie,
nPlayersH ) ) { nPlayersH, false ) ) {
found = cref; found = cref;
} }
cref->Unlock(); cref->Unlock();
@ -145,8 +146,8 @@ CRefMgr::FindOpenGameFor( const char* cookie, const char* connName,
found = cref; found = cref;
} else { } else {
if ( cref->Lock() ) { if ( cref->Lock() ) {
if ( cref->AcceptingReconnections( hid, cookie, if ( cref->GameOpen( hid, cookie,
nPlayersH ) ) { nPlayersH, true ) ) {
found = cref; found = cref;
} else if ( cref->HasSocket_locked(socket) ) { } else if ( cref->HasSocket_locked(socket) ) {
logf( XW_LOGINFO, "%s: HasSocket case", __func__); logf( XW_LOGINFO, "%s: HasSocket case", __func__);
@ -276,6 +277,11 @@ CRefMgr::getMakeCookieRef_locked( const char* cookie, const char* connName,
cref = FindOpenGameFor( cookie, connName, hid, socket, nPlayersH, nPlayersT ); cref = FindOpenGameFor( cookie, connName, hid, socket, nPlayersH, nPlayersT );
if ( cref == NULL ) { if ( cref == NULL ) {
string s;
if ( NULL == connName ) {
s = PermID::GetNextUniqueID();
connName = s.c_str();
}
cref = AddNew( cookie, connName, nextCID( NULL ) ); cref = AddNew( cookie, connName, nextCID( NULL ) );
} }
@ -437,6 +443,7 @@ CRefMgr::AddNew( const char* cookie, const char* connName, CookieID id )
} }
#endif #endif
logf( XW_LOGINFO, "%s=>%p", __func__, ref );
return ref; return ref;
} /* AddNew */ } /* AddNew */

View file

@ -51,7 +51,8 @@ enum { XWRELAY_NONE /* 0 is an illegal value */
, XWRELAY_CONNECT_RESP , XWRELAY_CONNECT_RESP
/* Sent from relay to device in response to XWRELAY_CONNECT. Format: /* Sent from relay to device in response to XWRELAY_CONNECT. Format:
heartbeat_seconds: 2; connectionID: 2; */ heartbeat_seconds: 2; connectionID: 2; connNameLen: 1;
connName<connNameLen>*/
, XWRELAY_RECONNECT_RESP , XWRELAY_RECONNECT_RESP
/* Sent from relay to device in response to XWRELAY_RECONNECT. Format: /* Sent from relay to device in response to XWRELAY_RECONNECT. Format:
@ -62,8 +63,7 @@ enum { XWRELAY_NONE /* 0 is an illegal value */
are here (at start of new game or after having been gone for a are here (at start of new game or after having been gone for a
while). Devices should not attempt to forward messages before this while). Devices should not attempt to forward messages before this
message is received or after XWRELAY_DISCONNECT_OTHER is received. message is received or after XWRELAY_DISCONNECT_OTHER is received.
Format: hostID: 1; hasName: 1; [connNameLen: 1; connName: Format: hostID: 1; */
<connNameLen>]; */
, XWRELAY_DISCONNECT_YOU , XWRELAY_DISCONNECT_YOU
/* Sent from relay when existing connection is terminated. /* Sent from relay when existing connection is terminated.