From 87bb103038cc4eee800f9f75f9bcb616a752ce8d Mon Sep 17 00:00:00 2001 From: stephan Date: Tue, 22 Aug 2023 18:36:30 +0000 Subject: [PATCH] Disassociate JNI db handles from the thread that created them, as it's no longer relevant. FossilOrigin-Name: 8b78b737e66a399b04e555a8197f63a73198a4105cb2f37ffd5b0e6014302caf --- ext/jni/src/c/sqlite3-jni.c | 39 ++------------- ext/jni/src/org/sqlite/jni/SQLite3Jni.java | 22 +++------ ext/jni/src/org/sqlite/jni/Tester1.java | 55 +++++++++++++--------- manifest | 16 +++---- manifest.uuid | 2 +- 5 files changed, 53 insertions(+), 81 deletions(-) diff --git a/ext/jni/src/c/sqlite3-jni.c b/ext/jni/src/c/sqlite3-jni.c index fc96bdd384..47269824b5 100644 --- a/ext/jni/src/c/sqlite3-jni.c +++ b/ext/jni/src/c/sqlite3-jni.c @@ -380,10 +380,6 @@ struct S3JniHook{ */ typedef struct S3JniDb S3JniDb; struct S3JniDb { - JNIEnv *env /* Used for cleaning up all dbs owned by a given - ** thread, noting that this ownership is an artificial - ** one imposed by our threading constraints, not by - ** the core library. */; sqlite3 *pDb /* The associated db handle */; jobject jDb /* A global ref of the output object which gets returned from sqlite3_open(_v2)(). We need this in @@ -558,6 +554,9 @@ struct S3JniGlobalType { static S3JniGlobalType S3JniGlobal = {}; #define SJG S3JniGlobal +/* Helpers for working with specific mutexes. */ +#define MUTEX_ENV_ASSERT_LOCKED \ + assert( 0 != SJG.envCache.locker && "Misuse of S3JniGlobal.envCache.mutex" ) #define MUTEX_ENV_ASSERT_LOCKER \ assert( (env) == SJG.envCache.locker && "Misuse of S3JniGlobal.envCache.mutex" ) #define MUTEX_ENV_ASSERT_NOTLOCKER \ @@ -604,7 +603,7 @@ static S3JniGlobalType S3JniGlobal = {}; assert( 0 != SJG.perDb.locker && "Misuse of S3JniGlobal.perDb.mutex" ) #define OOM_CHECK(VAR) if(!(VAR)) s3jni_oom(env) -static void s3jni_oom(JNIEnv * const env){ +static inline void s3jni_oom(JNIEnv * const env){ (*env)->FatalError(env, "Out of memory.") /* does not return */; } @@ -941,41 +940,15 @@ static void S3JniDb_set_aside(S3JniDb * const s){ //if(s->pNext) MARKER(("next: %p->pPrev@%p\n", s->pNext, s->pNext->pPrev)); } } -/** - Cleans up all state in S3JniGlobal.perDb for th given JNIEnv. - Results are undefined if a Java-side db uses the API - from the given JNIEnv after this call. -*/ -static void S3JniDb_free_for_env(JNIEnv *env){ - S3JniDb * ps; - S3JniDb * pNext = 0; - MUTEX_PDB_ENTER; - ps = SJG.perDb.aUsed; - for( ; ps; ps = pNext ){ - pNext = ps->pNext; - if(ps->env == env){ -#ifndef NDEBUG - S3JniDb * const pPrev = ps->pPrev; -#endif - S3JniDb_set_aside(ps); - assert( pPrev ? pPrev->pNext==pNext : 1 ); - assert( ps == SJG.perDb.aFree ); - } - } - MUTEX_PDB_LEAVE; -} /** Uncache any state for the given JNIEnv, clearing all Java references the cache owns. Returns true if env was cached and false if it was not found in the cache. - - Also passes env to S3JniDb_free_for_env() to free up - what would otherwise be stale references. */ static int S3JniGlobal_env_uncache(JNIEnv * const env){ struct S3JniEnv * row; - MUTEX_ENV_ASSERT_LOCKER; + MUTEX_ENV_ASSERT_LOCKED; row = SJG.envCache.aHead; for( ; row; row = row->pNext ){ if( row->env == env ){ @@ -992,7 +965,6 @@ static int S3JniGlobal_env_uncache(JNIEnv * const env){ assert( !row->pPrev ); SJG.envCache.aHead = row->pNext; } - S3JniDb_free_for_env(env); memset(row, 0, sizeof(S3JniEnv)); row->pNext = SJG.envCache.aFree; if( row->pNext ) row->pNext->pPrev = row; @@ -1130,7 +1102,6 @@ static S3JniDb * S3JniDb_alloc(JNIEnv * const env, sqlite3 *pDb, } rv->jDb = REF_G(jDb); rv->pDb = pDb; - rv->env = env; } MUTEX_PDB_LEAVE; return rv; diff --git a/ext/jni/src/org/sqlite/jni/SQLite3Jni.java b/ext/jni/src/org/sqlite/jni/SQLite3Jni.java index cfca4fc77c..22178faed6 100644 --- a/ext/jni/src/org/sqlite/jni/SQLite3Jni.java +++ b/ext/jni/src/org/sqlite/jni/SQLite3Jni.java @@ -122,21 +122,13 @@ public final class SQLite3Jni { uncacheJniEnv() when it is done with the library - either right before it terminates or when it is finished using the SQLite API. This will clean up any cached per-JNIEnv info. Calling into the - library again after that "should" re-initialize the cache on - demand, but that's untested. - - This call forcibly wipes out all cached information for the - current JNIEnv, a side-effect of which is that behavior is - undefined if any database objects are (A) still active at the - time it is called _and_ (B) calls are subsequently made into the - library with such a database. Doing so will, at best, lead to a - crash. At worst, it will lead to the db possibly misbehaving - because some of its Java-bound state has been cleared. There is - no immediate harm in (A) so long as condition (B) is not met. - This process does _not_ actually close any databases or finalize - any prepared statements. For proper library behavior, and to - avoid C-side leaks, be sure to close them before calling this - function. + library will re-initialize the cache on demand. + + This process does _not_ close any databases or finalize + any prepared statements because their ownership does not depend on + a given thread. For proper library behavior, and to + avoid C-side leaks, be sure to finalize all statements and close + all databases before calling this function. Calling this from the main application thread is not strictly required but is "polite." Additional threads must call this diff --git a/ext/jni/src/org/sqlite/jni/Tester1.java b/ext/jni/src/org/sqlite/jni/Tester1.java index 9677ca57d1..e010e24703 100644 --- a/ext/jni/src/org/sqlite/jni/Tester1.java +++ b/ext/jni/src/org/sqlite/jni/Tester1.java @@ -24,6 +24,7 @@ import java.util.concurrent.Future; public class Tester1 implements Runnable { //! True when running in multi-threaded mode. private static boolean mtMode = false; + private static boolean takeNaps = false; private static final class Metrics { int dbOpen; @@ -1167,32 +1168,38 @@ public class Tester1 implements Runnable { outln("Woke up."); } + private void nap() throws InterruptedException { + if( takeNaps ){ + Thread.sleep(java.util.concurrent.ThreadLocalRandom.current().nextInt(3, 28), 0); + } + } + private void runTests(boolean fromThread) throws Exception { if(false) testCompileOption(); testToUtf8(); test1(); - testOpenDb1(); - testOpenDb2(); - testCollation(); - testPrepare123(); - testBindFetchInt(); - testBindFetchInt64(); - testBindFetchDouble(); - testBindFetchText(); - testBindFetchBlob(); - testSql(); - testStatus(); - testUdf1(); - testUdfJavaObject(); - testUdfAggregate(); - testUdfWindow(); - testTrace(); - testProgress(); - testCommitHook(); - testRollbackHook(); - testUpdateHook(); - testAuthorizer(); - testAutoExtension(); + nap(); testOpenDb1(); + nap(); testOpenDb2(); + nap(); testCollation(); + nap(); testPrepare123(); + nap(); testBindFetchInt(); + nap(); testBindFetchInt64(); + nap(); testBindFetchDouble(); + nap(); testBindFetchText(); + nap(); testBindFetchBlob(); + nap(); testSql(); + nap(); testStatus(); + nap(); testUdf1(); + nap(); testUdfJavaObject(); + nap(); testUdfAggregate(); + nap(); testUdfWindow(); + nap(); testTrace(); + nap(); testProgress(); + nap(); testCommitHook(); + nap(); testRollbackHook(); + nap(); testUpdateHook(); + nap(); testAuthorizer(); + nap(); testAutoExtension(); if(!fromThread){ testBusy(); if( !mtMode ){ @@ -1227,6 +1234,8 @@ public class Tester1 implements Runnable { nThread = Integer.parseInt(args[i++]); }else if(arg.equals("r") || arg.equals("runs")){ nRepeat = Integer.parseInt(args[i++]); + }else if(arg.equals("naps")){ + takeNaps = true; }else{ throw new IllegalArgumentException("Unhandled flag:"+arg); } @@ -1247,7 +1256,7 @@ public class Tester1 implements Runnable { final ExecutorService ex = Executors.newFixedThreadPool( nThread ); //final List> futures = new ArrayList<>(); ++nLoop; - out(nLoop+" "); + out((1==nLoop ? "" : " ")+nLoop); for( int i = 0; i < nThread; ++i ){ ex.submit( new Tester1(i), i ); } diff --git a/manifest b/manifest index 39fe198d94..71e4b52cfe 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Correct\sJNI\sbinding\sof\ssqlite3_shutdown()\sto\sclean\sup\sall\scached\sJNIEnv\sobjects. -D 2023-08-22T17:51:57.423 +C Disassociate\sJNI\sdb\shandles\sfrom\sthe\sthread\sthat\screated\sthem,\sas\sit's\sno\slonger\srelevant. +D 2023-08-22T18:36:30.981 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724 @@ -235,7 +235,7 @@ F ext/icu/sqliteicu.h fa373836ed5a1ee7478bdf8a1650689294e41d0c89c1daab26e9ae78a3 F ext/jni/GNUmakefile 1ccd09095447709ffd7a4f32514fd586512491c6bed06d009bab4294b451ed62 F ext/jni/README.md 975b35173debbbf3a4ab7166e14d2ffa2bacff9b6850414f09cc919805e81ba4 F ext/jni/jar-dist.make 9a03d10dbb5a74c724bfec4b76fd9e4c9865cbbc858d731cb48f38ac897d73a3 -F ext/jni/src/c/sqlite3-jni.c fb2ca8b6f846632b3432096d5533bef8251965dd5ed9f490441293da641a3fb5 +F ext/jni/src/c/sqlite3-jni.c 50edc462e8fdf54f9b8ede692a7c865c5e4315930899276664dd6744764d4723 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 @@ -255,8 +255,8 @@ F ext/jni/src/org/sqlite/jni/ProgressHandler.java 6f62053a828a572de809828b1ee495 F ext/jni/src/org/sqlite/jni/ResultCode.java ba701f20213a5f259e94cfbfdd36eb7ac7ce7797f2c6c7fca2004ff12ce20f86 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 0eea21f1015704e495b1a47aa9c7c90d081f51777981fe3f07760486aed092d8 -F ext/jni/src/org/sqlite/jni/Tester1.java e83a5635878cf73b463014b5137ce5c057ee9c5f6d67fbb40496894552785f46 +F ext/jni/src/org/sqlite/jni/SQLite3Jni.java 2f36370cfdec01d309720392b2c3e4af6afce0b6ece8188b5c3ed688a5a1e63a +F ext/jni/src/org/sqlite/jni/Tester1.java 58a058f718215ff32fbdf8026a2d4eb88f9d7e939a5640d5a944efafdfda4b7c 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 f927a30b5bba35991f472084ebaf02779e84c343a4e84f0efb3df7679ff212f8 -R fd18badbeb7e1f743addadd7405d6449 +P 02e868690f97ca728b0f2dd018aa79a9d13c85dd85b164caa895d319ae8f3ff5 +R 4fc9fbe1829e4bd8b0e17e9933291453 U stephan -Z 7d01e74d30abc2fec1f43939a768f2c1 +Z 9fda786e7e193bbf92304385fbc27c96 # Remove this line to create a well-formed Fossil manifest. diff --git a/manifest.uuid b/manifest.uuid index 3da3391a9e..cfa2599e0d 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -02e868690f97ca728b0f2dd018aa79a9d13c85dd85b164caa895d319ae8f3ff5 \ No newline at end of file +8b78b737e66a399b04e555a8197f63a73198a4105cb2f37ffd5b0e6014302caf \ No newline at end of file -- 2.47.2