diff --git a/xwords4/common/comms.c b/xwords4/common/comms.c index 4f5389312..b80172b14 100644 --- a/xwords4/common/comms.c +++ b/xwords4/common/comms.c @@ -417,7 +417,7 @@ comms_make( MPFORMAL XWEnv xwe, XW_UtilCtxt* util, XP_Bool isServer, XP_U16 forceChannel ) { CommsCtxt* comms = (CommsCtxt*)XP_CALLOC( mpool, sizeof(*comms) ); - mtx_init( &comms->mutex, XP_TRUE ); + MUTEX_INIT( &comms->mutex, XP_TRUE ); comms->util = util; comms->dutil = util_getDevUtilCtxt( util, xwe ); #ifdef DEBUG @@ -587,7 +587,7 @@ comms_destroy( CommsCtxt* comms, XWEnv xwe ) util_clearTimer( comms->util, xwe, TIMER_COMMS ); END_WITH_MUTEX(); - mtx_destroy( &comms->mutex ); + MUTEX_DESTROY( &comms->mutex ); XP_FREE( comms->mpool, comms ); } /* comms_destroy */ diff --git a/xwords4/common/comtypes.h b/xwords4/common/comtypes.h index 432592199..7c8d824bc 100644 --- a/xwords4/common/comtypes.h +++ b/xwords4/common/comtypes.h @@ -28,6 +28,8 @@ # define RELCONST const #endif +#include + #include "xptypes.h" #ifndef EXTERN_C_START @@ -344,6 +346,14 @@ typedef struct _TrayTileSet { TrayTile tiles[MAX_TRAY_TILES]; } TrayTileSet; +/* making this a struct in case I want to add e.g. a chain of holders */ +typedef struct _MutexState { + pthread_mutex_t mutex; +#ifdef DEBUG + XP_U16 waitSecs; +#endif +} MutexState; + #ifdef XWFEATURE_BLUETOOTH /* temporary debugging hack */ diff --git a/xwords4/common/device.c b/xwords4/common/device.c index de753bed1..c5052dc02 100644 --- a/xwords4/common/device.c +++ b/xwords4/common/device.c @@ -733,6 +733,7 @@ static void delWSDatum( DLHead* elem, void* closure ) { XW_DUtilCtxt* dutil = (XW_DUtilCtxt*)closure; + XP_USE(dutil); /* for release builds */ XP_FREEP( dutil->mpool, &elem ); } @@ -1178,8 +1179,8 @@ dvc_init( XW_DUtilCtxt* dutil, XWEnv xwe ) dc->webSendData = NULL; dc->mWebSendKey = 0; - mtx_init( &dc->webSendMutex, XP_FALSE ); - mtx_init( &dc->ackTimer.mutex, XP_FALSE ); + MUTEX_INIT( &dc->webSendMutex, XP_FALSE ); + MUTEX_INIT( &dc->ackTimer.mutex, XP_FALSE ); loadPhoniesData( dutil, xwe, dc ); @@ -1195,8 +1196,8 @@ dvc_cleanup( XW_DUtilCtxt* dutil, XWEnv xwe ) DevCtxt* dc = freePhonyState( dutil, xwe ); freeWSState( dutil, dc ); - mtx_destroy( &dc->webSendMutex ); - mtx_destroy( &dc->ackTimer.mutex ); + MUTEX_DESTROY( &dc->webSendMutex ); + MUTEX_DESTROY( &dc->ackTimer.mutex ); XP_FREEP( dutil->mpool, &dc ); } diff --git a/xwords4/common/dictmgr.c b/xwords4/common/dictmgr.c index bfc0b7d42..f769bed91 100644 --- a/xwords4/common/dictmgr.c +++ b/xwords4/common/dictmgr.c @@ -64,7 +64,7 @@ DictMgrCtxt* dmgr_make( MPFORMAL_NOCOMMA ) { DictMgrCtxt* dmgr = XP_CALLOC( mpool, sizeof(*dmgr) ); - mtx_init( &dmgr->mutex, XP_FALSE ); + MUTEX_INIT( &dmgr->mutex, XP_FALSE ); MPASSIGN( dmgr->mpool, mpool ); return dmgr; } @@ -78,7 +78,7 @@ dmgr_destroy( DictMgrCtxt* dmgr, XWEnv xwe ) dict_unref( pair->dict, xwe ); XP_FREEP( dmgr->mpool, &pair->key ); } - mtx_destroy( &dmgr->mutex ); + MUTEX_DESTROY( &dmgr->mutex ); XP_FREE( dmgr->mpool, dmgr ); } @@ -87,7 +87,7 @@ dmgr_get( DictMgrCtxt* dmgr, XWEnv xwe, const XP_UCHAR* key ) { const DictionaryCtxt* result = NULL; - WITH_MUTEX( &dmgr->mutex ); + WITH_MUTEX_CHECKED( &dmgr->mutex, 1 ); XP_S16 index = findFor( dmgr, key ); if ( 0 <= index ) { diff --git a/xwords4/common/dictnry.c b/xwords4/common/dictnry.c index be27a9ccd..9e536afe3 100644 --- a/xwords4/common/dictnry.c +++ b/xwords4/common/dictnry.c @@ -85,7 +85,7 @@ p_dict_unref( const DictionaryCtxt* dict, XWEnv xwe if ( 0 == _dict->refCount ) { /* There's a race here. If another thread locks the mutex we'll still destroy the dict (and the locked mutex!!!) PENDING */ - mtx_destroy( &_dict->mutex ); + MUTEX_DESTROY( &_dict->mutex ); (*dict->destructor)( _dict, xwe ); } } @@ -1209,7 +1209,7 @@ dict_super_init( MPFORMAL DictionaryCtxt* dict ) dict->func_dict_edge_with_tile = dict_super_edge_with_tile; dict->func_dict_getShortName = dict_getName; - mtx_init( &dict->mutex, XP_FALSE ); + MUTEX_INIT( &dict->mutex, XP_FALSE ); } /* dict_super_init */ void diff --git a/xwords4/common/dictnry.h b/xwords4/common/dictnry.h index 086546855..7bd42eb07 100644 --- a/xwords4/common/dictnry.h +++ b/xwords4/common/dictnry.h @@ -32,7 +32,6 @@ #include "dawg.h" #include "model.h" #include "mempool.h" -#include "xwmutex.h" #ifdef CPLUS extern "C" { diff --git a/xwords4/common/dutil.c b/xwords4/common/dutil.c index cd255fb67..a5e370473 100644 --- a/xwords4/common/dutil.c +++ b/xwords4/common/dutil.c @@ -27,6 +27,7 @@ #include "device.h" #include "stats.h" #include "timers.h" +#include "xwmutex.h" static void super_dutil_storeStream( XW_DUtilCtxt* duc, XWEnv xwe, const XP_UCHAR* keys[], @@ -59,7 +60,7 @@ void dutil_super_init( MPFORMAL XW_DUtilCtxt* dutil ) { #ifdef XWFEATURE_KNOWNPLAYERS - mtx_init( &dutil->kpMutex, XP_FALSE ); + MUTEX_INIT( &dutil->kpMutex, XP_FALSE ); #endif MPASSIGN( dutil->mpool, mpool ); diff --git a/xwords4/common/dutil.h b/xwords4/common/dutil.h index 87471b9d3..faf0c3897 100644 --- a/xwords4/common/dutil.h +++ b/xwords4/common/dutil.h @@ -28,7 +28,6 @@ #include "vtabmgr.h" #include "commstyp.h" #include "nlityp.h" -#include "xwmutex.h" #include "cJSON.h" typedef enum { UNPAUSED, diff --git a/xwords4/common/knownplyr.c b/xwords4/common/knownplyr.c index f46d55069..3b3e6dc65 100644 --- a/xwords4/common/knownplyr.c +++ b/xwords4/common/knownplyr.c @@ -25,6 +25,7 @@ #include "comms.h" #include "dbgutil.h" #include "dllist.h" +#include "xwmutex.h" typedef struct _KnownPlayer { DLHead links; diff --git a/xwords4/common/mempool.c b/xwords4/common/mempool.c index 965ae41ee..03c4df138 100644 --- a/xwords4/common/mempool.c +++ b/xwords4/common/mempool.c @@ -64,7 +64,7 @@ mpool_make( const XP_UCHAR* tag ) { MemPoolCtx* result = (MemPoolCtx*)XP_PLATMALLOC( sizeof(*result) ); XP_MEMSET( result, 0, sizeof(*result) ); - mtx_init( &result->mutex, XP_TRUE ); + MUTEX_INIT( &result->mutex, XP_TRUE ); mpool_setTag( result, tag ); return result; } /* mpool_make */ @@ -160,7 +160,7 @@ mpool_destroy( MemPoolCtx* mpool ) #endif freeList( mpool->freeList ); - mtx_destroy( &mpool->mutex ); + MUTEX_DESTROY( &mpool->mutex ); XP_PLATFREE( mpool ); } /* mpool_destroy */ diff --git a/xwords4/common/smsproto.c b/xwords4/common/smsproto.c index 3fc5350a5..8695e65e0 100644 --- a/xwords4/common/smsproto.c +++ b/xwords4/common/smsproto.c @@ -121,7 +121,7 @@ SMSProto* smsproto_init( MPFORMAL XWEnv xwe, XW_DUtilCtxt* dutil ) { SMSProto* state = (SMSProto*)XP_CALLOC( mpool, sizeof(*state) ); - mtx_init( &state->mutex, XP_FALSE ); + MUTEX_INIT( &state->mutex, XP_FALSE ); state->dutil = dutil; MPASSIGN( state->mpool, mpool ); @@ -157,7 +157,7 @@ smsproto_free( SMSProto* state ) } XP_ASSERT( !state->fromPhoneRecs ); /* above nulls this once empty */ - mtx_destroy( &state->mutex ); + MUTEX_DESTROY( &state->mutex ); XP_FREEP( state->mpool, &state ); } diff --git a/xwords4/common/stats.c b/xwords4/common/stats.c index fdef70567..577742d66 100644 --- a/xwords4/common/stats.c +++ b/xwords4/common/stats.c @@ -18,13 +18,14 @@ */ #include "stats.h" -#include "xwmutex.h" #include "device.h" #include "xwstream.h" #include "strutils.h" #include "dbgutil.h" #include "timers.h" +#include "xwmutex.h" + typedef struct StatsState { XP_U32* statsVals; MutexState mutex; @@ -40,7 +41,7 @@ void sts_init( XW_DUtilCtxt* dutil ) { StatsState* ss = XP_CALLOC( dutil->mpool, sizeof(*ss) ); - mtx_init( &ss->mutex, XP_TRUE ); + MUTEX_INIT( &ss->mutex, XP_TRUE ); dutil->statsState = ss; } @@ -49,7 +50,7 @@ sts_cleanup( XW_DUtilCtxt* dutil, XWEnv XP_UNUSED(xwe) ) { StatsState* ss = dutil->statsState; XP_ASSERT( !!ss ); - mtx_destroy( &ss->mutex ); + MUTEX_DESTROY( &ss->mutex ); XP_FREEP( dutil->mpool, &ss->statsVals ); XP_FREEP( dutil->mpool, &ss ); } @@ -180,7 +181,7 @@ loadCounts( XW_DUtilCtxt* dutil, XWEnv xwe ) #ifdef DUTIL_TIMERS static void -onStoreTimer( void* closure, XWEnv xwe, XP_Bool fired ) +onStoreTimer( void* closure, XWEnv xwe, XP_Bool XP_UNUSED_DBG(fired) ) { XP_LOGFF( "(fired: %s)", boolToStr(fired) ); XW_DUtilCtxt* dutil = (XW_DUtilCtxt*)closure; @@ -204,7 +205,10 @@ setStoreTimerLocked( XW_DUtilCtxt* dutil, XWEnv xwe ) if ( !ss->timerSet ) { ss->timerSet = XP_TRUE; XP_U32 inWhenMS = 5 * 1000; - TimerKey key = tmr_set( dutil, xwe, inWhenMS, onStoreTimer, dutil ); +#ifdef DEBUG + TimerKey key = +#endif + tmr_set( dutil, xwe, inWhenMS, onStoreTimer, dutil ); XP_LOGFF( "tmr_set() => %d", key ); } else { XP_LOGFF( "timer already set" ); diff --git a/xwords4/common/timers.c b/xwords4/common/timers.c index 08e4877c4..24a091b09 100644 --- a/xwords4/common/timers.c +++ b/xwords4/common/timers.c @@ -47,7 +47,7 @@ tmr_init( XW_DUtilCtxt* dutil ) TimerMgrState* tms = XP_CALLOC( dutil->mpool, sizeof(*tms) ); tms->dutil = dutil; dutil->timersState = tms; - mtx_init( &tms->mutex, XP_TRUE ); + MUTEX_INIT( &tms->mutex, XP_TRUE ); } void @@ -56,7 +56,7 @@ tmr_cleanup( XW_DUtilCtxt* dutil, XWEnv xwe ) TimerMgrState* timersState = (TimerMgrState*)dutil->timersState; XP_ASSERT( !!timersState ); clearPendings( dutil, xwe ); - mtx_destroy( &timersState->mutex ); + MUTEX_DESTROY( &timersState->mutex ); XP_FREEP( dutil->mpool, &dutil->timersState ); } @@ -85,7 +85,8 @@ tmr_set( XW_DUtilCtxt* dutil, XWEnv xwe, XP_U32 inWhen, } static void -timerFired( XW_DUtilCtxt* dutil, XWEnv xwe, TimerState* timer, XP_Bool fired ) +timerFired( XW_DUtilCtxt* XP_UNUSED_DBG(dutil), XWEnv xwe, TimerState* timer, + XP_Bool fired ) { XP_LOGFF( "(timer=%p); key=%d", timer, timer->key ); (*timer->proc)( timer->closure, xwe, fired ); diff --git a/xwords4/common/xwmutex.c b/xwords4/common/xwmutex.c index 0fff12e85..730e56f60 100644 --- a/xwords4/common/xwmutex.c +++ b/xwords4/common/xwmutex.c @@ -19,34 +19,102 @@ #include "xwmutex.h" +#ifdef DEBUG + +#define WAIT_ALL_SECS 3 + +typedef struct _WaitState { + const char* caller; + XP_U16 waitSecs; +} WaitState; + +static void* +checkLockProc(void* closure) +{ + WaitState* wsp = (WaitState*)closure; + sleep( wsp->waitSecs ); + XP_LOGFF( "failed to get mutex in %d secs (caller: %s())", + wsp->waitSecs, wsp->caller ); + XP_ASSERT(0); + return NULL; +} + void -mtx_init( MutexState* mutex, XP_Bool recursive ) +mtx_lock_prv( MutexState* state, XP_U16 waitSecs, const char* caller ) +{ + // XP_LOGFF( "(caller=%s, waitSecs=%d)", caller, waitSecs ); + pthread_t waitThread; + WaitState ws = { + .waitSecs = 0 < waitSecs ? waitSecs : state->waitSecs, + .caller = caller, + }; + if ( 0 < waitSecs ) { + (void)pthread_create( &waitThread, NULL, checkLockProc, &ws ); + } + pthread_mutex_lock( &state->mutex ); + if ( 0 < waitSecs ) { + (void)pthread_cancel(waitThread); + } +} + +void +mtx_unlock_prv( MutexState* state, XP_U16 XP_UNUSED(waitSecs), + const char* XP_UNUSED(caller) ) +{ + // XP_LOGFF( "(caller=%s, waitSecs=%d)", caller, waitSecs ); + pthread_mutex_unlock( &state->mutex ); +} +#endif + +void +mtx_init_prv( MutexState* mutex, XP_Bool recursive +#ifdef DEBUG + , XP_U16 waitSecs +#endif + ) { pthread_mutexattr_t attr; - int ret = pthread_mutexattr_init(&attr); +#ifdef DEBUG + int ret = +#endif + pthread_mutexattr_init(&attr); XP_ASSERT(0 == ret); if ( recursive ) { - ret = pthread_mutexattr_settype(&attr, +#ifdef DEBUG + ret = +#endif + pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE); XP_ASSERT(0 == ret); } +#ifdef DEBUG +# ifdef WAIT_ALL_SECS + if ( waitSecs < WAIT_ALL_SECS ) { + waitSecs = WAIT_ALL_SECS; + } +# endif + mutex->waitSecs = waitSecs; +#endif pthread_mutex_init( &mutex->mutex, &attr ); - ret = pthread_mutexattr_destroy(&attr); +#ifdef DEBUG + ret = +#endif + pthread_mutexattr_destroy(&attr); XP_ASSERT(0 == ret); -#ifdef DEBUG - /* if ( recursive ) { */ - /* XP_LOGFF( "testing recursive call..." ); */ - /* WITH_MUTEX( mutex ); */ - /* WITH_MUTEX( mutex ); */ - /* END_WITH_MUTEX(); */ - /* END_WITH_MUTEX(); */ - /* } */ -#endif +/* #ifdef DEBUG */ +/* if ( recursive ) { */ +/* XP_LOGFF( "testing recursive call..." ); */ +/* WITH_MUTEX( mutex ); */ +/* WITH_MUTEX( mutex ); */ +/* END_WITH_MUTEX(); */ +/* END_WITH_MUTEX(); */ +/* } */ +/* #endif */ } void -mtx_destroy( MutexState* mutex ) +mtx_destroy_prv( MutexState* mutex ) { pthread_mutex_destroy( &mutex->mutex ); } diff --git a/xwords4/common/xwmutex.h b/xwords4/common/xwmutex.h index 56376cf18..6df930e65 100644 --- a/xwords4/common/xwmutex.h +++ b/xwords4/common/xwmutex.h @@ -20,59 +20,44 @@ #ifndef _XWMUTEX_H_ #define _XWMUTEX_H_ -#include #include "xptypes.h" - -/* Making this a struct in case I want to add e.g. a chain of holders */ -typedef struct _MutexState { - pthread_mutex_t mutex; -} MutexState; - -#ifdef MUTEX_LOG_VERBOSE -# define MUTEX_LOG(...) XP_LOGFF(__VA_ARGS__) -#else -# define MUTEX_LOG(...) -#endif - -#define WITH_MUTEX_LOCK_DEBUG(STATEP) { \ - MutexState* _state = (STATEP); \ - time_t startTime = time(NULL); \ - MUTEX_LOG( "blocking for mutex %p", _state ); \ - pthread_mutex_lock(&_state->mutex); \ - MUTEX_LOG( "got mutex %p", _state ); \ - time_t gotItTime = time(NULL); \ - time_t _elapsed = gotItTime-startTime; \ - if ( 0 < _elapsed ) { \ - XP_LOGFF("took %lds to get mutex", _elapsed); \ - } \ - -#define WITH_MUTEX_UNLOCK_DEBUG() \ - time_t unlockTime = time(NULL); \ - _elapsed = unlockTime-gotItTime; \ - if ( 0 < _elapsed ) { \ - XP_LOGFF("held mutex for %lds", _elapsed); \ - } \ - pthread_mutex_unlock(&_state->mutex); \ - MUTEX_LOG( "released mutex %p", _state ); \ - } \ - -#define WITH_MUTEX_LOCK_RELEASE(COMMS) { \ - MutexState* _state = (STATEP); \ - pthread_mutex_lock(&_state->mutex); \ - -#define WITH_MUTEX_UNLOCK_RELEASE() \ - pthread_mutex_unlock(&_state->mutex); \ - } \ +#include "comtypes.h" #ifdef DEBUG -#define WITH_MUTEX WITH_MUTEX_LOCK_DEBUG -#define END_WITH_MUTEX WITH_MUTEX_UNLOCK_DEBUG + +void mtx_lock_prv(MutexState* state, XP_U16 waitSecs, const char* caller); +void mtx_unlock_prv(MutexState* state, XP_U16 waitSecs, const char* caller); +# define WITH_MUTEX_CHECKED(STATE, SECS) { \ + MutexState* _state = (STATE); \ + XP_U16 _waitSecs = (SECS); \ + mtx_lock_prv(_state, _waitSecs, __func__) +# define WITH_MUTEX(STATE) WITH_MUTEX_CHECKED(STATE, 0) +# define END_WITH_MUTEX() mtx_unlock_prv(_state, _waitSecs, __func__); \ + } #else -#define WITH_MUTEX WITH_MUTEX_LOCK_RELEASE -#define END_WITH_MUTEX WITH_MUTEX_UNLOCK_RELEASE +# define WITH_MUTEX(STATE) { \ + const pthread_mutex_t* _mutex = &(STATE)->mutex; \ + pthread_mutex_lock((pthread_mutex_t*)_mutex) +# define WITH_MUTEX_CHECKED(STATE, IGNORE) WITH_MUTEX(STATE) +# define END_WITH_MUTEX() pthread_mutex_unlock((pthread_mutex_t*)_mutex); \ + } #endif -void mtx_init( MutexState* mutex, XP_Bool recursive ); -void mtx_destroy( MutexState* mutex ); +void mtx_init_prv( MutexState* mutex, XP_Bool recursive +# ifdef DEBUG + , XP_U16 waitSecs +# endif + ); +void mtx_destroy_prv( MutexState* mutex ); + +#ifdef DEBUG +# define MUTEX_INIT_CHECKED(STATE, RECURSIVE, WS) mtx_init_prv((STATE), (RECURSIVE), (WS)) +# define MUTEX_INIT(STATE, RECURSIVE) MUTEX_INIT_CHECKED(STATE, RECURSIVE, 0) +#else +# define MUTEX_INIT(STATE, RECURSIVE) mtx_init_prv((STATE), (RECURSIVE)) +# define MUTEX_INIT_CHECKED(STATE, RECURSIVE, WS) MUTEX_INIT((STATE), (RECURSIVE)) +#endif + +#define MUTEX_DESTROY(STATE) mtx_destroy_prv((STATE)) #endif