rewrite lock detector to not use pthread_cancel

...which isn't available on Android
This commit is contained in:
Eric House 2024-08-22 22:00:37 -07:00
parent 466abffc43
commit a5132f22a4
3 changed files with 103 additions and 14 deletions

View file

@ -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 <unistd.h>
#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 );
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

View file

@ -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

View file

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