Add some assert() statements to the asychronous backend demo to enforce the strategy used to avoid deadlock. Also a minor change to avoid a potential deadlock. (CVS 4520)

FossilOrigin-Name: 6340ca5eee3d398a9ef4f37a442efad37c9bf547

manifest

@@ -1,5 +1,5 @@
-C Add\sa\sprototype\s"group_concat()"\saggregate\sfunction\sto\sfunc.c.\nDisabled\sby\sdefault.\s\sNo\sdocumentation\snor\stest\scases.\s\sNo\seffort\nto\smake\sit\sefficient.\s(CVS\s4519)
+C Add\ssome\sassert()\sstatements\sto\sthe\sasychronous\sbackend\sdemo\sto\senforce\sthe\sstrategy\sused\sto\savoid\sdeadlock.\sAlso\sa\sminor\schange\sto\savoid\sa\spotential\sdeadlock.\s(CVS\s4520)
-D 2007-11-01T17:38:31
+D 2007-11-02T09:07:58
 F Makefile.in 30c7e3ba426ddb253b8ef037d1873425da6009a8
 F Makefile.linux-gcc 65241babba6faf1152bf86574477baab19190499
 F README 9c4e2d6706bdcc3efdd773ce752a8cdab4f90028
@@ -146,7 +146,7 @@ F src/test6.c a9fc983d32d6f262eab300d742e49ff239b0bdbd
 F src/test7.c acec2256c7c2d279db5a8b5fa1a2a68fcc942c67
 F src/test8.c f113aa3723a52113d0fa7c28155ecd37e7e04077
 F src/test9.c b46c8fe02ac7cca1a7316436d8d38d50c66f4b2f
-F src/test_async.c 0bb1e38ca58acfe24f62759981486e9bd4bbf686
+F src/test_async.c dfdc4be8eb675c8ae6b6836e6c105931a2aeb6c4
 F src/test_autoext.c 855157d97aa28cf84233847548bfacda21807436
 F src/test_btree.c c1308ba0b88ab577fa56c9e493a09829dfcded9c
 F src/test_config.c fd6ba4c62dd943e794f00f6ea1e9e32d97bf27f1
@@ -584,7 +584,7 @@ F www/tclsqlite.tcl 8be95ee6dba05eabcd27a9d91331c803f2ce2130
 F www/vdbe.tcl 87a31ace769f20d3627a64fa1fade7fed47b90d0
 F www/version3.tcl 890248cf7b70e60c383b0e84d77d5132b3ead42b
 F www/whentouse.tcl fc46eae081251c3c181bd79c5faef8195d7991a5
-P c249d5da721b32f6fe409a5b55a5d49a58994fec
+P 61987a89d1c4af59c745d1c5f17bab3301588b6c
-R dd9daad3a663125d3750f5ba94a500df
+R d9f327755cf051e49069fca786597d4c
-U drh
+U danielk1977
-Z 6d1463c2813fdf26e95139312eb630a5
+Z a15fd5ac5eb07cc6080324647670aa63

manifest.uuid

@@ -1 +1 @@
-61987a89d1c4af59c745d1c5f17bab3301588b6c
+6340ca5eee3d398a9ef4f37a442efad37c9bf547

src/test_async.c

@@ -157,7 +157,7 @@ static void asyncTrace(const char *zFormat, ...){
 **       async.nFile variables.
 **
 **     * The async.aLock hash-table and all AsyncLock and AsyncFileLock
-**       structures must be protected by teh async.lockMutex mutex.
+**       structures must be protected by the async.lockMutex mutex.
 **
 **     * The file handles from the underlying system are assumed not to 
 **       be thread safe.
@@ -263,9 +263,9 @@ static void asyncTrace(const char *zFormat, ...){
 ** Both async.ioError and async.nFile are protected by async.queueMutex.
 */
 static struct TestAsyncStaticData {
+  pthread_mutex_t lockMutex;   /* For access to aLock hash table */
   pthread_mutex_t queueMutex;  /* Mutex for access to write operation queue */
   pthread_mutex_t writerMutex; /* Prevents multiple writer threads */
-  pthread_mutex_t lockMutex;   /* For access to aLock hash table */
   pthread_cond_t queueSignal;  /* For waking up sleeping writer thread */
   pthread_cond_t emptySignal;  /* Notify when the write queue is empty */
   AsyncWrite *pQueueFirst;     /* Next write operation to be processed */
@@ -412,6 +412,163 @@ struct AsyncFileData {
   AsyncWrite close;
 };
 
+/*
+** The following async_XXX functions are debugging wrappers around the
+** corresponding pthread_XXX functions:
+**
+**     pthread_mutex_lock();
+**     pthread_mutex_unlock();
+**     pthread_mutex_trylock();
+**     pthread_cond_wait();
+**
+** It is illegal to pass any mutex other than those stored in the
+** following global variables of these functions.
+**
+**     async.queueMutex
+**     async.writerMutex
+**     async.lockMutex
+**
+** If NDEBUG is defined, these wrappers do nothing except call the 
+** corresponding pthreads function. If NDEBUG is not defined, then the
+** following variables are used to store the thread-id (as returned
+** by pthread_self()) currently holding the mutex, or 0 otherwise:
+**
+**     asyncdebug.queueMutexHolder
+**     asyncdebug.writerMutexHolder
+**     asyncdebug.lockMutexHolder
+**
+** These variables are used by some assert() statements that verify
+** the statements made in the "Deadlock Prevention" notes earlier
+** in this file.
+*/
+#ifndef NDEBUG
+
+static struct TestAsyncDebugData {
+  pthread_t lockMutexHolder;
+  pthread_t queueMutexHolder;
+  pthread_t writerMutexHolder;
+} asyncdebug = {0, 0, 0};
+
+/*
+** Wrapper around pthread_mutex_lock(). Checks that we have not violated
+** the anti-deadlock rules (see "Deadlock prevention" above).
+*/
+static int async_mutex_lock(pthread_mutex_t *pMutex){
+  int iIdx;
+  int rc;
+  pthread_mutex_t *aMutex = (pthread_mutex_t *)(&async);
+  pthread_t *aHolder = (pthread_t *)(&asyncdebug);
+
+  /* The code in this 'ifndef NDEBUG' block depends on a certain alignment
+   * of the variables in TestAsyncStaticData and TestAsyncDebugData. The
+   * following assert() statements check that this has not been changed.
+   *
+   * Really, these only need to be run once at startup time.
+   */
+  assert(&(aMutex[0])==&async.lockMutex);
+  assert(&(aMutex[1])==&async.queueMutex);
+  assert(&(aMutex[2])==&async.writerMutex);
+  assert(&(aHolder[0])==&asyncdebug.lockMutexHolder);
+  assert(&(aHolder[1])==&asyncdebug.queueMutexHolder);
+  assert(&(aHolder[2])==&asyncdebug.writerMutexHolder);
+
+  assert( pthread_self()!=0 );
+
+  for(iIdx=0; iIdx<3; iIdx++){
+    if( pMutex==&aMutex[iIdx] ) break;
+
+    /* This is the key assert(). Here we are checking that if the caller
+     * is trying to block on async.writerMutex, neither of the other two
+     * mutex are held. If the caller is trying to block on async.queueMutex,
+     * lockMutex is not held.
+     */
+    assert(!pthread_equal(aHolder[iIdx], pthread_self()));
+  }
+  assert(iIdx<3);
+
+  rc = pthread_mutex_lock(pMutex);
+  if( rc==0 ){
+    assert(aHolder[iIdx]==0);
+    aHolder[iIdx] = pthread_self();
+  }
+  return rc;
+}
+
+/*
+** Wrapper around pthread_mutex_unlock().
+*/
+static int async_mutex_unlock(pthread_mutex_t *pMutex){
+  int iIdx;
+  int rc;
+  pthread_mutex_t *aMutex = (pthread_mutex_t *)(&async);
+  pthread_t *aHolder = (pthread_t *)(&asyncdebug);
+
+  for(iIdx=0; iIdx<3; iIdx++){
+    if( pMutex==&aMutex[iIdx] ) break;
+  }
+  assert(iIdx<3);
+
+  assert(pthread_equal(aHolder[iIdx], pthread_self()));
+  aHolder[iIdx] = 0;
+  rc = pthread_mutex_unlock(pMutex);
+  assert(rc==0);
+
+  return 0;
+}
+
+/*
+** Wrapper around pthread_mutex_trylock().
+*/
+static int async_mutex_trylock(pthread_mutex_t *pMutex){
+  int iIdx;
+  int rc;
+  pthread_mutex_t *aMutex = (pthread_mutex_t *)(&async);
+  pthread_t *aHolder = (pthread_t *)(&asyncdebug);
+
+  for(iIdx=0; iIdx<3; iIdx++){
+    if( pMutex==&aMutex[iIdx] ) break;
+  }
+  assert(iIdx<3);
+
+  rc = pthread_mutex_trylock(pMutex);
+  if( rc==0 ){
+    assert(aHolder[iIdx]==0);
+    aHolder[iIdx] = pthread_self();
+  }
+  return rc;
+}
+
+/*
+** Wrapper around pthread_cond_wait().
+*/
+static int async_cond_wait(pthread_cond_t *pCond, pthread_mutex_t *pMutex){
+  int iIdx;
+  int rc;
+  pthread_mutex_t *aMutex = (pthread_mutex_t *)(&async);
+  pthread_t *aHolder = (pthread_t *)(&asyncdebug);
+
+  for(iIdx=0; iIdx<3; iIdx++){
+    if( pMutex==&aMutex[iIdx] ) break;
+  }
+  assert(iIdx<3);
+
+  assert(pthread_equal(aHolder[iIdx],pthread_self()));
+  aHolder[iIdx] = 0;
+  rc = pthread_cond_wait(pCond, pMutex);
+  if( rc==0 ){
+    aHolder[iIdx] = pthread_self();
+  }
+  return rc;
+}
+
+/* Call our async_XX wrappers instead of selected pthread_XX functions */
+#define pthread_mutex_lock    async_mutex_lock
+#define pthread_mutex_unlock  async_mutex_unlock
+#define pthread_mutex_trylock async_mutex_trylock
+#define pthread_cond_wait     async_cond_wait
+
+#endif   /* !defined(NDEBUG) */
+
 /*
 ** Add an entry to the end of the global write-op list. pWrite should point 
 ** to an AsyncWrite structure allocated using sqlite3_malloc().  The writer
@@ -914,7 +1071,6 @@ static int asyncOpen(
     HashElem *pElem;
     p->pMethod = &async_methods;
     p->pData = pData;
-    incrOpenFileCount();
 
     /* Link AsyncFileData.lock into the linked list of 
     ** AsyncFileLock structures for this file.
@@ -932,6 +1088,10 @@ static int asyncOpen(
 
   pthread_mutex_unlock(&async.lockMutex);
 
+  if( rc==SQLITE_OK ){
+    incrOpenFileCount();
+  }
+
   if( rc==SQLITE_OK && isExclusive ){
     rc = addNewAsyncWrite(pData, ASYNC_OPENEXCLUSIVE, (i64)flags, 0, 0);
     if( rc==SQLITE_OK ){
@@ -1475,7 +1635,7 @@ static int testAsyncStart(
   pthread_t x;
   int rc;
   volatile int isStarted = 0;
-  rc = pthread_create(&x, 0, asyncWriterThread, &isStarted);
+  rc = pthread_create(&x, 0, asyncWriterThread, (void *)&isStarted);
   if( rc ){
     Tcl_AppendResult(interp, "failed to create the thread", 0);
     return TCL_ERROR;