rewrite mutex stuff to make timeout/asserting easier

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 */
 

xwords4/common/comtypes.h

@@ -28,6 +28,8 @@
 # define RELCONST const
 #endif
 
+#include <pthread.h>
+
 #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 */
 

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 );
+    MUTEX_INIT( &dc->webSendMutex, XP_FALSE );
-    mtx_init( &dc->ackTimer.mutex, 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 );
+    MUTEX_DESTROY( &dc->webSendMutex );
-    mtx_destroy( &dc->ackTimer.mutex );
+    MUTEX_DESTROY( &dc->ackTimer.mutex );
 
     XP_FREEP( dutil->mpool, &dc );
 }

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 ) {

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

xwords4/common/dictnry.h

@@ -32,7 +32,6 @@
 #include "dawg.h"
 #include "model.h"
 #include "mempool.h"
-#include "xwmutex.h"
 
 #ifdef CPLUS
 extern "C" {

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 );

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,

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;

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 */
 

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 );
     }

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" );

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 );

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
+/* #ifdef DEBUG */
-    /* if ( recursive ) { */
+/*     if ( recursive ) { */
-    /*     XP_LOGFF( "testing recursive call..." ); */
+/*         XP_LOGFF( "testing recursive call..." ); */
-    /*     WITH_MUTEX( mutex ); */
+/*         WITH_MUTEX( mutex ); */
-    /*     WITH_MUTEX( mutex ); */
+/*         WITH_MUTEX( mutex ); */
-    /*     END_WITH_MUTEX(); */
+/*         END_WITH_MUTEX(); */
-    /*     END_WITH_MUTEX(); */
+/*         END_WITH_MUTEX(); */
-    /* } */
+/*     } */
-#endif
+/* #endif */
 }
 
 void
-mtx_destroy( MutexState* mutex )
+mtx_destroy_prv( MutexState* mutex )
 {
     pthread_mutex_destroy( &mutex->mutex );
 }

xwords4/common/xwmutex.h

@@ -20,59 +20,44 @@
 #ifndef _XWMUTEX_H_
 #define _XWMUTEX_H_
 
-#include <pthread.h>
 #include "xptypes.h"
-
+#include "comtypes.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);       \
-    }                                           \
 
 #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 WITH_MUTEX(STATE) {                            \
-#define END_WITH_MUTEX WITH_MUTEX_UNLOCK_RELEASE
+    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_init_prv( MutexState* mutex, XP_Bool recursive
-void mtx_destroy( MutexState* mutex );
+# 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