JNI-internal docs and removal of obsolete code.

FossilOrigin-Name: b62d93258b6a661f3a9b61468b3b641c14faf2d2196f78aca95fe14de43c9444

ext/jni/src/c/sqlite3-jni.c

@@ -432,9 +432,11 @@ struct S3JniHook{
   jmethodID midCallback   /* callback method. Signature depends on
                              jObj's type */;
   jclass klazz            /* global ref to jObj's class. Only needed
-                             by hooks which have an xDestroy() method,
-                             as lookup of that method is deferred
-                             until the object requires cleanup. */;
+                             by hooks which have an xDestroy() method.
+                             We can probably eliminate this and simply
+                             do the class lookup at the same
+                             (deferred) time we do the xDestroy()
+                             lookup. */;
 };
 
 /**
@@ -522,19 +524,38 @@ static struct {
       unsigned nInverse;
     } udf;
   } metrics;
+  /**
+     The list of bound auto-extensions (Java-side:
+     org.sqlite.jni.AutoExtension objects). Because this data
+     structure cannot be manipulated during traversal (without adding
+     more code to deal with it), and to avoid a small window where a
+     call to sqlite3_reset/clear_auto_extension() could lock the
+     structure during an open() call, we lock this mutex before
+     sqlite3_open() is called and unlock it once sqlite3_open()
+     returns.
+   */
   struct {
     S3JniAutoExtension *pHead  /* Head of the auto-extension list */;
-    S3JniDb * pdbOpening /* FIXME: move into envCache. Handle to the
-                            being-opened db. We need this so that auto
-                            extensions can have a consistent view of
-                            the cross-language db connection and
-                            behave property if they call further db
-                            APIs. */;
-    int isRunning        /* True while auto extensions are
-                            running.  This is used to prohibit
-                            manipulation of the auto-extension
-                            list while extensions are
-                            running. */;
+    /**
+       pdbOpening is used to coordinate the Java/DB connection of a
+       being-open()'d db. "The problem" is that auto-extensions run
+       before we can bind the C db to its Java representation, but
+       auto-extensions require that binding. We handle this as
+       follows:
+
+       - At the start of open(), we lock on this->mutex.
+       - Allocate the Java side of that connection and set pdbOpening
+         to point to that object.
+       - Call open(), which triggers the auto-extension handler.
+         That handler uses pdbOpening to connect the native db handle
+         which it recieves with pdbOpening.
+       - Return from open().
+       - Clean up and unlock the mutex.
+
+       If open() did not block on a mutex, there would be a race
+       condition in which two open() calls could set pdbOpening.
+    */
+    S3JniDb * pdbOpening;
     sqlite3_mutex * mutex /* mutex for aUsed and aFree */;
     void const * locker   /* Mutex is locked on this object's behalf */;
   } autoExt;
@@ -1906,7 +1927,6 @@ static int s3jni_run_java_auto_extensions(sqlite3 *pDb, const char **pzErr,
   assert( S3JniGlobal.autoExt.locker == ps );
   S3JniGlobal.autoExt.pdbOpening = 0;
   if( !pAX ){
-    assert( 0==S3JniGlobal.autoExt.isRunning );
     return 0;
   }else if( S3JniGlobal.autoExt.locker != ps ) {
     *pzErr = sqlite3_mprintf("Internal error: unexpected path lead to "
@@ -1919,7 +1939,6 @@ static int s3jni_run_java_auto_extensions(sqlite3 *pDb, const char **pzErr,
   ps->pDb = pDb;
   assert( ps->jDb );
   NativePointerHolder_set(env, ps->jDb, pDb, S3JniClassNames.sqlite3);
-  ++S3JniGlobal.autoExt.isRunning;
   for( ; pAX; pAX = pAX->pNext ){
     rc = (*env)->CallIntMethod(env, pAX->jObj, pAX->midFunc, ps->jDb);
     IFTHREW {
@@ -1936,7 +1955,6 @@ static int s3jni_run_java_auto_extensions(sqlite3 *pDb, const char **pzErr,
       break;
     }
   }
-  --S3JniGlobal.autoExt.isRunning;
   return rc;
 }
 
@@ -2100,13 +2118,11 @@ JDECL(jboolean,1cancel_1auto_1extension)(JENV_CSELF, jobject jAutoExt){
   S3JniAutoExtension * ax;
   jboolean rc = JNI_FALSE;
   MUTEX_ENTER_EXT;
-  if( !S3JniGlobal.autoExt.isRunning ) {
-    for( ax = S3JniGlobal.autoExt.pHead; ax; ax = ax->pNext ){
-      if( (*env)->IsSameObject(env, ax->jObj, jAutoExt) ){
-        S3JniAutoExtension_free(env, ax);
-        rc = JNI_TRUE;
-        break;
-      }
+  for( ax = S3JniGlobal.autoExt.pHead; ax; ax = ax->pNext ){
+    if( (*env)->IsSameObject(env, ax->jObj, jAutoExt) ){
+      S3JniAutoExtension_free(env, ax);
+      rc = JNI_TRUE;
+      break;
     }
   }
   MUTEX_LEAVE_EXT;
@@ -2660,7 +2676,14 @@ static int s3jni_open_pre(JNIEnv * const env, S3JniEnv **jc,
                           jstring jDbName, char **zDbName,
                           S3JniDb ** ps, jobject *jDb){
   int rc = 0;
-  MUTEX_TRY_EXT(return SQLITE_BUSY);
+  MUTEX_TRY_EXT(return SQLITE_BUSY)
+    /* we don't wait forever here because it could lead to a deadlock
+       if an auto-extension opens a database. Without a mutex, that
+       situation leads to infinite recursion and stack overflow, which
+       is infinitely easier to track down from client code. Note that
+       we rely on the Java methods for open() and auto-extension
+       handling to be synchronized so that this BUSY cannot be
+       triggered by a race condition with those functions. */;
   *jc = S3JniGlobal_env_cache(env);
   if(!*jc){
     rc = SQLITE_NOMEM;
@@ -2714,8 +2737,12 @@ static int s3jni_open_post(JNIEnv * const env, S3JniDb * ps,
   S3JniGlobal.autoExt.pdbOpening = 0;
   if(*ppDb){
     assert(ps->jDb);
-    ps->pDb = *ppDb;
-    NativePointerHolder_set(env, ps->jDb, *ppDb, S3JniClassNames.sqlite3);
+    if( 0==ps->pDb ){
+      ps->pDb = *ppDb;
+      NativePointerHolder_set(env, ps->jDb, *ppDb, S3JniClassNames.sqlite3);
+    }else{
+      assert( ps->pDb == *ppDb /* set up via s3jni_run_java_auto_extensions() */);
+    }
   }else{
     MUTEX_ENTER_PDB;
     S3JniDb_set_aside(ps);

ext/jni/src/org/sqlite/jni/Tester1.java

@@ -1079,10 +1079,17 @@ public class Tester1 {
     affirm( 0==rc );
     sqlite3_close( createNewDb() );
     affirm( 3==val.value );
+
+    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. */ );
+    sqlite3_close(db);
+
     affirm( sqlite3_cancel_auto_extension(ax) );
     affirm( !sqlite3_cancel_auto_extension(ax) );
     sqlite3_close(createNewDb());
-    affirm( 3==val.value );
+    affirm( 4==val.value );
     rc = sqlite3_auto_extension( ax );
     affirm( 0==rc );
     Exception err = null;

manifest

@@ -1,5 +1,5 @@
-C Internal\sAPI\srenaming\sfor\sclarity's\ssake.
-D 2023-08-13T20:58:12.729
+C JNI-internal\sdocs\sand\sremoval\sof\sobsolete\scode.
+D 2023-08-14T08:28:46.677
 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1
 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea
 F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724
@@ -234,7 +234,7 @@ F ext/icu/sqliteicu.h fa373836ed5a1ee7478bdf8a1650689294e41d0c89c1daab26e9ae78a3
 F ext/jni/GNUmakefile a9e11b92e620058558cbc1a2d49f8ec53c78d6a989b9db0b7d0b649b9f174881
 F ext/jni/README.md 7a614a2fa6c561205f7a53fd8626cf93a7b5711ff454fc1814517f796df398eb
 F ext/jni/jar-dist.make f90a553203a57934bf275bed86479485135a52f48ac5c1cfe6499ae07b0b35a4
-F ext/jni/src/c/sqlite3-jni.c 1b1f3fa286476933171e75465214deab44c55714aaaa80b5ce145e89f10b0df8
+F ext/jni/src/c/sqlite3-jni.c 1bc1cbaf075065793f83cf5bfba631216ced48d7beae0768ebd838e923e7e590
 F ext/jni/src/c/sqlite3-jni.h f10d2f38720687c70ecdd5e44f6e8db98efee2caa05fc86b2d9e0c76e6cc0a18
 F ext/jni/src/org/sqlite/jni/Authorizer.java 1308988f7f40579ea0e4deeaec3c6be971630566bd021c31367fe3f5140db892
 F ext/jni/src/org/sqlite/jni/AutoExtension.java 18e83f6f463e306df60b2dceb65247d32af1f78af4bbbae9155411a8c6cdb093
@@ -255,7 +255,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 09ce81c1c637e31c3a830d4c859cce95d65f5e02ff45f8bd1985b3479381bc46
 F ext/jni/src/org/sqlite/jni/SQLite3Jni.java 5eeba0b1a00fb34bc93fe60186f6032fcf4d568fc5868d70029883d3d07cc306
-F ext/jni/src/org/sqlite/jni/Tester1.java fc2ec1f1be58474112b9df8284f0157b64872107f446154c3d0bf1742b924d2b
+F ext/jni/src/org/sqlite/jni/Tester1.java 368e836d943d9e882d2a217d0f582ed4141d164f174bebc50715acd57549a09b
 F ext/jni/src/org/sqlite/jni/TesterFts5.java 59e22dd24af033ea8827d36225a2f3297908fb6af8818ead8850c6c6847557b1
 F ext/jni/src/org/sqlite/jni/Tracer.java a5cece9f947b0af27669b8baec300b6dd7ff859c3e6a6e4a1bd8b50f9714775d
 F ext/jni/src/org/sqlite/jni/UpdateHook.java e58645a1727f8a9bbe72dc072ec5b40d9f9362cb0aa24acfe93f49ff56a9016d
@@ -2091,8 +2091,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 8da97e0db4eeacf91aa6fd909fd7cb73b050d194dfc7739a502b55f7eca6d7b1
-R 582810911695a14eb1f701cb46e67a37
+P 911e4fc5aaf9478214095a65f74af3ebca883922c36cf7a8d911116c42cf9de8
+R c6d49900ed87c14449d9c08a36536c71
 U stephan
-Z 414dea18ee7e2187dd201f9308cbe98b
+Z 09908deaf1ed6a1fae2d9bef97861a7c
 # Remove this line to create a well-formed Fossil manifest.

manifest.uuid

@@ -1 +1 @@
-911e4fc5aaf9478214095a65f74af3ebca883922c36cf7a8d911116c42cf9de8
+b62d93258b6a661f3a9b61468b3b641c14faf2d2196f78aca95fe14de43c9444