Fix Tester1 so that exceptions triggered via threads are not silently ignored. Disabl...

FossilOrigin-Name: 56b2a077ace6e6ad5834e1a597b710f212a5b7d5db5b9a27a41f2aa0f6952c55

diff --git a/ext/jni/GNUmakefile b/ext/jni/GNUmakefile
index 788c07206218765c392b1f4a6f44ebe5720096ed..0cda6bf2e425457378e887df2a8bae494cc7bbab 100644 (file)
--- a/ext/jni/GNUmakefile
+++ b/ext/jni/GNUmakefile
@@ -166,8 +166,8 @@ SQLITE_OPT = \
   -DSQLITE_THREADSAFE=1 \
   -DSQLITE_TEMP_STORE=2 \
   -DSQLITE_USE_URI=1 \
-  -DSQLITE_C=$(sqlite3.c)
-#  -DSQLITE_DEBUG
+  -DSQLITE_C=$(sqlite3.c) \
+  -DSQLITE_DEBUG
 
 SQLITE_OPT += -g -DDEBUG -UNDEBUG
 

diff --git a/ext/jni/src/c/sqlite3-jni.c b/ext/jni/src/c/sqlite3-jni.c
index 2299a4c6c08d6f0fbc938118ae69eba79ef47929..f1bc049545658dc461f84ac6406b3c06e0072b27 100644 (file)
--- a/ext/jni/src/c/sqlite3-jni.c
+++ b/ext/jni/src/c/sqlite3-jni.c
@@ -450,6 +450,16 @@ struct S3JniAutoExtension {
   jmethodID midFunc  /* xEntryPoint() callback */;
 };
 
+/*
+** If true, modifying S3JniGlobal.metrics is protected by a mutex,
+** else it isn't.
+*/
+#ifdef SQLITE_DEBUG
+#define S3JNI_METRICS_MUTEX 1
+#else
+#define S3JNI_METRICS_MUTEX 0
+#endif
+
 /*
 ** Global state, e.g. caches and metrics.
 */
@@ -537,6 +547,10 @@ struct S3JniGlobalType {
       volatile unsigned nValue;
       volatile unsigned nInverse;
     } udf;
+    unsigned nMetrics                 /* Total number of mutex-locked metrics increments. */;
+#if S3JNI_METRICS_MUTEX
+    sqlite3_mutex * mutex;
+#endif
   } metrics;
   /**
      The list of bound auto-extensions (Java-side:
@@ -554,6 +568,18 @@ struct S3JniGlobalType {
 static S3JniGlobalType S3JniGlobal = {};
 #define SJG S3JniGlobal
 
+static void s3jni_incr( volatile unsigned int * const p ){
+#if S3JNI_METRICS_MUTEX
+  sqlite3_mutex * const m = SJG.metrics.mutex;
+  sqlite3_mutex_enter(m);
+  ++SJG.metrics.nMetrics;
+  ++(*p);
+  sqlite3_mutex_leave(m);
+#else
+  ++(*p);
+#endif
+}
+
 /* Helpers for working with specific mutexes. */
 #define MUTEX_ENV_ASSERT_LOCKED  \
   assert( 0 != SJG.envCache.locker && "Misuse of S3JniGlobal.envCache.mutex" )
@@ -629,12 +655,12 @@ static S3JniEnv * S3JniGlobal_env_cache(JNIEnv * const env){
   row = SJG.envCache.aHead;
   for( ; row; row = row->pNext ){
     if( row->env == env ){
-      ++SJG.metrics.envCacheHits;
+      s3jni_incr( &SJG.metrics.envCacheHits );
       MUTEX_ENV_LEAVE;
       return row;
     }
   }
-  ++SJG.metrics.envCacheMisses;
+  s3jni_incr( &SJG.metrics.envCacheMisses );
   row = SJG.envCache.aFree;
   if( row ){
     assert(!row->pPrev);
@@ -642,7 +668,7 @@ static S3JniEnv * S3JniGlobal_env_cache(JNIEnv * const env){
     if( row->pNext ) row->pNext->pPrev = 0;
   }else{
     row = s3jni_malloc(env, sizeof(S3JniEnv));
-    ++SJG.metrics.envCacheAllocs;
+    s3jni_incr( &SJG.metrics.envCacheAllocs );
   }
   memset(row, 0, sizeof(*row));
   row->pNext = SJG.envCache.aHead;
@@ -872,7 +898,7 @@ static void s3jni_call_xDestroy(JNIEnv * const env, jobject jObj, jclass klazz){
     }
     method = (*env)->GetMethodID(env, klazz, "xDestroy", "()V");
     if(method){
-      ++SJG.metrics.nDestroy;
+      s3jni_incr( &SJG.metrics.nDestroy );
       (*env)->CallVoidMethod(env, jObj, method);
       IFTHREW{
         EXCEPTION_WARN_CALLBACK_THREW("xDestroy() callback");
@@ -1083,13 +1109,13 @@ static S3JniDb * S3JniDb_alloc(JNIEnv * const env, sqlite3 *pDb,
       rv->pNext->pPrev = 0;
       rv->pNext = 0;
     }
-    ++SJG.metrics.nPdbRecycled;
+    s3jni_incr( &SJG.metrics.nPdbRecycled );
   }else{
     rv = s3jni_malloc(env, sizeof(S3JniDb));
     //MARKER(("state@%p for db allocating for db@%p from heap\n", rv, pDb));
     if(rv){
       memset(rv, 0, sizeof(S3JniDb));
-      ++SJG.metrics.nPdbAlloc;
+      s3jni_incr( &SJG.metrics.nPdbAlloc );
     }
   }
   if(rv){
@@ -1631,29 +1657,29 @@ static int udf_xFV(sqlite3_context* cx, S3JniUdf * s,
 static void udf_xFunc(sqlite3_context* cx, int argc,
                       sqlite3_value** argv){
   S3JniUdf * const s = (S3JniUdf*)sqlite3_user_data(cx);
-  ++SJG.metrics.udf.nFunc;
+  s3jni_incr( &SJG.metrics.udf.nFunc );
   udf_xFSI(cx, argc, argv, s, s->jmidxFunc, "xFunc");
 }
 static void udf_xStep(sqlite3_context* cx, int argc,
                       sqlite3_value** argv){
   S3JniUdf * const s = (S3JniUdf*)sqlite3_user_data(cx);
-  ++SJG.metrics.udf.nStep;
+  s3jni_incr( &SJG.metrics.udf.nStep );
   udf_xFSI(cx, argc, argv, s, s->jmidxStep, "xStep");
 }
 static void udf_xFinal(sqlite3_context* cx){
   S3JniUdf * const s = (S3JniUdf*)sqlite3_user_data(cx);
-  ++SJG.metrics.udf.nFinal;
+  s3jni_incr( &SJG.metrics.udf.nFinal );
   udf_xFV(cx, s, s->jmidxFinal, "xFinal");
 }
 static void udf_xValue(sqlite3_context* cx){
   S3JniUdf * const s = (S3JniUdf*)sqlite3_user_data(cx);
-  ++SJG.metrics.udf.nValue;
+  s3jni_incr( &SJG.metrics.udf.nValue );
   udf_xFV(cx, s, s->jmidxValue, "xValue");
 }
 static void udf_xInverse(sqlite3_context* cx, int argc,
                          sqlite3_value** argv){
   S3JniUdf * const s = (S3JniUdf*)sqlite3_user_data(cx);
-  ++SJG.metrics.udf.nInverse;
+  s3jni_incr( &SJG.metrics.udf.nInverse );
   udf_xFSI(cx, argc, argv, s, s->jmidxInverse, "xInverse");
 }
 
@@ -3301,8 +3327,7 @@ JDECL(jbyteArray,1value_1text16be)(JENV_CSELF, jobject jpSVal){
 }
 
 JDECL(void,1do_1something_1for_1developer)(JENV_CSELF){
-  MARKER(("\nVarious bits of internal info:\n"
-          "Any metrics here are invalid in multi-thread use.\n"));
+  MARKER(("\nVarious bits of internal info:\n"));
   puts("FTS5 is "
 #ifdef SQLITE_ENABLE_FTS5
        "available"
@@ -3334,9 +3359,11 @@ JDECL(void,1do_1something_1for_1developer)(JENV_CSELF){
          "\n\tenv %u"
          "\n\tnph inits %u"
          "\n\tperDb %u"
-         "\n\tautoExt %u list accesses\n",
+         "\n\tautoExt %u list accesses"
+         "\n\tmetrics %u\n",
          SJG.metrics.nMutexEnv, SJG.metrics.nMutexEnv2,
-         SJG.metrics.nMutexPerDb, SJG.metrics.nMutexAutoExt);
+         SJG.metrics.nMutexPerDb, SJG.metrics.nMutexAutoExt,
+         SJG.metrics.nMetrics);
   printf("S3JniDb: %u alloced (*%u = %u bytes), %u recycled\n",
          SJG.metrics.nPdbAlloc, (unsigned) sizeof(S3JniDb),
          (unsigned)(SJG.metrics.nPdbAlloc * sizeof(S3JniDb)),
@@ -4295,6 +4322,11 @@ Java_org_sqlite_jni_SQLite3Jni_init(JENV_CSELF){
   SJG.autoExt.mutex = sqlite3_mutex_alloc(SQLITE_MUTEX_FAST);
   OOM_CHECK( SJG.autoExt.mutex );
 
+#if S3JNI_METRICS_MUTEX
+  SJG.metrics.mutex = sqlite3_mutex_alloc(SQLITE_MUTEX_FAST);
+  OOM_CHECK( SJG.metrics.mutex );
+#endif
+
 #if 0
   /* Just for sanity checking... */
   (void)S3JniGlobal_env_cache(env);

diff --git a/ext/jni/src/org/sqlite/jni/Tester1.java b/ext/jni/src/org/sqlite/jni/Tester1.java
index 951ded16aa2ce862a2316003b7a377a4090b34c8..602134f155078446487db202db0aaa904ce69459 100644 (file)
--- a/ext/jni/src/org/sqlite/jni/Tester1.java
+++ b/ext/jni/src/org/sqlite/jni/Tester1.java
@@ -28,6 +28,7 @@ public class Tester1 implements Runnable {
   private static boolean shuffle = false;
   private static boolean listRunTests = false;
   private static List<java.lang.reflect.Method> testMethods = null;
+  private static List<Exception> listErrors = new ArrayList<>();
   private static final class Metrics {
     int dbOpen;
   }
@@ -67,7 +68,7 @@ public class Tester1 implements Runnable {
   static volatile int affirmCount = 0;
   public synchronized static void affirm(Boolean v, String comment){
     ++affirmCount;
-    assert( v /* prefer assert over exception if it's enabled because
+    if( false ) assert( v /* prefer assert over exception if it's enabled because
                  the JNI layer sometimes has to suppress exceptions,
                  so they might be squelched on their way back to the
                  top. */);
@@ -1106,7 +1107,7 @@ public class Tester1 implements Runnable {
     sqlite3 db = createNewDb();
     affirm( 4==val.value );
     execSql(db, "ATTACH ':memory' as foo");
-    affirm( 4==val.value /* ATTACH uses the same connection, not sub-connections. */ );
+    affirm( 4==val.value, "ATTACH uses the same connection, not sub-connections." );
     sqlite3_close(db);
     db = null;
 
@@ -1176,55 +1177,62 @@ public class Tester1 implements Runnable {
     }
   }
 
+  private void testFail(){
+    affirm( false, "Intentional failure." );
+  }
+
   private void runTests(boolean fromThread) throws Exception {
     if(false) showCompileOption();
     List<java.lang.reflect.Method> mlist = testMethods;
     affirm( null!=mlist );
     if( shuffle ){
       mlist = new ArrayList<>( testMethods.subList(0, testMethods.size()) );
-      java.util.Collections.shuffle(
-        mlist
-        //java.util.concurrent.ThreadLocalRandom.current()
-      );
+      java.util.Collections.shuffle(mlist);
     }
     if( listRunTests ){
       synchronized(this.getClass()){
-        out("Initial test"," list: ");
-        for(java.lang.reflect.Method m : testMethods){
-          out(m.getName()+" ");
+        if( !fromThread ){
+          out("Initial test"," list: ");
+          for(java.lang.reflect.Method m : testMethods){
+            out(m.getName()+" ");
+          }
+          outln();
+          outln("(That list excludes some which are hard-coded to run.)");
         }
-        outln();
-
         out("Running"," tests: ");
         for(java.lang.reflect.Method m : mlist){
           out(m.getName()+" ");
         }
         outln();
-        out("(That list excludes some which are hard-coded to run.)\n");
       }
     }
     testToUtf8();
     test1();
-    int n = 0;
     for(java.lang.reflect.Method m : mlist){
-      ++n;
       nap();
-      m.invoke(this);
+      try{
+        m.invoke(this);
+      }catch(java.lang.reflect.InvocationTargetException e){
+        outln("FAILURE: ",m.getName(),"(): ", e.getCause());
+        throw e;
+      }
     }
-    affirm( n == mlist.size() );
-    if(!fromThread){
+    if( !fromThread ){
       testBusy();
       if( !mtMode ){
+        testAutoExtension() /* threads rightfully muck up these results */;
         testFts5();
       }
     }
   }
 
-  public void run(){
+  public void run() {
     try {
       runTests(0!=this.tId);
     }catch(Exception e){
-      throw new RuntimeException(e);
+      synchronized( listErrors ){
+        listErrors.add(e);
+      }
     }finally{
       affirm( SQLite3Jni.uncacheJniEnv() );
       affirm( !SQLite3Jni.uncacheJniEnv() );
@@ -1256,6 +1264,7 @@ public class Tester1 implements Runnable {
     Integer nThread = null;
     boolean doSomethingForDev = false;
     Integer nRepeat = 1;
+    boolean forceFail = false;
     for( int i = 0; i < args.length; ){
       String arg = args[i++];
       if(arg.startsWith("-")){
@@ -1269,8 +1278,13 @@ public class Tester1 implements Runnable {
           nRepeat = Integer.parseInt(args[i++]);
         }else if(arg.equals("shuffle")){
           shuffle = true;
+          outln("WARNING: -shuffle mode is known to run ",
+                "the same number of tests but provide far ",
+                "lower, unpredictable metrics for unknown reasons.");
         }else if(arg.equals("list-tests")){
           listRunTests = true;
+        }else if(arg.equals("fail")){
+          forceFail = true;
         }else if(arg.equals("naps")){
           takeNaps = true;
         }else{
@@ -1284,11 +1298,13 @@ public class Tester1 implements Runnable {
       testMethods = new ArrayList<>();
       final List<String> excludes = new ArrayList<>();
       // Tests we want to control the order of:
-      excludes.add("testSleep");
-      excludes.add("testToUtf8");
+      if( !forceFail ) excludes.add("testFail");
       excludes.add("test1");
+      excludes.add("testAutoExtension");
       excludes.add("testBusy");
       excludes.add("testFts5");
+      excludes.add("testSleep");
+      excludes.add("testToUtf8");
       for(java.lang.reflect.Method m : Tester1.class.getDeclaredMethods()){
         final String name = m.getName();
         if( name.startsWith("test") && excludes.indexOf(name)<0 ){
@@ -1306,23 +1322,32 @@ public class Tester1 implements Runnable {
     for( int n = 0; n < nRepeat; ++n ){
       if( nThread==null || nThread<=1 ){
         new Tester1(0).runTests(false);
-      }else{
-        Tester1.mtMode = true;
-        final ExecutorService ex = Executors.newFixedThreadPool( nThread );
-        //final List<Future<?>> futures = new ArrayList<>();
-        ++nLoop;
-        out((1==nLoop ? "" : " ")+nLoop);
-        for( int i = 0; i < nThread; ++i ){
-          ex.submit( new Tester1(i), i );
-        }
-        ex.shutdown();
-        try {
-          ex.awaitTermination(nThread*200, java.util.concurrent.TimeUnit.MILLISECONDS);
-          ex.shutdownNow();
-        } catch (InterruptedException ie) {
-          ex.shutdownNow();
-          Thread.currentThread().interrupt();
+        continue;
+      }
+      Tester1.mtMode = true;
+      final ExecutorService ex = Executors.newFixedThreadPool( nThread );
+      //final List<Future<?>> futures = new ArrayList<>();
+      ++nLoop;
+      out((1==nLoop ? "" : " ")+nLoop);
+      for( int i = 0; i < nThread; ++i ){
+        ex.submit( new Tester1(i), i );
+      }
+      ex.shutdown();
+      try{
+        ex.awaitTermination(nThread*200, java.util.concurrent.TimeUnit.MILLISECONDS);
+        ex.shutdownNow();
+      }catch (InterruptedException ie){
+        ex.shutdownNow();
+        Thread.currentThread().interrupt();
+      }
+      if( !listErrors.isEmpty() ){
+        outln("TEST ERRORS:");
+        Exception err = null;
+        for( Exception e : listErrors ){
+          e.printStackTrace();
+          if( null==err ) err = e;
         }
+        if( null!=err ) throw err;
       }
     }
     outln();

diff --git a/manifest b/manifest
index eb006ab13a559c7d4cb7483e9dbd02e9e3048264..dbd6267c347c28389ae22a893046689beb8c2f98 100644 (file)
--- a/manifest
+++ b/manifest
@@ -1,5 +1,5 @@
-C More\swork\son\sthe\sJNI\smulti-threaded\stest\srunner.
-D 2023-08-22T20:10:28.237
+C Fix\sTester1\sso\sthat\sexceptions\striggered\svia\sthreads\sare\snot\ssilently\signored.\sDisable\sauto-extension\stests\sin\smulti-thread\smode\sbecause\sconcurrent\sthreads\srightfully\sinterfere\swith\sthat.
+D 2023-08-22T22:13:08.053
 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1
 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea
 F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724
@@ -232,10 +232,10 @@ F ext/fts5/tool/showfts5.tcl d54da0e067306663e2d5d523965ca487698e722c
 F ext/icu/README.txt 7ab7ced8ae78e3a645b57e78570ff589d4c672b71370f5aa9e1cd7024f400fc9
 F ext/icu/icu.c c074519b46baa484bb5396c7e01e051034da8884bad1a1cb7f09bbe6be3f0282
 F ext/icu/sqliteicu.h fa373836ed5a1ee7478bdf8a1650689294e41d0c89c1daab26e9ae78a32075a8
-F ext/jni/GNUmakefile 1ccd09095447709ffd7a4f32514fd586512491c6bed06d009bab4294b451ed62
+F ext/jni/GNUmakefile 95dfae98709a8ffd9bb2087fc53239b37a22cad6d7495db77f46ae56835623bc
 F ext/jni/README.md 975b35173debbbf3a4ab7166e14d2ffa2bacff9b6850414f09cc919805e81ba4
 F ext/jni/jar-dist.make 9a03d10dbb5a74c724bfec4b76fd9e4c9865cbbc858d731cb48f38ac897d73a3
-F ext/jni/src/c/sqlite3-jni.c c38c18875b946a3bdc4eda0b2f19ad53b895118979ec85a630706c1c5575079b
+F ext/jni/src/c/sqlite3-jni.c 1064441d33cef541c9c9c84fb5093573a0767bb36563e6ea538c355b6148c4c6
 F ext/jni/src/c/sqlite3-jni.h 8b0ab1a3f0f92b75d4ff50db4a88b66a137cfb561268eb15bb3993ed174dbb74
 F ext/jni/src/org/sqlite/jni/Authorizer.java 1308988f7f40579ea0e4deeaec3c6be971630566bd021c31367fe3f5140db892
 F ext/jni/src/org/sqlite/jni/AutoExtension.java 3b62c915e45ce73f63343ca9195ec63592244d616a1908b7587bdd45de1b97dd
@@ -256,7 +256,7 @@ F ext/jni/src/org/sqlite/jni/ResultCode.java ba701f20213a5f259e94cfbfdd36eb7ac7c
 F ext/jni/src/org/sqlite/jni/RollbackHook.java b04c8abcc6ade44a8a57129e33765793f69df0ba909e49ba18d73f4268d92564
 F ext/jni/src/org/sqlite/jni/SQLFunction.java 8c1ad92c35bcc1b2f7256cf6e229b31340ed6d1a404d487f0a9adb28ba7fc332
 F ext/jni/src/org/sqlite/jni/SQLite3Jni.java 2f36370cfdec01d309720392b2c3e4af6afce0b6ece8188b5c3ed688a5a1e63a
-F ext/jni/src/org/sqlite/jni/Tester1.java da8bc65f52d310ae17b372eeaef25726be47d3a2052e8a33ce44606a7dc451d7
+F ext/jni/src/org/sqlite/jni/Tester1.java 9e44d27226eea7486c32eaea6789e8505422c9202f328ff0c1473b75f4ebeeb8
 F ext/jni/src/org/sqlite/jni/TesterFts5.java c729d5b3cb91888b7e2a3a3ef450852f184697df78721574f6c0bf9043e4b84c
 F ext/jni/src/org/sqlite/jni/Tracer.java a5cece9f947b0af27669b8baec300b6dd7ff859c3e6a6e4a1bd8b50f9714775d
 F ext/jni/src/org/sqlite/jni/UpdateHook.java e58645a1727f8a9bbe72dc072ec5b40d9f9362cb0aa24acfe93f49ff56a9016d
@@ -2092,8 +2092,8 @@ F vsixtest/vsixtest.tcl 6a9a6ab600c25a91a7acc6293828957a386a8a93
 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc
 F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e
 F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0
-P 8b78b737e66a399b04e555a8197f63a73198a4105cb2f37ffd5b0e6014302caf
-R 31a03bd5ee6de6309ec09a8781691d85
+P 9a74ad716bded1e14333bf7c72392916f800d58a96240eabe4988ca5fc9e8752
+R 1b5653fffe70ebb6615cb201c18f879a
 U stephan
-Z 973ac484c97d56f1bb0793972fc0262a
+Z a4cd6dac629b552f49728a69325b689c
 # Remove this line to create a well-formed Fossil manifest.

diff --git a/manifest.uuid b/manifest.uuid
index 0f657227751e9c1c4f746d4d23e126bf4f8e20eb..ccf7dfc8fbcb56b056c50a261b69dcd6f0748eae 100644 (file)
--- a/manifest.uuid
+++ b/manifest.uuid
@@ -1 +1 @@
-9a74ad716bded1e14333bf7c72392916f800d58a96240eabe4988ca5fc9e8752
\ No newline at end of file
+56b2a077ace6e6ad5834e1a597b710f212a5b7d5db5b9a27a41f2aa0f6952c55
\ No newline at end of file