From a13e1d6488d304ca0d8b27a07dc414707201c8db Mon Sep 17 00:00:00 2001 From: ehouse Date: Mon, 21 Sep 2009 12:49:08 +0000 Subject: [PATCH] add util_clearTimer, implement on all platforms, and call when closing comms to fix crash on Wince when timer fired after comms had been deleted. The closure stored and passed in was no longer a valid ptr. --- xwords4/common/comms.c | 9 ++++++- xwords4/common/util.h | 3 +++ xwords4/linux/cursesmain.c | 8 ++++++ xwords4/linux/gtkmain.c | 8 ++++++ xwords4/linux/linuxmain.c | 6 ++++- xwords4/palm/palmmain.c | 21 ++++++++++----- xwords4/wince/cemain.c | 55 +++++++++++++++++++++----------------- xwords4/wince/cemain.h | 12 ++++++--- 8 files changed, 86 insertions(+), 36 deletions(-) diff --git a/xwords4/common/comms.c b/xwords4/common/comms.c index 8759c02b0..780e734ba 100644 --- a/xwords4/common/comms.c +++ b/xwords4/common/comms.c @@ -336,7 +336,7 @@ set_reset_timer( CommsCtxt* comms ) /* This timer is allowed to overwrite a heartbeat timer, but not vice-versa. Make sure we can restart it. */ comms->hbTimerPending = XP_FALSE; - util_setTimer( comms->util, TIMER_COMMS, 15, /* five seconds */ + util_setTimer( comms->util, TIMER_COMMS, 15, p_comms_resetTimer, comms ); comms->reconTimerPending = XP_TRUE; } @@ -345,6 +345,7 @@ void comms_transportFailed( CommsCtxt* comms ) { LOG_FUNC(); + XP_ASSERT( !!comms ); if ( COMMS_CONN_RELAY == comms->addr.conType ) { relayDisconnect( comms ); @@ -364,6 +365,8 @@ comms_destroy( CommsCtxt* comms ) cleanupInternal( comms ); cleanupAddrRecs( comms ); + util_clearTimer( comms->util, TIMER_COMMS ); + XP_FREE( comms->mpool, comms ); } /* comms_destroy */ @@ -536,6 +539,7 @@ setDoHeartbeat( CommsCtxt* comms ) void comms_start( CommsCtxt* comms ) { + XP_ASSERT( !!comms ); setDoHeartbeat( comms ); sendConnect( comms, XP_FALSE ); } /* comms_start */ @@ -807,6 +811,7 @@ makeElemWithID( CommsCtxt* comms, MsgID msgID, AddressRecord* rec, XP_S16 comms_send( CommsCtxt* comms, XWStreamCtxt* stream ) { + XP_ASSERT( !!comms ); XP_PlayerAddr channelNo = stream_getAddress( stream ); AddressRecord* rec = getRecordFor( comms, NULL, channelNo ); MsgID msgID = (!!rec)? ++rec->nextMsgID : 0; @@ -988,6 +993,7 @@ sendMsg( CommsCtxt* comms, MsgQueueElem* elem ) XP_S16 comms_resendAll( CommsCtxt* comms ) { + XP_ASSERT( !!comms ); MsgQueueElem* msg; XP_S16 result = 0; @@ -1359,6 +1365,7 @@ XP_Bool comms_checkIncomingStream( CommsCtxt* comms, XWStreamCtxt* stream, const CommsAddrRec* retAddr ) { + XP_ASSERT( !!comms ); XP_Bool messageValid = XP_FALSE; XWHostID senderID = 0; /* unset; default for non-relay cases */ XP_Bool usingRelay = XP_FALSE; diff --git a/xwords4/common/util.h b/xwords4/common/util.h index c80652c0f..9b77df6c1 100644 --- a/xwords4/common/util.h +++ b/xwords4/common/util.h @@ -138,6 +138,7 @@ typedef struct UtilVtable { void (*m_util_setTimer)( XW_UtilCtxt* uc, XWTimerReason why, XP_U16 when, XWTimerProc proc, void* closure ); + void (*m_util_clearTimer)( XW_UtilCtxt* uc, XWTimerReason why ); void (*m_util_requestTime)( XW_UtilCtxt* uc ); @@ -226,6 +227,8 @@ struct XW_UtilCtxt { #define util_setTimer( uc, why, when, proc, clos ) \ (uc)->vtable->m_util_setTimer((uc),(why),(when),(proc),(clos)) +#define util_clearTimer( uc, why ) \ + (uc)->vtable->m_util_clearTimer((uc),(why)) #define util_requestTime( uc ) \ (uc)->vtable->m_util_requestTime((uc)) diff --git a/xwords4/linux/cursesmain.c b/xwords4/linux/cursesmain.c index 071c76a99..91641e77b 100644 --- a/xwords4/linux/cursesmain.c +++ b/xwords4/linux/cursesmain.c @@ -377,6 +377,13 @@ curses_util_setTimer( XW_UtilCtxt* uc, XWTimerReason why, XP_U16 when, } } /* curses_util_setTimer */ +static void +curses_util_clearTimer( XW_UtilCtxt* uc, XWTimerReason why ) +{ + CursesAppGlobals* globals = (CursesAppGlobals*)uc->closure; + globals->cGlobals.timerInfo[why].proc = NULL; +} + static void curses_util_requestTime( XW_UtilCtxt* uc ) { @@ -1291,6 +1298,7 @@ setupCursesUtilCallbacks( CursesAppGlobals* globals, XW_UtilCtxt* util ) curses_util_engineProgressCallback; util->vtable->m_util_setTimer = curses_util_setTimer; + util->vtable->m_util_clearTimer = curses_util_clearTimer; util->vtable->m_util_requestTime = curses_util_requestTime; util->closure = globals; diff --git a/xwords4/linux/gtkmain.c b/xwords4/linux/gtkmain.c index f4ad09db9..951573974 100644 --- a/xwords4/linux/gtkmain.c +++ b/xwords4/linux/gtkmain.c @@ -1340,6 +1340,13 @@ gtk_util_setTimer( XW_UtilCtxt* uc, XWTimerReason why, globals->timerSources[why-1] = newSrc; } /* gtk_util_setTimer */ +static void +gtk_util_clearTimer( XW_UtilCtxt* uc, XWTimerReason why ) +{ + GtkAppGlobals* globals = (GtkAppGlobals*)uc->closure; + globals->cGlobals.timerInfo[why].proc = NULL; +} + static gint idle_func( gpointer data ) { @@ -1633,6 +1640,7 @@ setupGtkUtilCallbacks( GtkAppGlobals* globals, XW_UtilCtxt* util ) util->vtable->m_util_engineProgressCallback = gtk_util_engineProgressCallback; util->vtable->m_util_setTimer = gtk_util_setTimer; + util->vtable->m_util_clearTimer = gtk_util_clearTimer; util->vtable->m_util_requestTime = gtk_util_requestTime; util->vtable->m_util_warnIllegalWord = gtk_util_warnIllegalWord; util->vtable->m_util_remSelected = gtk_util_remSelected; diff --git a/xwords4/linux/linuxmain.c b/xwords4/linux/linuxmain.c index 9ee369932..08c924e87 100644 --- a/xwords4/linux/linuxmain.c +++ b/xwords4/linux/linuxmain.c @@ -584,7 +584,11 @@ linuxFireTimer( CommonGlobals* cGlobals, XWTimerReason why ) tip->proc = NULL; - (*proc)( closure, why ); + if ( !!proc ) { + (*proc)( closure, why ); + } else { + XP_LOGF( "%s: skipping timer %d; cancelled?", __func__, why ); + } } /* fireTimer */ #ifndef XWFEATURE_STANDALONE_ONLY diff --git a/xwords4/palm/palmmain.c b/xwords4/palm/palmmain.c index 20f156f57..131878013 100644 --- a/xwords4/palm/palmmain.c +++ b/xwords4/palm/palmmain.c @@ -123,6 +123,7 @@ static XP_Bool palm_util_hiliteCell( XW_UtilCtxt* uc, XP_U16 col, static XP_Bool palm_util_engineProgressCallback( XW_UtilCtxt* uc ); static void palm_util_setTimer( XW_UtilCtxt* uc, XWTimerReason why, XP_U16 when, XWTimerProc proc, void* closure ); +static void palm_util_clearTimer( XW_UtilCtxt* uc, XWTimerReason why ); static XP_Bool palm_util_altKeyDown( XW_UtilCtxt* uc ); static XP_U32 palm_util_getCurSeconds( XW_UtilCtxt* uc ); static void palm_util_requestTime( XW_UtilCtxt* uc ); @@ -664,6 +665,7 @@ initUtilFuncs( PalmAppGlobals* globals ) vtable->m_util_hiliteCell = palm_util_hiliteCell; vtable->m_util_engineProgressCallback = palm_util_engineProgressCallback; vtable->m_util_setTimer = palm_util_setTimer; + vtable->m_util_clearTimer = palm_util_clearTimer; vtable->m_util_requestTime = palm_util_requestTime; vtable->m_util_altKeyDown = palm_util_altKeyDown; vtable->m_util_getCurSeconds = palm_util_getCurSeconds; @@ -1572,16 +1574,16 @@ palmFireTimer( PalmAppGlobals* globals, XWTimerReason why ) static XP_Bool timeForTimer( PalmAppGlobals* globals, XWTimerReason* why, XP_U32* when ) { - XP_U16 i; + XP_U16 ii; XWTimerReason nextWhy = 0; XP_U32 nextWhen = 0xFFFFFFFF; XP_Bool found; - for ( i = 1; i < NUM_PALM_TIMERS; ++i ) { - if ( (globals->timerProcs[i] != NULL) && - (globals->timerFireAt[i] < nextWhen) ) { - nextWhy = i; - nextWhen = globals->timerFireAt[i]; + for ( ii = 1; ii < NUM_PALM_TIMERS; ++ii ) { + if ( (globals->timerProcs[ii] != NULL) && + (globals->timerFireAt[ii] < nextWhen) ) { + nextWhy = ii; + nextWhen = globals->timerFireAt[ii]; } } @@ -3908,6 +3910,13 @@ palm_util_setTimer( XW_UtilCtxt* uc, XWTimerReason why, postEmptyEvent( noopEvent ); } /* palm_util_setTimer */ +static void +palm_util_clearTimer( XW_UtilCtxt* uc, XWTimerReason why ) +{ + PalmAppGlobals* globals = (PalmAppGlobals*)uc->closure; + globals->timerProcs[why] = NULL; +} + static XP_Bool palm_util_altKeyDown( XW_UtilCtxt* XP_UNUSED(uc) ) { diff --git a/xwords4/wince/cemain.c b/xwords4/wince/cemain.c index 914d47bcb..61b7c1754 100755 --- a/xwords4/wince/cemain.c +++ b/xwords4/wince/cemain.c @@ -113,6 +113,7 @@ static XP_Bool ce_util_hiliteCell( XW_UtilCtxt* uc, XP_U16 col, static XP_Bool ce_util_engineProgressCallback( XW_UtilCtxt* uc ); static void ce_util_setTimer( XW_UtilCtxt* uc, XWTimerReason why, XP_U16 when, XWTimerProc proc, void* closure); +static void ce_util_clearTimer( XW_UtilCtxt* uc, XWTimerReason why ); static XP_Bool ce_util_altKeyDown( XW_UtilCtxt* uc ); static void ce_util_requestTime( XW_UtilCtxt* uc ); static XP_U32 ce_util_getCurSeconds( XW_UtilCtxt* uc ); @@ -360,6 +361,7 @@ ceInitUtilFuncs( CEAppGlobals* globals ) vtable->m_util_hiliteCell = ce_util_hiliteCell; vtable->m_util_engineProgressCallback = ce_util_engineProgressCallback; vtable->m_util_setTimer = ce_util_setTimer; + vtable->m_util_clearTimer = ce_util_clearTimer; vtable->m_util_altKeyDown = ce_util_altKeyDown; vtable->m_util_requestTime = ce_util_requestTime; vtable->m_util_getCurSeconds = ce_util_getCurSeconds; @@ -2146,14 +2148,12 @@ static XP_Bool ceFireTimer( CEAppGlobals* globals, XWTimerReason why ) { XP_Bool draw = XP_FALSE; - XWTimerProc proc; - void* closure; + TimerData* timer = &globals->timerData[why]; + XWTimerProc proc = timer->proc; - proc = globals->timerProcs[why]; if ( !!proc ) { - globals->timerProcs[why] = NULL; - closure = globals->timerClosures[why]; - draw = (*proc)( closure, why ); + timer->proc = NULL; + draw = (*proc)( timer->closure, why ); /* Setting draw after firing timer allows scrolling to happen while pen is held down. This is a hack. Perhaps having the timer proc return whether drawing is needed would be @@ -2175,21 +2175,14 @@ static XP_Bool checkFireLateKeyTimer( CEAppGlobals* globals ) { XP_Bool drop = XP_FALSE; - XWTimerReason whys[] = { TIMER_PENDOWN, TIMER_TIMERTICK -#if defined XWFEATURE_RELAY || defined COMMS_HEARTBEAT - , TIMER_COMMS -#endif - }; + XWTimerReason why; XP_U32 now = GetCurrentTime(); - XP_U16 i; - for ( i = 0; i < sizeof(whys)/sizeof(whys[0]); ++i ) { - XWTimerReason why = whys[i]; - if ( !!globals->timerProcs[why] ) { - if ( now >= globals->timerWhens[why] ) { - (void)ceFireTimer( globals, why ); - drop = XP_TRUE; - } + for ( why = 1; why < NUM_TIMERS_PLUS_ONE; ++why ) { + TimerData* timer = &globals->timerData[why]; + if ( !!timer->proc && now >= timer->when ) { + (void)ceFireTimer( globals, why ); + drop = XP_TRUE; } } @@ -2659,7 +2652,7 @@ WndProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam) /* Kill since they otherwise repeat, but kill before firing as fired proc may set another. */ - (void)KillTimer( hWnd, globals->timerIDs[why] ); + (void)KillTimer( hWnd, globals->timerData[why].id ); draw = ceFireTimer( globals, why ); } @@ -3071,6 +3064,8 @@ ce_send_proc( const XP_U8* buf, XP_U16 len, const CommsAddrRec* addrp, CommsAddrRec addr; LOG_FUNC(); + XP_ASSERT( !!globals->game.comms ); + if ( !addrp ) { comms_getAddr( globals->game.comms, &addr ); addrp = &addr; @@ -3391,10 +3386,11 @@ ce_util_setTimer( XW_UtilCtxt* uc, XWTimerReason why, CEAppGlobals* globals = (CEAppGlobals*)uc->closure; XP_U32 timerID; XP_U32 howLong; + TimerData* timer = &globals->timerData[why]; XP_ASSERT( why < NUM_TIMERS_PLUS_ONE ); - globals->timerProcs[why] = proc; - globals->timerClosures[why] = closure; + timer->proc = proc; + timer->closure = closure; switch ( why ) { case TIMER_PENDOWN: @@ -3413,13 +3409,24 @@ ce_util_setTimer( XW_UtilCtxt* uc, XWTimerReason why, return; } - globals->timerWhens[why] = GetCurrentTime() + howLong; + timer->when = GetCurrentTime() + howLong; timerID = SetTimer( globals->hWnd, why, howLong, NULL); - globals->timerIDs[why] = timerID; + timer->id = timerID; } /* ce_util_setTimer */ +static void +ce_util_clearTimer( XW_UtilCtxt* uc, XWTimerReason why ) +{ + CEAppGlobals* globals = (CEAppGlobals*)uc->closure; + TimerData* timer = &globals->timerData[why]; + if ( !!timer->proc ) { + timer->proc = NULL; + KillTimer( globals->hWnd, timer->id ); + } +} + static XP_Bool ce_util_altKeyDown( XW_UtilCtxt* XP_UNUSED(uc) ) { diff --git a/xwords4/wince/cemain.h b/xwords4/wince/cemain.h index 0a747ee31..a2c1fd804 100755 --- a/xwords4/wince/cemain.h +++ b/xwords4/wince/cemain.h @@ -110,6 +110,13 @@ enum { N_CACHED_PATHS }; +typedef struct _TimerData { + XP_U32 id; + XWTimerProc proc; + void* closure; + XP_U32 when; +} TimerData; + typedef struct _CEAppGlobals { HINSTANCE hInst; HINSTANCE locInst; /* same as hInst if no l10n DLL in use */ @@ -139,10 +146,7 @@ typedef struct _CEAppGlobals { VTableMgr* vtMgr; XP_U16* bonusInfo; - XP_U32 timerIDs[NUM_TIMERS_PLUS_ONE]; - XWTimerProc timerProcs[NUM_TIMERS_PLUS_ONE]; - void* timerClosures[NUM_TIMERS_PLUS_ONE]; - XP_U32 timerWhens[NUM_TIMERS_PLUS_ONE]; + TimerData timerData[NUM_TIMERS_PLUS_ONE]; RECT ownedRects[N_OWNED_RECTS];