Bind sqlite3_collation_needed() to JNI. Related adjacent cleanups and fixes.

FossilOrigin-Name: 16ff167691733350907d2d995c774a885214acd0fe8ec491c16b786f00fe85d4

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

@@ -367,7 +367,13 @@ struct PerDbStateJni {
   JNIEnv *env;
   sqlite3 * pDb;
   jobject jDb /* a global ref of the object which was passed to
-                 sqlite3_open(_v2)() */;
+                 sqlite3_open(_v2)(). We need this in order to have an
+                 object to pass to sqlite3_collation_needed()'s
+                 callback, or else we have to dynamically create one
+                 for that purpose, which would be fine except that it
+                 would be a different instance (and maybe even a
+                 different class) than the one the user expects to
+                 receive. */;
   PerDbStateJni * pNext;
   PerDbStateJni * pPrev;
   JniHookState trace;
@@ -375,6 +381,7 @@ struct PerDbStateJni {
   JniHookState commitHook;
   JniHookState rollbackHook;
   JniHookState updateHook;
+  JniHookState collationNeeded;
   BusyHandlerJni busyHandler;
 };
 
@@ -744,6 +751,7 @@ static void PerDbStateJni_dump(PerDbStateJni *s){
 FIXME_THREADING
 static PerDbStateJni * PerDbStateJni_for_db(JNIEnv *env, jobject jDb, sqlite3 *pDb, int allocIfNeeded){
   PerDbStateJni * s = S3Global.perDb.aUsed;
+  if(!jDb) return 0;
   assert(allocIfNeeded ? !!pDb : 1);
   if(!allocIfNeeded && !pDb){
     pDb = PtrGet_sqlite3_value(jDb);
@@ -1461,6 +1469,75 @@ JDECL(jint,1close)(JENV_JSELF, jobject pDb){
   return s3jni_close_db(env, pDb, 1);
 }
 
+/**
+   Assumes z is an array of unsigned short and returns the index in
+   that array of the first element with the value 0.
+*/
+static unsigned int s3jni_utf16_strlen(void const * z){
+  unsigned int i = 0;
+  const unsigned short * p = z;
+  while( p[i] ) ++i;
+  return i;
+}
+
+static void s3jni_collation_needed_impl16(void *pState, sqlite3 *pDb,
+                                          int eTextRep, const void * z16Name){
+  PerDbStateJni * const ps = pState;
+  JNIEnv * const env = ps->env;
+  unsigned int const nName = s3jni_utf16_strlen(z16Name);
+  jstring jName;
+  jName  = (*env)->NewString(env, (jchar const *)z16Name, nName);
+  IFTHREW {
+    s3jni_db_error(ps->pDb, SQLITE_NOMEM, 0);
+  }else{
+    (*env)->CallVoidMethod(env, ps->collationNeeded.jObj,
+                           ps->collationNeeded.midCallback,
+                           ps->jDb, (jint)eTextRep, jName);
+    IFTHREW{
+      EXCEPTION_WARN_CALLBACK_THREW;
+      EXCEPTION_CLEAR;
+      s3jni_db_error(ps->pDb, SQLITE_ERROR, "collation-needed hook threw.");
+    }
+  }
+  UNREF_L(jName);
+}
+
+JDECL(jint,1collation_1needed)(JENV_JSELF, jobject jDb, jobject jHook){
+  PerDbStateJni * const ps = PerDbStateJni_for_db(env, jDb, 0, 0);
+  jclass klazz;
+  jobject pOld = 0;
+  jmethodID xCallback;
+  JniHookState * const pHook = &ps->collationNeeded;
+  int rc;
+  if(!ps) return SQLITE_MISUSE;
+  pOld = pHook->jObj;
+  if(pOld && jHook &&
+     (*env)->IsSameObject(env, pOld, jHook)){
+    return 0;
+  }
+  if( !jHook ){
+    UNREF_G(pOld);
+    memset(pHook, 0, sizeof(JniHookState));
+    sqlite3_collation_needed(ps->pDb, 0, 0);
+    return 0;
+  }
+  klazz = (*env)->GetObjectClass(env, jHook);
+  xCallback = (*env)->GetMethodID(env, klazz, "xCollationNeeded",
+                                  "(Lorg/sqlite/jni/sqlite3;ILjava/lang/String;)I");
+  IFTHREW {
+    EXCEPTION_CLEAR;
+    rc = s3jni_db_error(ps->pDb, SQLITE_MISUSE,
+                        "Cannot not find matching callback on "
+                        "collation-needed hook object.");
+  }else{
+    pHook->midCallback = xCallback;
+    pHook->jObj = REF_G(jHook);
+    UNREF_G(pOld);
+    rc = sqlite3_collation_needed16(ps->pDb, ps, s3jni_collation_needed_impl16);
+  }
+  return rc;
+}
+
 JDECL(jbyteArray,1column_1blob)(JENV_JSELF, jobject jpStmt,
                                 jint ndx){
   sqlite3_stmt * const pStmt = PtrGet_sqlite3_stmt(jpStmt);
@@ -1753,6 +1830,11 @@ JDECL(jlong,1last_1insert_1rowid)(JENV_JSELF, jobject jpDb){
   return (jlong)sqlite3_last_insert_rowid(PtrGet_sqlite3(jpDb));
 }
 
+/**
+   Code common to both the sqlite3_open() and sqlite3_open_v2()
+   bindings. Allocates the PerDbStateJni for *ppDb if *ppDb is not
+   NULL.
+*/
 static int s3jni_open_post(JNIEnv *env, sqlite3 **ppDb, jobject jDb, int theRc){
   if(1 && *ppDb){
     PerDbStateJni * const s = PerDbStateJni_for_db(env, jDb, *ppDb, 1);
@@ -2173,7 +2255,7 @@ JDECL(jobject,1update_1hook)(JENV_JSELF, jobject jDb, jobject jHook){
   jmethodID xCallback;
   JniHookState * const pHook = &ps->updateHook;
   if(!ps){
-    s3jni_db_error(ps->pDb, SQLITE_NOMEM, 0);
+    s3jni_db_error(ps->pDb, SQLITE_MISUSE, 0);
     return 0;
   }
   pOld = pHook->jObj;

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

@@ -1019,6 +1019,14 @@ JNIEXPORT jint JNICALL Java_org_sqlite_jni_SQLite3Jni_sqlite3_1column_1type
 JNIEXPORT jobject JNICALL Java_org_sqlite_jni_SQLite3Jni_sqlite3_1column_1value
   (JNIEnv *, jclass, jobject, jint);
 
+/*
+ * Class:     org_sqlite_jni_SQLite3Jni
+ * Method:    sqlite3_collation_needed
+ * Signature: (Lorg/sqlite/jni/sqlite3;Lorg/sqlite/jni/CollationNeeded;)I
+ */
+JNIEXPORT jint JNICALL Java_org_sqlite_jni_SQLite3Jni_sqlite3_1collation_1needed
+  (JNIEnv *, jclass, jobject, jobject);
+
 /*
  * Class:     org_sqlite_jni_SQLite3Jni
  * Method:    sqlite3_context_db_handle

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

@@ -18,15 +18,13 @@ package org.sqlite.jni;
 */
 public interface CollationNeeded {
   /**
-     Works as documented for the sqlite3_create_collation() callback.
-     Must not throw.
+     Has the same semantics as the C-level sqlite3_create_collation()
+     callback.  Must not throw.
 
-     Achtung: the first argument to this function is not guaranteed to
-     be the same object upon which ealier DB operations have been
-     performed, e.g. not the one passed to sqlite3_collation_needed(),
-     but it will refer to the same underlying C-level database
-     pointer. This quirk is a side effect of how per-db state is
-     managed in the JNI layer.
+     Pedantic note: the first argument to this function will always be
+     the same object reference which was passed to sqlite3_open() or
+     sqlite3_open_v2(), even if the client has managed to create other
+     Java-side references to the same C-level object.
   */
   int xCollationNeeded(sqlite3 db, int eTextRep, String collationName);
 }

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

@@ -235,8 +235,8 @@ public final class SQLite3Jni {
   public static native sqlite3_value sqlite3_column_value(@NotNull sqlite3_stmt stmt,
                                                           int ndx);
 
-  // TODO public static native int sqlite3_collation_needed(
-  //sqlite3 db, void(*)(void*,sqlite3*,int eTextRep,const char*))
+  public static native int sqlite3_collation_needed(@NotNull sqlite3 db,
+                                                    @Nullable CollationNeeded callback);
 
   //TODO public static native int sqlite3_collation_needed16(
   //  sqlite3 db, void(*)(void*,sqlite3*,int eTextRep,const void*)
@@ -284,6 +284,20 @@ public final class SQLite3Jni {
 
   public static native int sqlite3_libversion_number();
 
+  /**
+     Works like its C counterpart and makes the native pointer of the
+     underling (sqlite3*) object available via
+     ppDb.getNativePointer(). That pointer is necessary for looking up
+     the JNI-side native, but clients need not pay it any
+     heed. Passing the object to sqlite3_close() or sqlite3_close_v2()
+     will clear that pointer mapping.
+
+     Pedantic note: though any number of Java-level sqlite3 objects
+     may refer to/wrap a single C-level (sqlite3*), the JNI internals
+     take a reference to the object which is passed to sqlite3_open()
+     or sqlite3_open_v2() so that they have a predictible object to
+     pass to, e.g., the sqlite3_collation_needed() callback.
+  */
   public static native int sqlite3_open(@Nullable String filename,
                                         @NotNull sqlite3 ppDb);
 

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

@@ -353,7 +353,7 @@ public class Tester1 {
   }
 
   private static void testCollation(){
-    sqlite3 db = createNewDb();
+    final sqlite3 db = createNewDb();
     execSql(db, "CREATE TABLE t(a); INSERT INTO t(a) VALUES('a'),('b'),('c')");
     final ValueHolder<Boolean> xDestroyCalled = new ValueHolder<>(false);
     final Collation myCollation = new Collation() {
@@ -380,10 +380,19 @@ public class Tester1 {
           xDestroyCalled.value = true;
         }
       };
-    int rc = sqlite3_create_collation(db, "reversi", SQLITE_UTF8, myCollation);
-    affirm(0 == rc);
+    final CollationNeeded collLoader = new CollationNeeded(){
+        public int xCollationNeeded(sqlite3 dbArg, int eTextRep, String collationName){
+          affirm(dbArg == db/* as opposed to a temporary object*/);
+          return sqlite3_create_collation(dbArg, "reversi", eTextRep, myCollation);
+        }
+      };
+    int rc = sqlite3_collation_needed(db, collLoader);
+    affirm( 0 == rc );
+    rc = sqlite3_collation_needed(db, collLoader);
+    affirm( 0 == rc /* Installing the same object again is a no-op */);
     sqlite3_stmt stmt = new sqlite3_stmt();
-    sqlite3_prepare(db, "SELECT a FROM t ORDER BY a COLLATE reversi", stmt);
+    rc = sqlite3_prepare(db, "SELECT a FROM t ORDER BY a COLLATE reversi", stmt);
+    affirm( 0 == rc );
     int counter = 0;
     while( SQLITE_ROW == sqlite3_step(stmt) ){
       final String val = sqlite3_column_text(stmt, 0);
@@ -412,6 +421,8 @@ public class Tester1 {
     affirm(3 == counter);
     sqlite3_finalize(stmt);
     affirm(!xDestroyCalled.value);
+    rc = sqlite3_collation_needed(db, null);
+    affirm( 0 == rc );
     sqlite3_close_v2(db);
     affirm(xDestroyCalled.value);
   }

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

@@ -23,13 +23,4 @@ public class sqlite3 extends NativePointerHolder<sqlite3> {
   public sqlite3() {
     super();
   }
-  /**
-     Construct a new instance which refers to an existing
-     native (sqlite3*). The argument may be 0. Results are
-     undefined if it is not 0 and refers to a memory address
-     other than a valid (sqlite*).
-  */
-  public sqlite3(long nativePointer) {
-    super(nativePointer);
-  }
 }

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

@@ -23,13 +23,4 @@ public class sqlite3_stmt extends NativePointerHolder<sqlite3_stmt> {
   public sqlite3_stmt() {
     super();
   }
-  /**
-     Construct a new instance which refers to an existing native
-     (sqlite3_stmt*). The argument may be 0. Results are undefined if
-     it is not 0 and refers to a memory address other than a valid
-     (sqlite_stmt*).
-  */
-  public sqlite3_stmt(long nativePointer) {
-    super(nativePointer);
-  }
 }

manifest

@@ -1,5 +1,5 @@
-C Internal\sJNI\srefacoring\sto\ssupport\sthe\spending\ssqlite3_collation_needed()\scallback.\sCorrect\sa\sbug\sin\sthe\slinked-list\shandling\sof\sPerDbStateJni\swhich\striggered\san\sassert().
-D 2023-07-30T10:47:38.755
+C Bind\ssqlite3_collation_needed()\sto\sJNI.\sRelated\sadjacent\scleanups\sand\sfixes.
+D 2023-07-30T11:36:41.439
 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1
 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea
 F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724
@@ -232,25 +232,25 @@ F ext/icu/icu.c c074519b46baa484bb5396c7e01e051034da8884bad1a1cb7f09bbe6be3f0282
 F ext/icu/sqliteicu.h fa373836ed5a1ee7478bdf8a1650689294e41d0c89c1daab26e9ae78a32075a8
 F ext/jni/GNUmakefile 56a014dbff9516774d895ec1ae9df0ed442765b556f79a0fc0b5bc438217200d
 F ext/jni/README.md c0e6e80935e7761acead89b69c87765b23a6bcb2858c321c3d05681fd338292a
-F ext/jni/src/c/sqlite3-jni.c 57db39bd2443435764777a1e43e2f8e356b8c411ee2649ad08df4b32087cbe80
-F ext/jni/src/c/sqlite3-jni.h 85345dd3c970b539f1de4e6ad59c245fa6e80ca775a498ab1ed3d67f8615ce34
+F ext/jni/src/c/sqlite3-jni.c 1934a72f33fe356d8af810a8a662dd8109026cd0bbf298dda1fe8bd1146603ad
+F ext/jni/src/c/sqlite3-jni.h 28def286ee305c1c89a43ac5918a6862d985d0534f7ccbbd74df4885d3918b73
 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 15ca4e92b669c6412594120a9379459cd3e6e9e8ffba18c8698d879ce1142c91
+F ext/jni/src/org/sqlite/jni/CollationNeeded.java ebc7cd96d46a70daa76016a308e80f70a3f21d3282787c8d139aa840fdcb1bd7
 F ext/jni/src/org/sqlite/jni/CommitHook.java 87c6a8e5138c61a8eeff018fe16d23f29219150239746032687f245938baca1a
 F ext/jni/src/org/sqlite/jni/NativePointerHolder.java 70dc7bc41f80352ff3d4331e2e24f45fcd23353b3641e2f68a81bd8262215861
 F ext/jni/src/org/sqlite/jni/OutputPointer.java 08a752b58a33696c5eaf0eb9361a0966b188dec40f4a3613eb133123951f6c5f
 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 663a4e479ec65bfbf893586439e12d30b8237898064a22ab64f5658b57315f37
-F ext/jni/src/org/sqlite/jni/SQLite3Jni.java e64bae3357cc16f9416926539e2aa08fbb5a35022a828a158cfdd3e310575324
-F ext/jni/src/org/sqlite/jni/Tester1.java 2d43b851db4189e54527e7fb4d50493c8efaa6c0781d0f5cc7f249c95b48ce3b
+F ext/jni/src/org/sqlite/jni/SQLite3Jni.java 2c4564b19f5366927c9a5062e36ffb7744e7f69d00b3f8ce35fe59b2f3d60698
+F ext/jni/src/org/sqlite/jni/Tester1.java a89a87f8debd89f3488a65cb42af8e14fb0150b05d5a4a3592fb86d0cfda3287
 F ext/jni/src/org/sqlite/jni/Tracer.java c2fe1eba4a76581b93b375a7b95ab1919e5ae60accfb06d6beb067b033e9bae1
 F ext/jni/src/org/sqlite/jni/UpdateHook.java e58645a1727f8a9bbe72dc072ec5b40d9f9362cb0aa24acfe93f49ff56a9016d
 F ext/jni/src/org/sqlite/jni/ValueHolder.java f022873abaabf64f3dd71ab0d6037c6e71cece3b8819fa10bf26a5461dc973ee
-F ext/jni/src/org/sqlite/jni/sqlite3.java c7d0500c7269882243aafb41425928d094b2fcbdbc2fd1caffc276871cd3fae3
+F ext/jni/src/org/sqlite/jni/sqlite3.java 4058fbd63eb7085b5dc2daef4130623f464efdc838aafab8b9a4808c7cb01b6b
 F ext/jni/src/org/sqlite/jni/sqlite3_context.java 841ac0384ec23e7d24ad9a928f8728b98bd3c4c3814d401200c6531786b9c241
-F ext/jni/src/org/sqlite/jni/sqlite3_stmt.java 3193693440071998a66870544d1d2314f144bea397ce4c3f83ff225d587067a0
+F ext/jni/src/org/sqlite/jni/sqlite3_stmt.java f602b12521a66992299ca2877260d87bc69176b1bb05201f3b46825cb3cba315
 F ext/jni/src/org/sqlite/jni/sqlite3_value.java f9d8c0766b1d1b290564cb35db8d37be54c42adc8df22ee77b8d39e3e93398cd
 F ext/lsm1/Makefile a553b728bba6c11201b795188c5708915cc4290f02b7df6ba7e8c4c943fd5cd9
 F ext/lsm1/Makefile.msc f8c878b467232226de288da320e1ac71c131f5ec91e08b21f502303347260013
@@ -2071,8 +2071,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 2d7a91b1396d87852f1153ab7af7385514a9537cb64ba3bbd0faba2d28704214
-R 5bff367529a1829789141e745a6b193a
+P 7ac6614e69b03304d09745619ed83f12c7eb775aaf4a636a79289b01642ddd14
+R fe2ec7cfe7eced93fd3b168114e0d2e0
 U stephan
-Z a4b019fdb819701e83942a71127183e7
+Z 9246dbb52619ce19a9defcf2e690b44f
 # Remove this line to create a well-formed Fossil manifest.

manifest.uuid

@@ -1 +1 @@
-7ac6614e69b03304d09745619ed83f12c7eb775aaf4a636a79289b01642ddd14
+16ff167691733350907d2d995c774a885214acd0fe8ec491c16b786f00fe85d4