diff --git a/xwords4/common/xwmutex.c b/xwords4/common/xwmutex.c index 730e56f60..38ce2329d 100644 --- a/xwords4/common/xwmutex.c +++ b/xwords4/common/xwmutex.c @@ -1,4 +1,4 @@ -/* -*-mode: C; fill-column: 78; c-basic-offset: 4; -*- */ +/* -*- compile-command: "cd ../linux && make MEMDEBUG=TRUE -j3"; -*- */ /* * Copyright 2024 by Eric House (xwords@eehouse.org). All rights reserved. * @@ -20,40 +20,90 @@ #include "xwmutex.h" #ifdef DEBUG +#include #define WAIT_ALL_SECS 3 +/* What is this? It's my attempt to, on debug builds and for all or only + chosen mutexes, get an assertion when the app doesn't get a lock in the + specified number of seconds. It's complicated because I want it to run on + Android, which doesn't have pthread_cancel(). + + It works by spawining a thread immediately before calling + pthread_mutex_lock(). If the lock succeeds, a flag is set so that the + thread won't raise an alarm. If the thread wakes from its sleep to find + that that flag isn't yet set, assertion failure... + + The tricky part is how to share state between the checker thread and its + parent (that's calling pthread_mutex_lock().) Normally the parent might + pass a pointer to something on the stack to the thread proc, but normally + the stack frame will be long gone while the thread proc still wants to use + it. So: the flag that the checker thread will check lives in the thread's + own frame. The closure it's passed by the parent lets it set a pointer to + that flag so the parent can see it (the closure struct being in the + caller's scope.) So that the parent doesn't get out of pthread_mutex_lock() + before that pointer is set up, it busy-waits on its being set. Sue me: it's + debug code, and only active when set up for a particular mutex. + */ + typedef struct _WaitState { const char* caller; + const char* file; + int lineNo; XP_U16 waitSecs; + XP_Bool* flagLoc; } WaitState; static void* -checkLockProc(void* closure) +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); + const XP_UCHAR* file = wsp->file; + const XP_UCHAR* caller = wsp->caller; + XP_U16 waitSecs = wsp->waitSecs; + int lineNo = wsp->lineNo; + XP_Bool setMe = XP_FALSE; /* caller will change on success */ + XP_ASSERT( !wsp->flagLoc ); + wsp->flagLoc = &setMe; /* tells busy-waiting caller to run */ + + sleep( waitSecs ); + if ( !setMe ) { + XP_LOGFF( "failed to get mutex in %d secs (caller: %s(), line %d in %s)", + waitSecs, caller, lineNo, file ); + XP_ASSERT(0); + } return NULL; } void -mtx_lock_prv( MutexState* state, XP_U16 waitSecs, const char* caller ) +mtx_lock_prv( MutexState* state, XP_U16 waitSecs, + const char* file, const char* caller, int lineNo ) { - // XP_LOGFF( "(caller=%s, waitSecs=%d)", caller, waitSecs ); - pthread_t waitThread; + if ( 0 == waitSecs ) { + waitSecs =state->waitSecs; + } + WaitState ws = { - .waitSecs = 0 < waitSecs ? waitSecs : state->waitSecs, + .waitSecs = waitSecs, + .file = file, .caller = caller, + .lineNo = lineNo, }; if ( 0 < waitSecs ) { + XP_ASSERT( !ws.flagLoc ); + pthread_t waitThread; (void)pthread_create( &waitThread, NULL, checkLockProc, &ws ); + int count = 0; + while ( !ws.flagLoc ) { + usleep(50); /* wait for thread to start */ + if ( 0 == (++count%10) ) { + XP_LOGFF( "count %d; flagLoc still null", count ); + } + } } pthread_mutex_lock( &state->mutex ); - if ( 0 < waitSecs ) { - (void)pthread_cancel(waitThread); + if ( 0 < ws.waitSecs ) { + *ws.flagLoc = XP_TRUE; } } @@ -94,6 +144,7 @@ mtx_init_prv( MutexState* mutex, XP_Bool recursive } # endif mutex->waitSecs = waitSecs; + XP_LOGFF( "set waitSecs: %d", mutex->waitSecs ); #endif pthread_mutex_init( &mutex->mutex, &attr ); #ifdef DEBUG @@ -118,3 +169,35 @@ mtx_destroy_prv( MutexState* mutex ) { pthread_mutex_destroy( &mutex->mutex ); } + +#ifdef DEBUG +static void* +grabProc( void* closure ) +{ + LOG_FUNC(); + MutexState* ms = (MutexState*)closure; + WITH_MUTEX(ms); + sleep(20); + END_WITH_MUTEX(); + LOG_RETURN_VOID(); + return NULL; +} + +void +mtx_crashToTest() +{ + LOG_FUNC(); + MutexState state1; + MUTEX_INIT_CHECKED( &state1, XP_FALSE, 5 ); + + /* One thread to grab it for 20 seconds */ + pthread_t thread1; + (void)pthread_create( &thread1, NULL, grabProc, &state1 ); + + /* Another to try to grab it */ + pthread_t thread2; + (void)pthread_create( &thread2, NULL, grabProc, &state1 ); + + pthread_join( thread2, NULL ); +} +#endif diff --git a/xwords4/common/xwmutex.h b/xwords4/common/xwmutex.h index 6df930e65..cb61247fa 100644 --- a/xwords4/common/xwmutex.h +++ b/xwords4/common/xwmutex.h @@ -25,12 +25,13 @@ #ifdef DEBUG -void mtx_lock_prv(MutexState* state, XP_U16 waitSecs, const char* caller); +void mtx_lock_prv(MutexState* state, XP_U16 waitSecs, const char* file, + const char* caller, int lineNo); 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__) + mtx_lock_prv(_state, _waitSecs, __FILE__, __func__, __LINE__) # define WITH_MUTEX(STATE) WITH_MUTEX_CHECKED(STATE, 0) # define END_WITH_MUTEX() mtx_unlock_prv(_state, _waitSecs, __func__); \ } @@ -51,9 +52,12 @@ void mtx_init_prv( MutexState* mutex, XP_Bool recursive void mtx_destroy_prv( MutexState* mutex ); #ifdef DEBUG +void mtx_crashToTest(); + # 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 mtx_crashToTest() # define MUTEX_INIT(STATE, RECURSIVE) mtx_init_prv((STATE), (RECURSIVE)) # define MUTEX_INIT_CHECKED(STATE, RECURSIVE, WS) MUTEX_INIT((STATE), (RECURSIVE)) #endif diff --git a/xwords4/linux/linuxmain.c b/xwords4/linux/linuxmain.c index 0a8dd7c69..44cb250c4 100644 --- a/xwords4/linux/linuxmain.c +++ b/xwords4/linux/linuxmain.c @@ -76,6 +76,7 @@ #include "dictiter.h" #include "gsrcwrap.h" #include "dllist.h" +#include "xwmutex.h" /* #include "commgr.h" */ /* #include "compipe.h" */ #include "memstream.h" @@ -2686,6 +2687,7 @@ main( int argc, char** argv ) XP_LOGFF( "%s starting; ptr size: %zu", argv[0], sizeof(argv) ); testDLL(); + // mtx_crashToTest(); // return 0; int opt;