Add SQLite3Jni.uncacheJniEnv(), a way for Java threads to clear their thread-specific...

FossilOrigin-Name: 7468f8761bece58f7ced3d112bbe2fb454432d9c54c9b96cedb5a15bc2926d0f

diff --git a/ext/jni/src/c/sqlite3-jni.c b/ext/jni/src/c/sqlite3-jni.c
index 67682e130f1e175bd19cab80e05c12a5c192d041..0c9862d4029b97f722cb69261466205f6069eec1 100644 (file)
--- a/ext/jni/src/c/sqlite3-jni.c
+++ b/ext/jni/src/c/sqlite3-jni.c
@@ -286,7 +286,7 @@ static const struct {
 #define JSTR_TOC(ARG) (*env)->GetStringUTFChars(env, ARG, NULL)
 #define JSTR_RELEASE(ARG,VAR) if(VAR) (*env)->ReleaseStringUTFChars(env, ARG, VAR)
 #define JBA_TOC(ARG) (*env)->GetByteArrayElements(env,ARG, NULL)
-#define JBA_RELEASE(ARG,VAR) (*env)->ReleaseByteArrayElements(env, ARG, VAR, JNI_ABORT)
+#define JBA_RELEASE(ARG,VAR) if(ARG) (*env)->ReleaseByteArrayElements(env, ARG, VAR, JNI_ABORT)
 
 /* Marker for code which needs(?) to be made thread-safe. */
 #define FIXME_THREADING
@@ -336,6 +336,14 @@ struct NphCacheLine {
 
 /**
    Cache for per-JNIEnv data.
+
+   Potential TODO: move the jclass entries to global space because,
+   per https://developer.android.com/training/articles/perf-jni:
+
+   > once you have a valid jclass global reference you can use it from
+     any attached thread.
+
+   Whereas we cache new refs for each thread.
 */
 typedef struct JNIEnvCacheLine JNIEnvCacheLine;
 struct JNIEnvCacheLine {
@@ -863,7 +871,8 @@ static void PerDbStateJni_dump(PerDbStateJni *s){
    return a (sqlite3*) but do not take one as an argument.
 */
 FIXME_THREADING
-static PerDbStateJni * PerDbStateJni_for_db(JNIEnv * const env, jobject jDb, sqlite3 *pDb, int allocIfNeeded){
+static PerDbStateJni * PerDbStateJni_for_db(JNIEnv * const env, jobject jDb,
+                                            sqlite3 *pDb, int allocIfNeeded){
   PerDbStateJni * s = S3Global.perDb.aUsed;
   if(!jDb){
     if(pDb){
@@ -3310,16 +3319,52 @@ JDECLFtsXA(jobject,xUserData)(JENV_JSELF,jobject jFcx){
 // End of the main API bindings. What follows are internal utilities.
 ////////////////////////////////////////////////////////////////////////
 
+/**
+   Uncaches the current JNIEnv from the S3Global state, clearing any
+   resources owned by that cache entry and making that slot available
+   for re-use. It is important that the Java-side decl of this
+   function be declared as synchronous.
+*/
+JNIEXPORT jboolean JNICALL
+Java_org_sqlite_jni_SQLite3Jni_uncacheJniEnv(JNIEnv * const env, jclass self){
+  struct JNIEnvCacheLine * row = 0;
+  int i;
+  for( i = 0; i < JNIEnvCache_SIZE; ++i ){
+    row = &S3Global.envCache.lines[i];
+    if(row->env == env){
+      break;
+    }
+  }
+  if( i==JNIEnvCache_SIZE ){
+    //MARKER(("The given JNIEnv is not currently cached.\n"));
+    return JNI_FALSE;
+  }
+  //MARKER(("Uncaching S3Global.envCache entry #%d.\n", i));
+  assert(S3Global.envCache.used >= i);
+  JNIEnvCacheLine_clear(row);
+  /**
+     Move all entries down one slot. memmove() would be faster.  We'll
+     eventually turn this cache into a dynamically-allocated linked
+     list, anyway, so this part will go away.
+  */
+  for( ++i ; i < JNIEnvCache_SIZE; ++i ){
+    S3Global.envCache.lines[i-i] = S3Global.envCache.lines[i];
+  }
+  memset(&S3Global.envCache.lines[i], 0, sizeof(JNIEnvCacheLine));
+  --S3Global.envCache.used;
+  return JNI_TRUE;
+}
+
 
 /**
    Called during static init of the SQLite3Jni class to sync certain
    compile-time constants to Java-space.
 
-   This routine is why we have to #include sqlite3.c instead of
-   sqlite3.h.
+   This routine is part of the reason why we have to #include
+   sqlite3.c instead of sqlite3.h.
 */
 JNIEXPORT void JNICALL
-Java_org_sqlite_jni_SQLite3Jni_init(JNIEnv * const env, jclass self, jobject sJni){
+Java_org_sqlite_jni_SQLite3Jni_init(JNIEnv * const env, jclass self, jclass klazzSjni){
   enum JType {
     JTYPE_INT,
     JTYPE_BOOL
@@ -3365,7 +3410,7 @@ Java_org_sqlite_jni_SQLite3Jni_init(JNIEnv * const env, jclass self, jobject sJn
     {0,0}
   };
   jfieldID fieldId;
-  jclass const klazz = (*env)->GetObjectClass(env, sJni);
+  //jclass const klazz = (*env)->GetObjectClass(env, sJni);
   const ConfigFlagEntry * pConfFlag;
   memset(&S3Global, 0, sizeof(S3Global));
   (void)S3Global_env_cache(env);
@@ -3378,16 +3423,16 @@ Java_org_sqlite_jni_SQLite3Jni_init(JNIEnv * const env, jclass self, jobject sJn
 
   for( pConfFlag = &aLimits[0]; pConfFlag->zName; ++pConfFlag ){
     char const * zSig = (JTYPE_BOOL == pConfFlag->jtype) ? "Z" : "I";
-    fieldId = (*env)->GetStaticFieldID(env, klazz, pConfFlag->zName, zSig);
+    fieldId = (*env)->GetStaticFieldID(env, klazzSjni, pConfFlag->zName, zSig);
     EXCEPTION_IS_FATAL("Missing an expected static member of the SQLite3Jni class.");
     //MARKER(("Setting %s (field=%p) = %d\n", pConfFlag->zName, fieldId, pConfFlag->value));
     assert(fieldId);
     switch(pConfFlag->jtype){
       case JTYPE_INT:
-        (*env)->SetStaticIntField(env, klazz, fieldId, (jint)pConfFlag->value);
+        (*env)->SetStaticIntField(env, klazzSjni, fieldId, (jint)pConfFlag->value);
         break;
       case JTYPE_BOOL:
-        (*env)->SetStaticBooleanField(env, klazz, fieldId,
+        (*env)->SetStaticBooleanField(env, klazzSjni, fieldId,
                                       pConfFlag->value ? JNI_TRUE : JNI_FALSE);
         break;
     }

diff --git a/ext/jni/src/c/sqlite3-jni.h b/ext/jni/src/c/sqlite3-jni.h
index 17695b49084960ff98b1a16cc8a7c3e0b3afe450..d05bf77e4562898e98e2590220d580088e5cec7b 100644 (file)
--- a/ext/jni/src/c/sqlite3-jni.h
+++ b/ext/jni/src/c/sqlite3-jni.h
@@ -758,10 +758,18 @@ extern "C" {
 /*
  * Class:     org_sqlite_jni_SQLite3Jni
  * Method:    init
- * Signature: (Lorg/sqlite/jni/SQLite3Jni;)V
+ * Signature: (Ljava/lang/Class;)V
  */
 JNIEXPORT void JNICALL Java_org_sqlite_jni_SQLite3Jni_init
-  (JNIEnv *, jclass, jobject);
+  (JNIEnv *, jclass, jclass);
+
+/*
+ * Class:     org_sqlite_jni_SQLite3Jni
+ * Method:    uncacheJniEnv
+ * Signature: ()Z
+ */
+JNIEXPORT jboolean JNICALL Java_org_sqlite_jni_SQLite3Jni_uncacheJniEnv
+  (JNIEnv *, jclass);
 
 /*
  * Class:     org_sqlite_jni_SQLite3Jni

diff --git a/ext/jni/src/org/sqlite/jni/SQLite3Jni.java b/ext/jni/src/org/sqlite/jni/SQLite3Jni.java
index 80cbbad39195cecc52d8c95facf70f51914c9af4..0320b102fec153c5d047435d242895b382f690e2 100644 (file)
--- a/ext/jni/src/org/sqlite/jni/SQLite3Jni.java
+++ b/ext/jni/src/org/sqlite/jni/SQLite3Jni.java
@@ -66,8 +66,32 @@ public final class SQLite3Jni {
   static {
     System.loadLibrary("sqlite3-jni");
   }
+  //! Not used
   private SQLite3Jni(){}
-  private static native void init(@NotNull SQLite3Jni s);
+  //! Called from static init code.
+  private static native void init(@NotNull Class<SQLite3Jni> c);
+
+  /**
+     Each thread which uses the SQLite3 JNI APIs should call
+     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.
+
+     Calling this from the main application thread is not strictly
+     required but is "polite." Additional threads must call this
+     before ending or they will leak cache entries in the C memory,
+     which in turn may keep numerous Java-side global references
+     active.
+
+     This routine returns false without side effects if the current
+     JNIEnv is not cached, else returns true, but this information is
+     primarily for testing of the JNI bindings and is not information
+     which client-level code should use to make any informed
+     decisions.
+*/
+  public static synchronized native boolean uncacheJniEnv();
 
   //////////////////////////////////////////////////////////////////////
   // Maintenance reminder: please keep the functions alphabetized.
@@ -98,6 +122,9 @@ public final class SQLite3Jni {
   public static native int sqlite3_bind_parameter_count(@NotNull sqlite3_stmt stmt);
 
 
+  /** A level of indirection required to ensure that the input to the
+      C-level function of the same name is a NUL-terminated UTF-8
+      string. */
   private static native int sqlite3_bind_parameter_index(@NotNull sqlite3_stmt stmt,
                                                          byte[] paramName);
 
@@ -1264,6 +1291,6 @@ public final class SQLite3Jni {
   static {
     // This MUST come after the SQLITE_MAX_... values or else
     // attempting to modify them silently fails.
-    init(new SQLite3Jni());
+    init(SQLite3Jni.class);
   }
 }

diff --git a/ext/jni/src/org/sqlite/jni/Tester1.java b/ext/jni/src/org/sqlite/jni/Tester1.java
index a709b96fea928ab9c4852c1498b912d5e8f7cb80..df7b0ad82f47ca3352044656756f247c8ba229fe 100644 (file)
--- a/ext/jni/src/org/sqlite/jni/Tester1.java
+++ b/ext/jni/src/org/sqlite/jni/Tester1.java
@@ -988,6 +988,8 @@ public class Tester1 {
       //listBoundMethods();
     }
     final long timeEnd = System.nanoTime();
+    affirm( SQLite3Jni.uncacheJniEnv() );
+    affirm( !SQLite3Jni.uncacheJniEnv() );
     outln("Tests done. Metrics:");
     outln("\tAssertions checked: "+affirmCount);
     outln("\tDatabases opened: "+metrics.dbOpen);

diff --git a/manifest b/manifest
index c7fb5d16f39f8316d83559d91e19ff70346d3333..295725baceb2f03cd81b54069197394eadcbf07b 100644 (file)
--- a/manifest
+++ b/manifest
@@ -1,5 +1,5 @@
-C In\sthe\sJNI\sdocs,\snote\sthat\sthe\ssizeof\sSQLITE_TRANSIENT\sand\sSQLITE_STATIC\sshould\sideally\sbe\sthe\ssame\sas\sthe\splatform's\spointer\ssize.
-D 2023-08-05T19:20:15.499
+C Add\sSQLite3Jni.uncacheJniEnv(),\sa\sway\sfor\sJava\sthreads\sto\sclear\stheir\sthread-specific\scached\sstate\sfrom\sthe\sJNI\sbindings\swhen\sthey're\sabout\sto\sterminate\s(or\sare\sotherwise\sdone\susing\sthe\slibrary).
+D 2023-08-05T20:19:45.125
 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1
 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea
 F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724
@@ -232,8 +232,8 @@ F ext/icu/icu.c c074519b46baa484bb5396c7e01e051034da8884bad1a1cb7f09bbe6be3f0282
 F ext/icu/sqliteicu.h fa373836ed5a1ee7478bdf8a1650689294e41d0c89c1daab26e9ae78a32075a8
 F ext/jni/GNUmakefile bb4cd99bd8da534215cb6d278f05a626283eb5d2e8aebdb4d35e548637d35a9a
 F ext/jni/README.md 6ff7e1f4100dee980434a6ee37a199b653bceec62e233a6e2ccde6e7ae0c58bf
-F ext/jni/src/c/sqlite3-jni.c 110e133920b469d1ff83ca4c1f23f8b8e6abce40c62b905bedfbdd6d117338a9
-F ext/jni/src/c/sqlite3-jni.h c3824071f3b7f09149ebc9921c5d0295fe343b31eb7a2c41fb6ff76cd2975f64
+F ext/jni/src/c/sqlite3-jni.c 17389f38639294b6ace3030e04a91f8b2e938e191f4c853075d4f55e94665a0c
+F ext/jni/src/c/sqlite3-jni.h cd9b6367a260f55a14833861ceb1d87cb729c5414ba5d26fbb8854b0f22c7249
 F ext/jni/src/org/sqlite/jni/BusyHandler.java 1b1d3e5c86cd796a0580c81b6af6550ad943baa25e47ada0dcca3aff3ebe978c
 F ext/jni/src/org/sqlite/jni/Collation.java 8dffbb00938007ad0967b2ab424d3c908413af1bbd3d212b9c9899910f1218d1
 F ext/jni/src/org/sqlite/jni/CollationNeeded.java ebc7cd96d46a70daa76016a308e80f70a3f21d3282787c8d139aa840fdcb1bd7
@@ -249,8 +249,8 @@ F ext/jni/src/org/sqlite/jni/OutputPointer.java 013f2b5fe569d0585a695f5cfa605a3b
 F ext/jni/src/org/sqlite/jni/ProgressHandler.java 5979450e996416d28543f1d42634d308439565a99332a8bd84e424af667116cc
 F ext/jni/src/org/sqlite/jni/RollbackHook.java b04c8abcc6ade44a8a57129e33765793f69df0ba909e49ba18d73f4268d92564
 F ext/jni/src/org/sqlite/jni/SQLFunction.java 09ce81c1c637e31c3a830d4c859cce95d65f5e02ff45f8bd1985b3479381bc46
-F ext/jni/src/org/sqlite/jni/SQLite3Jni.java 0bdd63552ad5a8d8aa4429008c5da4607720f913ee995b622f8c8063ed2baf09
-F ext/jni/src/org/sqlite/jni/Tester1.java 732d26e858cfe32d664eab805ed8331fcef5cd460b19aa9afac8636f8a92bda3
+F ext/jni/src/org/sqlite/jni/SQLite3Jni.java 213e7dfae620767deb421020cd705a131fd8ad3774e2bc9461eab46d12bf240c
+F ext/jni/src/org/sqlite/jni/Tester1.java 868b5ea60b788a43f8b15c1b642015341fed8856abb1bb74e2eb4845ade50a4e
 F ext/jni/src/org/sqlite/jni/TesterFts5.java cf2d687baafffdeba219b77cf611fd47a0556248820ea794ae3e8259bfbdc5ee
 F ext/jni/src/org/sqlite/jni/Tracer.java a5cece9f947b0af27669b8baec300b6dd7ff859c3e6a6e4a1bd8b50f9714775d
 F ext/jni/src/org/sqlite/jni/UpdateHook.java e58645a1727f8a9bbe72dc072ec5b40d9f9362cb0aa24acfe93f49ff56a9016d
@@ -2081,8 +2081,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 7397973a2f191d75b149cf73a6d7ee1798820c1cd37d83af14e565067ede1b04
-R c054a9f4974b02bb5146402583a90c21
+P 7d4ac44ec419ed0474bdb9d237b97660cf0d8faba8fe686f6a914d7bc04dfa3b
+R 4e66fd3de8c1a8416110ff65fc063c39
 U stephan
-Z 0e23b6ff523f68c333f5ec7c4d0652b3
+Z de9fd1b4a7e3cbc6e1092ca5c70548cf
 # Remove this line to create a well-formed Fossil manifest.

diff --git a/manifest.uuid b/manifest.uuid
index af0a6667d393f04dc7647ab773efbb267efd1db3..5d1a062dd0f987fdfd16f22fb3d57f42c6bb3a37 100644 (file)
--- a/manifest.uuid
+++ b/manifest.uuid
@@ -1 +1 @@
-7d4ac44ec419ed0474bdb9d237b97660cf0d8faba8fe686f6a914d7bc04dfa3b
\ No newline at end of file
+7468f8761bece58f7ced3d112bbe2fb454432d9c54c9b96cedb5a15bc2926d0f
\ No newline at end of file