From a0a8a816a476b4891b005462598f9ab39adb70de Mon Sep 17 00:00:00 2001 From: Eric House Date: Mon, 21 Nov 2011 10:09:29 -0800 Subject: [PATCH 1/5] make number of devices configurable --- xwords4/linux/scripts/test_backsend.sh | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/xwords4/linux/scripts/test_backsend.sh b/xwords4/linux/scripts/test_backsend.sh index 446dda640..8571a6fbf 100755 --- a/xwords4/linux/scripts/test_backsend.sh +++ b/xwords4/linux/scripts/test_backsend.sh @@ -5,6 +5,7 @@ set -e -u SEED="" RELAY_LOG="../relay/xwrelay.log" +NDEVS=${NDEVS:-2} usage() { echo "usage: $(basename $0) [--seed RANDOM_SEED]" @@ -50,7 +51,7 @@ else fi # Run once to connect each with the relay -for NUM in $(seq 1 2); do +for NUM in $(seq 0 $((NDEVS-1))); do LOG="$(logname $NUM)" ARGS=$COMMON_ARGS if [ -n "$SEED" ]; then @@ -68,7 +69,7 @@ NBS=$DIR/nbs ZERO_COUNT=0 while [ $ZERO_COUNT -lt 2 ]; do WORK_DONE="" - for NUM in $(seq 1 2); do + for NUM in $(seq 0 $((NDEVS-1))); do LOG="$(logname $NUM)" RELAYID=$(./scripts/relayID.sh --short $LOG) MSG_COUNT=$(../relay/rq -m $RELAYID 2>/dev/null | sed 's,^.*-- ,,') @@ -98,7 +99,7 @@ done # the game's over. Strictly speaking we need to get beyond that, but # reaching it is the first step. Debug failure to get that far first. ENDED="" -for NUM in $(seq 1 2); do +for NUM in $(seq 0 $((NDEVS-1))); do LOG="$(logname $NUM)" if grep -q 'waiting for server to end game' $LOG; then ENDED=1 @@ -107,7 +108,7 @@ for NUM in $(seq 1 2); do done if [ -z "$ENDED" ]; then - for NUM in $(seq 1 2); do + for NUM in $(seq 0 $((NDEVS-1))); do LOG="$(logname $NUM)" if ! grep -q 'all remaining tiles' $LOG; then echo "$LOG didn't seem to end correctly" From 72a3ed18911e0f505762495f086d2a9c3c0aad52 Mon Sep 17 00:00:00 2001 From: Eric House Date: Mon, 21 Nov 2011 17:33:44 -0800 Subject: [PATCH 2/5] increase buffer size: longer relay name caused stack overwrite failures. --- xwords4/relay/rq.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/xwords4/relay/rq.c b/xwords4/relay/rq.c index 3d455b5b7..b626f6dd0 100644 --- a/xwords4/relay/rq.c +++ b/xwords4/relay/rq.c @@ -390,7 +390,8 @@ do_deletes( int sockfd, const char** connNames, int nConnNames ) char buf[4096]; int nused = 0; for ( ii = 0; ii < nConnNames; ++ii ) { - char tmp[32]; + char tmp[128]; + assert( strlen(connNames[ii]) < sizeof(tmp)-1 ); strcpy( tmp, connNames[ii] ); char* seedp = strrchr( tmp, '/' ); assert( !!seedp ); From b00327aa8b90e2e3e782c94fda0c6fb95f57b87a Mon Sep 17 00:00:00 2001 From: Eric House Date: Mon, 21 Nov 2011 17:35:08 -0800 Subject: [PATCH 3/5] fix to work for NDEVS > 2. --- xwords4/linux/scripts/test_backsend.sh | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/xwords4/linux/scripts/test_backsend.sh b/xwords4/linux/scripts/test_backsend.sh index 8571a6fbf..36c128f06 100755 --- a/xwords4/linux/scripts/test_backsend.sh +++ b/xwords4/linux/scripts/test_backsend.sh @@ -39,7 +39,10 @@ DIR=$(basename $0)_$PID DICT=dict.xwd APP=./obj_linux_memdbg/xwords -COMMON_ARGS="--room $ROOM --curses --robot Eric --remote-player --game-dict $DICT --quit-after 2" +COMMON_ARGS="--room $ROOM --curses --robot Eric --game-dict $DICT --quit-after 2" +for NUM in $(seq 2 $NDEVS); do + COMMON_ARGS="$COMMON_ARGS --remote-player" +done mkdir -p $DIR @@ -51,7 +54,7 @@ else fi # Run once to connect each with the relay -for NUM in $(seq 0 $((NDEVS-1))); do +for NUM in $(seq 1 $((NDEVS))); do LOG="$(logname $NUM)" ARGS=$COMMON_ARGS if [ -n "$SEED" ]; then @@ -59,7 +62,7 @@ for NUM in $(seq 0 $((NDEVS-1))); do fi $APP $ARGS --file $DIR/game_${NUM}.xwg > /dev/null 2>>$LOG & PID1=$! - sleep 4 + sleep 2 kill $PID1 wait $PID1 done @@ -69,7 +72,7 @@ NBS=$DIR/nbs ZERO_COUNT=0 while [ $ZERO_COUNT -lt 2 ]; do WORK_DONE="" - for NUM in $(seq 0 $((NDEVS-1))); do + for NUM in $(seq 1 $((NDEVS))); do LOG="$(logname $NUM)" RELAYID=$(./scripts/relayID.sh --short $LOG) MSG_COUNT=$(../relay/rq -m $RELAYID 2>/dev/null | sed 's,^.*-- ,,') @@ -80,7 +83,8 @@ while [ $ZERO_COUNT -lt 2 ]; do if [ -n "$SEED" ]; then ARGS="$ARGS --seed $((SEED+NUM))" fi - $APP $ARGS --file $DIR/game_${NUM}.xwg --with-nbs $NBS > /dev/null 2>>$LOG & + $APP $ARGS --file $DIR/game_${NUM}.xwg --with-nbs $NBS \ + > /dev/null 2>>$LOG & PID1=$! ../relay/rq -f $RELAYID -b $NBS @@ -99,7 +103,7 @@ done # the game's over. Strictly speaking we need to get beyond that, but # reaching it is the first step. Debug failure to get that far first. ENDED="" -for NUM in $(seq 0 $((NDEVS-1))); do +for NUM in $(seq 1 $((NDEVS))); do LOG="$(logname $NUM)" if grep -q 'waiting for server to end game' $LOG; then ENDED=1 @@ -108,7 +112,7 @@ for NUM in $(seq 0 $((NDEVS-1))); do done if [ -z "$ENDED" ]; then - for NUM in $(seq 0 $((NDEVS-1))); do + for NUM in $(seq 1 $((NDEVS))); do LOG="$(logname $NUM)" if ! grep -q 'all remaining tiles' $LOG; then echo "$LOG didn't seem to end correctly" From eec83fe5b682b482f434a7be00f2ff3e97ae5f80 Mon Sep 17 00:00:00 2001 From: Eric House Date: Mon, 21 Nov 2011 17:38:50 -0800 Subject: [PATCH 4/5] don't fail when RELAY_ALLHERE comes without a connection, as that will happen when messages come in background. I'm not sure what the test I'm removing was there to catch so need to test thoroughly. --- xwords4/common/comms.c | 58 +++++++++++++++++++++--------------------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/xwords4/common/comms.c b/xwords4/common/comms.c index 4d64f03e4..af93a6d62 100644 --- a/xwords4/common/comms.c +++ b/xwords4/common/comms.c @@ -1267,43 +1267,43 @@ relayPreProcess( CommsCtxt* comms, XWStreamCtxt* stream, XWHostID* senderID ) || comms->r.myHostID == srcID ); if ( 0 == comms->r.cookieID ) { - XP_LOGF( "%s: cookieID still 0; most likely packet was lost", + XP_LOGF( "%s: cookieID still 0; background send?", __func__ ); - comms_transportFailed( comms ); - } else { - comms->r.myHostID = srcID; + } + comms->r.myHostID = srcID; - XP_LOGF( "%s: set hostid: %x", __func__, comms->r.myHostID ); + XP_LOGF( "%s: set hostid: %x", __func__, comms->r.myHostID ); #ifdef DEBUG - { - XP_UCHAR connName[MAX_CONNNAME_LEN+1]; - stringFromStreamHere( stream, connName, sizeof(connName) ); - XP_ASSERT( comms->r.connName[0] == '\0' - || 0 == XP_STRCMP( comms->r.connName, connName ) ); - XP_MEMCPY( comms->r.connName, connName, sizeof(comms->r.connName) ); - XP_LOGF( "%s: connName: \"%s\"", __func__, connName ); - } + { + XP_UCHAR connName[MAX_CONNNAME_LEN+1]; + stringFromStreamHere( stream, connName, sizeof(connName) ); + XP_ASSERT( comms->r.connName[0] == '\0' + || 0 == XP_STRCMP( comms->r.connName, connName ) ); + XP_MEMCPY( comms->r.connName, connName, + sizeof(comms->r.connName) ); + XP_LOGF( "%s: connName: \"%s\"", __func__, connName ); + } #else - stringFromStreamHere( stream, comms->r.connName, - sizeof(comms->r.connName) ); + stringFromStreamHere( stream, comms->r.connName, + sizeof(comms->r.connName) ); #endif - /* We're [re-]connected now. Send any pending messages. This may - need to be done later since we're inside the platform's socket read - proc now. But don't resend if we were previously REconnected, as - we'll have sent then. -- I don't see any send on RECONNECTED, so - removing the test for now to fix recon problems on android. */ - /* if ( COMMS_RELAYSTATE_RECONNECTED != comms->r.relayState ) { */ - comms_resendAll( comms ); - /* } */ - if ( XWRELAY_ALLHERE == cmd ) { /* initial connect? */ - (*comms->procs.rconnd)( comms->procs.closure, - comms->addr.u.ip_relay.invite, XP_FALSE, - comms->r.myHostID, XP_TRUE, 0 ); - } - set_relay_state( comms, COMMS_RELAYSTATE_ALLCONNECTED ); + /* We're [re-]connected now. Send any pending messages. This may + need to be done later since we're inside the platform's socket + read proc now. But don't resend if we were previously + REconnected, as we'll have sent then. -- I don't see any send + on RECONNECTED, so removing the test for now to fix recon + problems on android. */ + /* if ( COMMS_RELAYSTATE_RECONNECTED != comms->r.relayState ) { */ + comms_resendAll( comms ); + /* } */ + if ( XWRELAY_ALLHERE == cmd ) { /* initial connect? */ + (*comms->procs.rconnd)( comms->procs.closure, + comms->addr.u.ip_relay.invite, XP_FALSE, + comms->r.myHostID, XP_TRUE, 0 ); } + set_relay_state( comms, COMMS_RELAYSTATE_ALLCONNECTED ); break; case XWRELAY_MSG_FROMRELAY: cookieID = stream_getU16( stream ); From 9115b344fb579b6e3f9b81ca602e57fa4a0fc590 Mon Sep 17 00:00:00 2001 From: Eric House Date: Mon, 21 Nov 2011 17:57:38 -0800 Subject: [PATCH 5/5] use a hash of lists rather than a single list to store pending background messages to fix assertion failure when more than two devices are playing: messages must be sent grouped by relayID. --- xwords4/linux/linuxutl.c | 57 ++++++++++++++++++++++------------------ xwords4/linux/main.h | 3 ++- 2 files changed, 33 insertions(+), 27 deletions(-) diff --git a/xwords4/linux/linuxutl.c b/xwords4/linux/linuxutl.c index c8cc8db0a..61d37b0ce 100644 --- a/xwords4/linux/linuxutl.c +++ b/xwords4/linux/linuxutl.c @@ -440,32 +440,35 @@ formatConfirmTrade( const XP_UCHAR** tiles, XP_U16 nTiles, typedef struct _MsgRec { XP_U8* msg; - XP_U16 len; - gchar* relayID; + XP_U16 msglen; } MsgRec; void initNoConnStorage( CommonGlobals* cGlobals ) { XP_ASSERT( NULL == cGlobals->noConnMsgs ); - cGlobals->noConnMsgs = (GSList*)-1; /* -1 is flag meaning "use me" */ + cGlobals->noConnMsgs = g_hash_table_new( g_str_hash, g_str_equal ); } XP_Bool -storeNoConnMsg( CommonGlobals* cGlobals, const XP_U8* msg, XP_U16 len, +storeNoConnMsg( CommonGlobals* cGlobals, const XP_U8* msg, XP_U16 msglen, const XP_UCHAR* relayID ) { - XP_Bool inUse = NULL != cGlobals->noConnMsgs; + XP_ASSERT( 0 < msglen ); + XP_Bool inUse = NULL != cGlobals->noConnMsgs; if ( inUse ) { - if ( (GSList*)-1 == cGlobals->noConnMsgs ) { - cGlobals->noConnMsgs = NULL; - } + GSList* list = g_hash_table_lookup( cGlobals->noConnMsgs, relayID ); + gboolean missing = NULL == list; + MsgRec* msgrec = g_malloc( sizeof(*msgrec) ); - msgrec->msg = g_malloc( len ); - XP_MEMCPY( msgrec->msg, msg, len ); - msgrec->len = len; - msgrec->relayID = g_strdup( relayID ); - cGlobals->noConnMsgs = g_slist_append( cGlobals->noConnMsgs, msgrec ); + msgrec->msg = g_malloc( msglen ); + XP_MEMCPY( msgrec->msg, msg, msglen ); + msgrec->msglen = msglen; + list = g_slist_append( list, msgrec ); + if ( missing ) { + gchar* key = g_strdup( relayID ); /* will leak */ + g_hash_table_insert( cGlobals->noConnMsgs, key, list ); + } } return inUse; } @@ -473,12 +476,14 @@ storeNoConnMsg( CommonGlobals* cGlobals, const XP_U8* msg, XP_U16 len, void writeNoConnMsgs( CommonGlobals* cGlobals, int fd ) { - guint nMsgs = (GSList*)-1 == cGlobals->noConnMsgs ? - 0 : g_slist_length( cGlobals->noConnMsgs ); - if ( 0 < nMsgs ) { - gchar relayID[128] = {0}; - - strcpy( relayID, ((MsgRec*)(cGlobals->noConnMsgs->data))->relayID ); + GHashTable* hash = cGlobals->noConnMsgs; + GList* keys = g_hash_table_get_keys( hash ); + GList* iter; + for ( iter = keys; !!iter; iter = iter->next ) { + XP_UCHAR* relayID = (XP_UCHAR*)iter->data; + GSList* list = (GSList*)g_hash_table_lookup( hash, relayID ); + guint nMsgs = g_slist_length( list ); + XP_ASSERT( 0 < nMsgs ); XWStreamCtxt* stream = mem_stream_make( MPPARM(cGlobals->params->util->mpool) @@ -491,17 +496,14 @@ writeNoConnMsgs( CommonGlobals* cGlobals, int fd ) int ii; for ( ii = 0; ii < nMsgs; ++ii ) { - MsgRec* rec = g_slist_nth_data( cGlobals->noConnMsgs, ii ); - stream_putU16( stream, rec->len ); - stream_putBytes( stream, rec->msg, rec->len ); + MsgRec* rec = g_slist_nth_data( list, ii ); + stream_putU16( stream, rec->msglen ); + stream_putBytes( stream, rec->msg, rec->msglen ); g_free( rec->msg ); - XP_ASSERT( 0 == strcmp( relayID, rec->relayID ) ); - g_free( rec->relayID ); g_free( rec ); } - g_slist_free( cGlobals->noConnMsgs ); - cGlobals->noConnMsgs = NULL; + g_slist_free( list ); XP_U16 siz = stream_getSize( stream ); /* XP_U8 buf[siz]; */ @@ -513,6 +515,9 @@ writeNoConnMsgs( CommonGlobals* cGlobals, int fd ) XP_ASSERT( nwritten == siz ); stream_destroy( stream ); } + g_list_free( keys ); + g_hash_table_unref( hash ); + cGlobals->noConnMsgs = NULL; } /* writeNoConnMsgs */ #ifdef TEXT_MODEL diff --git a/xwords4/linux/main.h b/xwords4/linux/main.h index acc155431..a22e7b453 100644 --- a/xwords4/linux/main.h +++ b/xwords4/linux/main.h @@ -163,7 +163,8 @@ struct CommonGlobals { AddAcceptorFunc addAcceptor; Acceptor acceptor; - GSList* noConnMsgs; + /* hash by relayID of lists of messages */ + GHashTable* noConnMsgs; #ifdef XWFEATURE_RELAY int socket; /* relay */