break connect and reconnect into separate paths to simplify. Be

consistent: nJoined in DB tracks players "ever seen and issued
connName" rather than "currently connected".  Next step: debug game
between two devices never connected at same time.

xwords4/relay/cref.cpp

@@ -594,6 +594,7 @@ CookieRef::handleEvents()
             case XWA_SEND_RERSP:
                 sendResponse( &evt, false );
                 sendAnyStored( &evt );
+                increasePlayerCounts( &evt, true );
                 // postCheckAllHere();
                 break;
 
@@ -776,9 +777,6 @@ CookieRef::increasePlayerCounts( const CRefEvent* evt, bool reconn )
     int nPlayersH = evt->u.con.nPlayersH;
     int socket = evt->u.con.socket;
     int seed = evt->u.con.seed;
-    bool addHost = false;
-    bool addAck = false;
-    /* XW_RELAY_EVENT newEvt = XWE_NONE; */
 
     assert( m_nPlayersSought > 0 );
     assert( m_nPlayersSought == evt->u.con.nPlayersS );
@@ -797,77 +795,28 @@ CookieRef::increasePlayerCounts( const CRefEvent* evt, bool reconn )
        recommend a new game as things must be pretty f*cked up.  Or somebody's
        mucking with me. */
 
-    if ( reconn ) {
-        if ( 0 != m_nPlayersSought && 
-             m_nPlayersHere + nPlayersH > m_nPlayersSought ) {
-            logf( XW_LOGERROR, "too many new players provided: %d > %d",
-                  m_nPlayersHere + nPlayersH, m_nPlayersSought );
-            goto drop;
-        }
-
-        m_nPlayersHere += nPlayersH;
-        /* if ( m_nPlayersHere == m_nPlayersSought ) { */
-        /*     newEvt = XWE_ALLHERE; */
-        /* } else { */
-        /*     newEvt = XWE_SOMEMISSING; */
-        /* } */
-        addHost = true;
-    } else {      /* a host; init values */
+    if ( !reconn ) {
         m_nPlayersHere += nPlayersH;
         assert( m_nPlayersHere <= m_nPlayersSought );
-        addAck = true;
         DBMgr::Get()->AddPlayers( ConnName(), nPlayersH );
-
-        /* if ( m_nPlayersHere == m_nPlayersSought ) { /\* complete! *\/ */
-        /*     newEvt = XWE_ALLHERE; */
-        /* } else { */
-        /*     newEvt = XWE_SOMEMISSING; */
-        /* } */
-    /* } else {                    /\* a guest *\/ */
-    /*     assert( reconn || m_nPlayersSought != 0 ); /\* host better be here *\/ */
-
-    /*     if ( m_nPlayersSought < m_nPlayersHere + nPlayersH ) { /\* too many; */
-    /*                                                               reject *\/ */
-    /*         newEvt = XWE_TOO_MANY; */
-    /*     } else { */
-    /*         m_nPlayersHere += nPlayersH; */
-    /*         if ( m_nPlayersHere == m_nPlayersSought ) { /\* complete! *\/ */
-    /*             newEvt = XWE_ALLHERE; */
-    /*             m_gameFull = true; */
-    /*         } else { */
-    /*             newEvt = XWE_SOMEMISSING; */
-    /*         } */
-    /*         addHost = true; */
-    /*     } */
     }
 
-    /* if ( newEvt != XWE_NONE ) { */
-    /*     CRefEvent evt( newEvt ); */
-    /*     m_eventQueue.push_back( evt ); */
-    /* } else { */
-    /*     logf( XW_LOGERROR, "%s: not pushing an event", __func__ ); */
-    /* } */
+    HostID hostid = evt->u.con.srcID;
+    /* first add the rec here, whether it'll get ack'd or not */
+    logf( XW_LOGINFO, "%s: remembering pair: hostid=%x, "
+          "socket=%d (size=%d)", 
+          __func__, hostid, socket, m_sockets.size());
 
-    if ( addHost || addAck ) {
-        HostID hostid = evt->u.con.srcID;
-        /* first add the rec here, whether it'll stay for not */
-        logf( XW_LOGINFO, "%s: remembering pair: hostid=%x, "
-              "socket=%d (size=%d)", 
-              __func__, hostid, socket, m_sockets.size());
+    HostRec hr( hostid, socket, nPlayersH, seed, !reconn );
+    m_sockets.push_back( hr );
 
-        HostRec hr( hostid, socket, nPlayersH, seed, addAck );
-        m_sockets.push_back( hr );
+    assert( !AlreadyHere( evt->u.con.seed, -1 ) );
+    m_seeds.push_back( evt->u.con.seed );
 
-        assert( !AlreadyHere( evt->u.con.seed, -1 ) );
-        m_seeds.push_back( evt->u.con.seed );
+    logf( XW_LOGVERBOSE1, "%s: here=%d; total=%d", __func__,
+          m_nPlayersHere, m_nPlayersSought );
 
-        logf( XW_LOGVERBOSE1, "%s: here=%d; total=%d", __func__,
-              m_nPlayersHere, m_nPlayersSought );
-    } else {
-        assert( 0 );
-    }
- drop:
-    return addHost || addAck;
+    return true;
 } /* increasePlayerCounts */
 
 void
@@ -1143,20 +1092,20 @@ void
 CookieRef::checkSomeMissing( void )
 {
     logf( XW_LOGINFO, "%s", __func__ );
-    int count = 0;
+    /* int count = 0; */
 
-    vector<HostRec>::iterator iter;
-    for ( iter = m_sockets.begin(); iter != m_sockets.end(); ++iter ) {
-        count += iter->m_nPlayersH;
-    }
-    /* Don't really need the iterator above.... */
-    assert( count == m_nPlayersHere );
+    /* vector<HostRec>::iterator iter; */
+    /* for ( iter = m_sockets.begin(); iter != m_sockets.end(); ++iter ) { */
+    /*     count += iter->m_nPlayersH; */
+    /* } */
+    /* /\* Don't really need the iterator above.... *\/ */
+    /* assert( count == m_nPlayersHere ); */
 
     logf( XW_LOGINFO, "%s; count=%d; nPlayersS=%d", __func__,
-          count, m_nPlayersSought );
+          m_nPlayersHere, m_nPlayersSought );
 
-    assert( count <= m_nPlayersSought );
-    if ( count < m_nPlayersSought ) {
+    assert( m_nPlayersHere <= m_nPlayersSought );
+    if ( m_nPlayersHere < m_nPlayersSought ) {
         CRefEvent evt( XWE_SOMEMISSING );
         m_eventQueue.push_back( evt );
     }

xwords4/relay/crefmgr.cpp

@@ -107,111 +107,6 @@ CRefMgr::CloseAll()
     }
 } /* CloseAll */
 
-/* Find a game to which this guy belongs.  Return NULL if none's found and
- * presumably a new one will be created.
- *
- * Match by connName if provided.  If I match, great.  But what if there isn't
- * room, which would normally mean I'm already there and this is a duplicate
- * packet.  Should a dup be simply dropped, or should I reply?  If the dup
- * happened because our earlier reply was dropped then we should in fact
- * reply.
- *
- * If match is by cookie (called Room in the UI) we need to be careful.  If
- * this is an established game, meaning that at some point all devices were
- * there and a connName was established, then check if the incoming seed is in
- * that connName, otherwise reject it (to form its own game.)  But otherwise,
- * if either we can't match seeds yet or this guy's does match, let him in.
- * Again there's the duplicate packet issue if the game is "full" already.
- *
- * Match can also occur on cookie only -- device has no connName perhaps
- * because the relay crashed before sending it -- where the device belongs in
- * an existing game.  That we detect by checking if the new arrival's seed is
- * a component of the candidate's connName.
- */
-
-CookieRef*
-CRefMgr::FindOpenGameFor( const char* cookie, const char* connName,
-                          HostID hid, int socket, int nPlayersH, int nPlayersT,
-                          int gameSeed, int langCode, bool wantsPublic,
-                          bool* alreadyHere )
-{
-#if 1
-    CookieRef* found = NULL;
-    CookieID cid = m_db->FindOpen( cookie, langCode, nPlayersT, nPlayersH, wantsPublic );
-    if ( cid > 0 ) {
-        found = getCookieRef_impl( cid );
-    }
-    return found;
-#else
-    logf( XW_LOGINFO, "%s(cookie=%s,connName=%s,hid=%d,seed=%x,socket=%d,"
-          "here=%d,total=%d)", __func__, cookie, connName, hid, gameSeed, 
-          socket, nPlayersH, nPlayersT );
-    CookieRef* found = NULL;
-
-    assert( !!cookie || !!connName );
-    RWReadLock rwl( &m_cookieMapRWLock );
-
-    CookieMap::iterator iter;
-    for ( iter = m_cookieMap.begin();
-          NULL == found && iter != m_cookieMap.end();
-          ++iter ) {
-        CookieRef* cref = iter->second;
-
-        /* Reject immediately if language code or proposed game size don't
-           match. */
-        if ( (cref->GetLangCode() != langCode)
-             || (cref->GetPlayersSought() != nPlayersT) ) {
-            continue;
-        }
-
-        if ( !!connName && 0 == strcmp( cref->ConnName(), connName ) ) {
-            found = cref;
-            /* if ( cref->Lock() ) { */
-                    
-            /*     assert( !cookie ||  */
-            /*             0 == strcasecmp( cookie, cref->Cookie() ) ); */
-            /*     if ( cref->SeedBelongs( gameSeed ) ) { */
-            /*         logf( XW_LOGINFO, "%s: SeedBelongs: dup packet?", */
-            /*               __func__ ); */
-            /*         *alreadyHere = true; */
-            /*         found = cref; */
-            /*     } else if ( cref->GameOpen( cookie, false,  */
-            /*                                 alreadyHere ) ) { */
-            /*         found = cref; */
-            /*     } else { */
-            /*         /\* drop if we match on connName and it's not */
-            /*            wanted; must be dup. *\/ */
-            /*         *alreadyHere = true; */
-            /*     } */
-            /*     cref->Unlock(); */
-            /* } */
-        } 
-
-        if ( !found && !!cookie ) {
-            if ( 0 == strcasecmp( cref->Cookie(), cookie ) ) {
-                if ( cref->Lock() ) {
-                    assert( cref->ConnName()[0] );
-                    if ( cref->AlreadyHere( gameSeed, -1 ) ) {
-                        found = cref;
-                        *alreadyHere = true;
-                    } else if ( cref->GameOpen( cookie ) ) {
-                        found = cref;
-                    } else if ( cref->HasSocket_locked(socket) ) {
-                        assert( 0 ); /* should have dumped the socket */
-                        logf( XW_LOGINFO, "%s: HasSocket case", __func__);
-                        found = cref;
-                    }
-                    cref->Unlock();
-                }
-            }
-        }
-    } /* for */
-
-    logf( XW_LOGINFO, "%s=>%p", __func__, found );
-    return found;
-#endif
-} /* FindOpenGameFor */
-
 CookieID
 CRefMgr::nextCID( const char* connName )
 {
@@ -334,10 +229,10 @@ CRefMgr::getFromFreeList( void )
 
 
 CookieRef*
-CRefMgr::getMakeCookieRef_locked( const char* cookie, const char* connName,
-                                  HostID hid, int socket, int nPlayersH, 
-                                  int nPlayersT, int langCode, int gameSeed,
-                                  bool wantsPublic, bool makePublic )
+CRefMgr::getMakeCookieRef_locked( const char* cookie, HostID hid, int socket, 
+                                  int nPlayersH, int nPlayersT, int langCode, 
+                                  int gameSeed, bool wantsPublic, 
+                                  bool makePublic )
 {
     CookieRef* cref;
 
@@ -349,18 +244,49 @@ CRefMgr::getMakeCookieRef_locked( const char* cookie, const char* connName,
        later when the game is complete.
     */
 
-    bool alreadyHere;
-    cref = FindOpenGameFor( cookie, connName, hid, socket, nPlayersH, nPlayersT,
-                            gameSeed, langCode, wantsPublic, &alreadyHere );
-    if ( cref == NULL && !alreadyHere ) {
-        CookieID cid = nextCID( NULL );
-        cref = AddNew( cookie, connName, cid, langCode, nPlayersT );
-        m_db->AddNew( cookie, cref->ConnName(), cid, langCode, nPlayersT, makePublic );
+    char connNameBuf[MAX_CONNNAME_LEN+1] = {0};
+    CookieID cid = m_db->FindOpen( cookie, langCode, nPlayersT, nPlayersH,
+                                   wantsPublic, 
+                                   connNameBuf, sizeof(connNameBuf) );
+    if ( cid > 0 ) {
+        cref = getCookieRef_impl( cid ); 
+    } else {
+        cid = nextCID( NULL );
+        cref = AddNew( cookie, connNameBuf, cid, langCode, nPlayersT );
+        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 );
+        }
     }
 
     return cref;
 } /* getMakeCookieRef_locked */
 
+CookieRef* 
+CRefMgr::getMakeCookieRef_locked( const char* connName, HostID hid, int socket, 
+                                  int nPlayersH, int seed )
+{
+    CookieRef* cref = NULL;
+
+    /* fetch these from DB */
+    char cookie[MAX_INVITE_LEN+1];
+    int langCode;
+    int nPlayersT;
+
+    CookieID cid = m_db->FindGame( connName, cookie, sizeof(cookie),
+                                   &langCode, &nPlayersT );
+    if ( 0 != cid ) {           /* already open */
+        cref = getCookieRef_impl( cid );
+    } else {
+        CookieID cid = nextCID( NULL );
+        cref = AddNew( cookie, connName, cid, langCode, nPlayersT );
+        m_db->AddCID( connName, cid );
+    }
+    return cref;
+}
+
 bool
 CRefMgr::Associate( int socket, CookieRef* cref )
 {
@@ -510,6 +436,9 @@ CookieRef*
 CRefMgr::AddNew( const char* cookie, const char* connName, CookieID id,
                  int langCode, int nPlayers )
 {
+    if ( 0 == connName[0] ) {
+        connName = NULL;
+    }
     /* PENDING: should this return a locked cref? */
     logf( XW_LOGINFO, "%s( cookie=%s, connName=%s, cid=%d)", __func__,
           cookie, connName, id );
@@ -684,7 +613,7 @@ SafeCref::SafeCref( const char* cookie, int socket, int nPlayersH, int nPlayersS
 {
     CookieRef* cref;
 
-    cref = m_mgr->getMakeCookieRef_locked( cookie, NULL, 0, socket,
+    cref = m_mgr->getMakeCookieRef_locked( cookie, 0, socket,
                                            nPlayersH, nPlayersS, langCode,
                                            gameSeed, wantsPublic, makePublic );
     if ( cref != NULL ) {
@@ -704,8 +633,8 @@ SafeCref::SafeCref( const char* connName, HostID hid,
 {
     CookieRef* cref;
 
-    cref = m_mgr->getMakeCookieRef_locked( NULL, connName, hid, socket, nPlayersH, 
-                                           nPlayersS, langCode, gameSeed, false, false );
+    cref = m_mgr->getMakeCookieRef_locked( connName, hid, socket, nPlayersH, 
+                                           gameSeed );
     if ( cref != NULL ) {
         m_locked = cref->Lock();
         m_cref = cref;

xwords4/relay/crefmgr.h

@@ -135,11 +135,17 @@ class CRefMgr {
     void addToFreeList( CookieRef* cref );
     CookieRef* getFromFreeList( void );
 
+    /* connect case */
     CookieRef* getMakeCookieRef_locked( const char* cookie, 
-                                        const char* connName,
                                         HostID hid, int socket, int nPlayersH,
                                         int nPlayersS, int langCode, int seed,
                                         bool wantsPublic, bool makePublic );
+
+    /* reconnect case; just the stuff we don't have in db */
+    CookieRef* getMakeCookieRef_locked( const char* connName,
+                                        HostID hid, int socket, int nPlayersH,
+                                        int seed );
+
     CookieRef* getCookieRef( CookieID cookieID );
     CookieRef* getCookieRef( int socket );
     bool checkCookieRef_locked( CookieRef* cref );

xwords4/relay/dbmgr.cpp

@@ -82,7 +82,7 @@ DBMgr::AddNew( const char* cookie, const char* connName, CookieID cid,
     if ( !connName ) connName = "";
 
     const char* fmt = "INSERT INTO " TABLE_NAME
-        "(cid, cookie, connName, nTotal, nHere, lang, ispublic, ctime) "
+        "(cid, cookie, connName, nTotal, nJoined, lang, ispublic, ctime) "
         "VALUES( %d, '%s', '%s', %d, %d, %d, %s, 'now' )";
     char buf[256];
     snprintf( buf, sizeof(buf), fmt, cid/*m_nextCID++*/, cookie, connName, 
@@ -113,14 +113,41 @@ DBMgr::AddNew( const char* cookie, const char* connName, CookieID cid,
 }
 
 CookieID
-DBMgr::FindOpen( const char* cookie, int lang, int nPlayersT, int nPlayersH, bool wantsPublic )
+DBMgr::FindGame( const char* connName, char* cookieBuf, int bufLen,
+                 int* langP, int* nPlayersTP )
 {
     CookieID cid = 0;
 
-    const char* fmt = "SELECT cid from " TABLE_NAME " where cookie = '%s' "
+    const char* fmt = "SELECT cid, cookie, lang, nTotal from " TABLE_NAME " where connName = '%s' "
+        "LIMIT 1";
+    char query[256];
+    snprintf( query, sizeof(query), fmt, connName );
+    logf( XW_LOGINFO, "query: %s", query );
+
+    PGresult* result = PQexec( m_pgconn, query );
+    if ( 1 == PQntuples( result ) ) {
+        cid = atoi( PQgetvalue( result, 0, 0 ) );
+        snprintf( cookieBuf, bufLen, "%s", PQgetvalue( result, 0, 1 ) );
+        *langP = atoi( PQgetvalue( result, 0, 2 ) );
+        *nPlayersTP = atoi( PQgetvalue( result, 0, 3 ) );
+    }
+    PQclear( result );
+
+    logf( XW_LOGINFO, "%s(%s)=>%d", __func__, connName, cid );
+    return cid;
+}
+
+CookieID
+DBMgr::FindOpen( const char* cookie, int lang, int nPlayersT, int nPlayersH,
+                 bool wantsPublic, char* connNameBuf, int bufLen )
+{
+    CookieID cid = 0;
+
+    const char* fmt = "SELECT cid, connName FROM " TABLE_NAME " "
+        "WHERE cookie = '%s' "
         "AND lang = %d "
         "AND nTotal = %d "
-        "AND %d <= nTotal-nHere "
+        "AND %d <= nTotal-nJoined "
         "AND %s = ispublic "
         "LIMIT 1";
     char query[256];
@@ -131,17 +158,18 @@ DBMgr::FindOpen( const char* cookie, int lang, int nPlayersT, int nPlayersH, boo
     PGresult* result = PQexec( m_pgconn, query );
     if ( 1 == PQntuples( result ) ) {
         cid = atoi( PQgetvalue( result, 0, 0 ) );
-        assert( cid > 0 );
+        snprintf( connNameBuf, bufLen, "%s", PQgetvalue( result, 0, 1 ) );
+        /* cid may be 0, but should use game anyway  */
     }
     PQclear( result );
     logf( XW_LOGINFO, "%s=>%d", __func__, cid );
     return cid;
-}
+} /* FindOpen */
 
 void
 DBMgr::AddPlayers( const char* connName, int nToAdd )
 {
-    const char* fmt = "UPDATE " TABLE_NAME " SET nHere = nHere+%d "
+    const char* fmt = "UPDATE " TABLE_NAME " SET nJoined = nJoined+%d "
         "WHERE connName = '%s'";
     char query[256];
     snprintf( query, sizeof(query), fmt, nToAdd, connName );
@@ -150,16 +178,32 @@ DBMgr::AddPlayers( const char* connName, int nToAdd )
     execSql( query );
 }
 
+void
+DBMgr::AddCID( const char* const connName, CookieID cid )
+{
+    const char* fmt = "UPDATE " TABLE_NAME " SET cid = %d "
+        "WHERE connName = '%s'";
+    char query[256];
+    snprintf( query, sizeof(query), fmt, cid, connName );
+    logf( XW_LOGINFO, "%s: query: %s", __func__, query );
+
+    execSql( query );
+}
+
 void
 DBMgr::ClearCIDs( void )
 {
-    execSql( "UPDATE " TABLE_NAME " set cid = 0" );
+    execSql( "UPDATE " TABLE_NAME " set cid = null" );
 }
 
 void
 DBMgr::execSql( const char* query )
 {
     PGresult* result = PQexec( m_pgconn, query );
+    if ( PGRES_COMMAND_OK != PQresultStatus(result) ) {
+        logf( XW_LOGERROR, "PQEXEC=>%s", PQresultErrorMessage(result) );
+        assert( 0 );
+    }
     PQclear( result );
     logf( XW_LOGINFO, "PQexecParams=>%d", result );
 }
@@ -171,7 +215,7 @@ DBMgr::execSql( const char* query )
   cookie VARCHAR(32),
   connName VARCHAR(64) UNIQUE PRIMARY KEY,
   nTotal INTEGER,
-  nHere INTEGER, 
+  nJoined INTEGER, 
   lang INTEGER,
   ctime TIMESTAMP,
   mtime TIMESTAMP

xwords4/relay/dbmgr.h

@@ -35,10 +35,14 @@ class DBMgr {
     void AddNew( const char* cookie, const char* connName, CookieID cid, 
                  int langCode, int nPlayersT, bool isPublic );
 
+    CookieID FindGame( const char* connName, char* cookieBuf, int bufLen,
+                       int* langP, int* nPlayersTP );
     CookieID FindOpen( const char* cookie, int lang, int nPlayersT, 
-                       int nPlayersH, bool wantsPublic );
+                       int nPlayersH, bool wantsPublic, 
+                       char* connNameBuf, int bufLen );
 
-    void AddPlayers( const char* connName, int nToAdd );
+    void AddPlayers( const char* const connName, int nToAdd );
+    void AddCID( const char* connName, CookieID cid );
 
  private:
     DBMgr();