From: stephan Date: Tue, 22 Aug 2023 22:13:08 +0000 (+0000) Subject: Fix Tester1 so that exceptions triggered via threads are not silently ignored. Disabl... X-Git-Tag: version-3.44.0~305^2~10 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=3600976bf1b4224858e16ae513049abb80325067;p=thirdparty%2Fsqlite.git Fix Tester1 so that exceptions triggered via threads are not silently ignored. Disable auto-extension tests in multi-thread mode because concurrent threads rightfully interfere with that. FossilOrigin-Name: 56b2a077ace6e6ad5834e1a597b710f212a5b7d5db5b9a27a41f2aa0f6952c55 --- diff --git a/ext/jni/GNUmakefile b/ext/jni/GNUmakefile index 788c072062..0cda6bf2e4 100644 --- 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 2299a4c6c0..f1bc049545 100644 --- 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 951ded16aa..602134f155 100644 --- 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 testMethods = null; + private static List 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 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 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> 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> 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 eb006ab13a..dbd6267c34 100644 --- 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 0f65722775..ccf7dfc8fb 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -9a74ad716bded1e14333bf7c72392916f800d58a96240eabe4988ca5fc9e8752 \ No newline at end of file +56b2a077ace6e6ad5834e1a597b710f212a5b7d5db5b9a27a41f2aa0f6952c55 \ No newline at end of file