Experimentally change the JNI sqlite3_trace_v2() callback type to have more convenien...

FossilOrigin-Name: 459db332af6ea358b42bac096b9d26f1045b9ec32fad8463bca06807b2396b2c

diff --git a/ext/jni/src/c/sqlite3-jni.c b/ext/jni/src/c/sqlite3-jni.c
index 088ae4f88b9bceed0ab09cbd67eb9a82a143eb78..ee389d40320356ae44cddb2b3544f2826c03cde0 100644 (file)
--- a/ext/jni/src/c/sqlite3-jni.c
+++ b/ext/jni/src/c/sqlite3-jni.c
@@ -164,9 +164,11 @@
 #define EXCEPTION_IGNORE (void)((*env)->ExceptionCheck(env))
 #define EXCEPTION_CLEAR (*env)->ExceptionClear(env)
 #define EXCEPTION_REPORT (*env)->ExceptionDescribe(env)
-#define EXCEPTION_WARN_CALLBACK_THREW \
-  MARKER(("WARNING: this routine MUST NOT THROW.\n"));  \
+#define EXCEPTION_WARN_CALLBACK_THREW1(STR)             \
+  MARKER(("WARNING: " STR " MUST NOT THROW.\n"));  \
   (*env)->ExceptionDescribe(env)
+#define EXCEPTION_WARN_CALLBACK_THREW \
+   EXCEPTION_WARN_CALLBACK_THREW1("this routine")
 #define IFTHREW_REPORT IFTHREW EXCEPTION_REPORT
 #define IFTHREW_CLEAR IFTHREW EXCEPTION_CLEAR
 #define EXCEPTION_CANNOT_HAPPEN IFTHREW{\
@@ -313,6 +315,18 @@ struct JNIEnvCacheLine {
   jclass globalClassObj  /* global ref to java.lang.Object */;
   jclass globalClassLong /* global ref to java.lang.Long */;
   jmethodID ctorLong1    /* the Long(long) constructor */;
+  jobject currentStmt    /* Current Java sqlite3_stmt object being
+                            prepared, stepped, reset, or
+                            finalized. Needed for tracing, the
+                            alternative being that we create a new
+                            sqlite3_stmt wrapper object for every
+                            tracing call which needs a stmt
+                            object. This approach is rather invasive,
+                            however, and there may well be places
+                            tracing may trigger which we have no
+                            accounted for, so it may be best to
+                            redefine the tracing API rather than
+                            passing through the statement handles. */;
   struct NphCacheLine nph[NphCache_SIZE];
 };
 typedef struct JNIEnvCache JNIEnvCache;
@@ -1070,6 +1084,10 @@ static jobject new_sqlite3_context_wrapper(JNIEnv *env, sqlite3_context *sv){
   return new_NativePointerHolder_object(env, "org/sqlite/jni/sqlite3_context", sv);
 }
 
+static jobject new_sqlite3_stmt_wrapper(JNIEnv *env, sqlite3_stmt *sv){
+  return new_NativePointerHolder_object(env, "org/sqlite/jni/sqlite3_stmt", sv);
+}
+
 enum UDFType {
   UDF_SCALAR = 1,
   UDF_AGGREGATE,
@@ -1320,10 +1338,8 @@ WRAP_INT_STMT_INT(1column_1type,       sqlite3_column_type)
 WRAP_INT_STMT(1data_1count,            sqlite3_data_count)
 WRAP_MUTF8_VOID(1libversion,           sqlite3_libversion)
 WRAP_INT_VOID(1libversion_1number,     sqlite3_libversion_number)
-WRAP_INT_STMT(1reset,                  sqlite3_reset)
 WRAP_INT_INT(1sleep,                   sqlite3_sleep)
 WRAP_MUTF8_VOID(1sourceid,             sqlite3_sourceid)
-WRAP_INT_STMT(1step,                   sqlite3_step)
 WRAP_INT_VOID(1threadsafe,             sqlite3_threadsafe)
 WRAP_INT_DB(1total_1changes,           sqlite3_total_changes)
 WRAP_INT64_DB(1total_1changes64,       sqlite3_total_changes64)
@@ -1846,13 +1862,27 @@ JDECL(jint,1initialize)(JENV_JSELF){
   return sqlite3_initialize();
 }
 
+/**
+   Sets jc->currentStmt to the 2nd arugment and returns the previous value
+   of jc->currentStmt.
+*/
+static jobject stmt_set_current(JNIEnvCacheLine * const jc, jobject jStmt){
+  jobject const old = jc->currentStmt;
+  jc->currentStmt = jStmt;
+  return old;
+}
+
 JDECL(jint,1finalize)(JENV_JSELF, jobject jpStmt){
-  if( jpStmt ){
-    sqlite3_stmt * pStmt = PtrGet_sqlite3_stmt(jpStmt);
+  int rc = 0;
+  sqlite3_stmt * const pStmt = PtrGet_sqlite3_stmt(jpStmt);
+  if( pStmt ){
+    JNIEnvCacheLine * const jc = S3Global_env_cache(env);
+    jobject const pPrev = stmt_set_current(jc, jpStmt);
+    rc = sqlite3_finalize(pStmt);
     setNativePointer(env, jpStmt, 0, ClassNames.sqlite3_stmt);
-    if( pStmt ) sqlite3_finalize(pStmt);
+    (void)stmt_set_current(jc, pPrev);
   }
-  return 0;
+  return rc;
 }
 
 
@@ -1908,10 +1938,12 @@ JDECL(jint,1open_1v2)(JENV_JSELF, jstring strName,
 static jint sqlite3_jni_prepare_v123(int prepVersion, JNIEnv *env, jclass self,
                                      jobject jpDb, jbyteArray baSql,
                                      jint nMax, jint prepFlags,
-                                     jobject outStmt, jobject outTail){
+                                     jobject jOutStmt, jobject outTail){
   sqlite3_stmt * pStmt = 0;
   const char * zTail = 0;
   jbyte * const pBuf = JBA_TOC(baSql);
+  JNIEnvCacheLine * const jc = S3Global_env_cache(env);
+  jobject const pOldStmt = stmt_set_current(jc, jOutStmt);
   int rc = SQLITE_ERROR;
   assert(prepVersion==1 || prepVersion==2 || prepVersion==3);
   switch( prepVersion ){
@@ -1934,23 +1966,24 @@ static jint sqlite3_jni_prepare_v123(int prepVersion, JNIEnv *env, jclass self,
     assert(zTail ? (((int)((void*)zTail - (void*)pBuf)) >= 0) : 1);
     setOutputInt32(env, outTail, (int)(zTail ? (zTail - (const char *)pBuf) : 0));
   }
-  setNativePointer(env, outStmt, pStmt, ClassNames.sqlite3_stmt);
+  setNativePointer(env, jOutStmt, pStmt, ClassNames.sqlite3_stmt);
+  (void)stmt_set_current(jc, pOldStmt);
   return (jint)rc;
 }
 JDECL(jint,1prepare)(JNIEnv *env, jclass self, jobject jpDb, jbyteArray baSql,
-                     jint nMax, jobject outStmt, jobject outTail){
+                     jint nMax, jobject jOutStmt, jobject outTail){
   return sqlite3_jni_prepare_v123(1, env, self, jpDb, baSql, nMax, 0,
-                                  outStmt, outTail);
+                                  jOutStmt, outTail);
 }
 JDECL(jint,1prepare_1v2)(JNIEnv *env, jclass self, jobject jpDb, jbyteArray baSql,
-                         jint nMax, jobject outStmt, jobject outTail){
+                         jint nMax, jobject jOutStmt, jobject outTail){
   return sqlite3_jni_prepare_v123(2, env, self, jpDb, baSql, nMax, 0,
-                                  outStmt, outTail);
+                                  jOutStmt, outTail);
 }
 JDECL(jint,1prepare_1v3)(JNIEnv *env, jclass self, jobject jpDb, jbyteArray baSql,
-                         jint nMax, jint prepFlags, jobject outStmt, jobject outTail){
+                         jint nMax, jint prepFlags, jobject jOutStmt, jobject outTail){
   return sqlite3_jni_prepare_v123(3, env, self, jpDb, baSql, nMax,
-                                  prepFlags, outStmt, outTail);
+                                  prepFlags, jOutStmt, outTail);
 }
 
 
@@ -1999,6 +2032,18 @@ JDECL(void,1progress_1handler)(JENV_JSELF,jobject jDb, jint n, jobject jProgress
 }
 
 
+JDECL(jint,1reset)(JENV_JSELF, jobject jpStmt){
+  int rc = 0;
+  sqlite3_stmt * const pStmt = PtrGet_sqlite3_stmt(jpStmt);
+  if( pStmt ){
+    JNIEnvCacheLine * const jc = S3Global_env_cache(env);
+    jobject const pPrev = stmt_set_current(jc, jpStmt);
+    rc = sqlite3_reset(pStmt);
+    (void)stmt_set_current(jc, pPrev);
+  }
+  return rc;
+}
+
 /* sqlite3_result_text/blob() and friends. */
 static void result_blob_text(int asBlob, int as64,
                              int eTextRep/*only for (asBlob=0)*/,
@@ -2191,38 +2236,71 @@ JDECL(jint,1shutdown)(JENV_JSELF){
   return sqlite3_shutdown();
 }
 
+JDECL(jint,1step)(JENV_JSELF,jobject jStmt){
+  jobject jPrevStmt = 0;
+  JNIEnvCacheLine * const jc = S3Global_env_cache(env);
+  int rc;
+  sqlite3_stmt * const pStmt = PtrGet_sqlite3_stmt(jStmt);
+  if(!pStmt) return SQLITE_MISUSE;
+  jPrevStmt = stmt_set_current(jc, jStmt);
+  rc = sqlite3_step(pStmt);
+  (void)stmt_set_current(jc, jPrevStmt);
+  return rc;
+}
+
 static int s3jni_trace_impl(unsigned traceflag, void *pC, void *pP, void *pX){
   PerDbStateJni * const ps = (PerDbStateJni *)pC;
   JNIEnv * const env = ps->env;
-  jobject jX = NULL;
-  JNIEnvCacheLine * const pEcl = S3Global_env_cache(env);
+  jobject jX = NULL  /* the tracer's X arg */;
+  jobject jP = NULL  /* the tracer's P arg */;
+  jobject jPUnref = NULL /* potentially a local ref to jP */;
+  JNIEnvCacheLine * const jc = S3Global_env_cache(env);
   int rc;
-  /**
-     TODO: convert pX depending on traceflag:
-
-     SQLITE_TRACE_STMT: String
-     SQLITE_TRACE_PROFILE: Long
-     others: null
-  */
   switch(traceflag){
-  case SQLITE_TRACE_STMT:
-    /* This is not _quite_ right: we're converting to MUTF-8.  It
-       should(?) suffice for purposes of tracing, though. */
-    jX = (*env)->NewStringUTF(env, (const char *)pX);
-    break;
-  case SQLITE_TRACE_PROFILE:
-    jX = (*env)->NewObject(env, pEcl->globalClassLong, pEcl->ctorLong1,
-                           (jlong)*((sqlite3_int64*)pX));
-    break;
+    case SQLITE_TRACE_STMT:
+      /* This is not _quite_ right: we're converting to MUTF-8.  It
+         should(?) suffice for purposes of tracing, though. */
+      jX = (*env)->NewStringUTF(env, (const char *)pX);
+      if(!jX) return SQLITE_NOMEM;
+      jP = jc->currentStmt;
+      break;
+    case SQLITE_TRACE_PROFILE:
+      jX = (*env)->NewObject(env, jc->globalClassLong, jc->ctorLong1,
+                             (jlong)*((sqlite3_int64*)pX));
+      // hmm. It really is zero.
+      // MARKER(("profile time = %llu\n", *((sqlite3_int64*)pX)));
+      jP = jc->currentStmt;
+      if(!jP){
+        jP = jPUnref = new_sqlite3_stmt_wrapper(env, pP);
+        if(!jP){
+          UNREF_L(jX);
+          return SQLITE_NOMEM;
+        }
+        MARKER(("WARNING: created new sqlite3_stmt wrapper for TRACE_PROFILE. stmt@%p\n"
+                "This means we have missed a route into the tracing API and it "
+                "needs the stmt_set_current() treatment littered around a handful "
+                "of other functions.\n", pP));
+      }
+      break;
+    case SQLITE_TRACE_ROW:
+      jP = jc->currentStmt;
+      break;
+    case SQLITE_TRACE_CLOSE:
+      jP = ps->jDb;
+      break;
+    default:
+      assert(!"cannot happen - unkown trace flag");
+      return SQLITE_ERROR;
   }
+  assert(jP);
   rc = (int)(*env)->CallIntMethod(env, ps->trace.jObj,
                                   ps->trace.midCallback,
-                                  (jint)traceflag, (jlong)pP, jX);
+                                  (jint)traceflag, jP, jX);
+  UNREF_L(jPUnref);
   UNREF_L(jX);
   IFTHREW{
-    EXCEPTION_CLEAR;
-    rc = s3jni_db_error(ps->pDb, SQLITE_ERROR,
-                        "sqlite3_trace_v2() callback threw.");
+    EXCEPTION_WARN_CALLBACK_THREW1("sqlite3_trace_v2() callback");
+    rc = SQLITE_ERROR;
   }
   return rc;
 }
@@ -2240,7 +2318,7 @@ JDECL(jint,1trace_1v2)(JENV_JSELF,jobject jDb, jint traceMask, jobject jTracer){
   if(!ps) return SQLITE_NOMEM;
   klazz = (*env)->GetObjectClass(env, jTracer);
   ps->trace.midCallback = (*env)->GetMethodID(env, klazz, "xCallback",
-                                              "(IJLjava/lang/Object;)I");
+                                              "(ILjava/lang/Object;Ljava/lang/Object;)I");
   IFTHREW {
     EXCEPTION_CLEAR;
     return s3jni_db_error(ps->pDb, SQLITE_ERROR,

diff --git a/ext/jni/src/org/sqlite/jni/Tester1.java b/ext/jni/src/org/sqlite/jni/Tester1.java
index c12245f68b6dad64ecab426b6d2cd6032a0ea774..23bd038348228ba00a253749ddd95387d274a3ac 100644 (file)
--- a/ext/jni/src/org/sqlite/jni/Tester1.java
+++ b/ext/jni/src/org/sqlite/jni/Tester1.java
@@ -644,7 +644,6 @@ public class Tester1 {
   }
 
   private static void listBoundMethods(){
-    //public static List<Field> getStatics(Class<?> clazz) {
     if(false){
       final java.lang.reflect.Field[] declaredFields =
         SQLite3Jni.class.getDeclaredFields();
@@ -682,26 +681,36 @@ public class Tester1 {
       db, SQLITE_TRACE_STMT | SQLITE_TRACE_PROFILE
           | SQLITE_TRACE_ROW | SQLITE_TRACE_CLOSE,
       new Tracer(){
-        public int xCallback(int traceFlag, long pNative, Object x){
+        public int xCallback(int traceFlag, Object pNative, Object x){
           ++counter.value;
-          //outln("Trace #"+counter.value+" flag="+traceFlag+": "+x);
           switch(traceFlag){
             case SQLITE_TRACE_STMT:
-              // pNative ==> sqlite3_stmt
-              affirm(x instanceof String); break;
+              affirm(pNative instanceof sqlite3_stmt);
+              affirm(x instanceof String);
+              //outln("TRACE_STMT sql = "+x);
+              break;
             case SQLITE_TRACE_PROFILE:
-              // pNative ==> sqlite3_stmt
-              affirm(x instanceof Long); break;
+              affirm(pNative instanceof sqlite3_stmt);
+              affirm(x instanceof Long);
+              //outln("TRACE_PROFILE time = "+x);
+              break;
             case SQLITE_TRACE_ROW:
-              // pNative ==> sqlite3_stmt
+              affirm(pNative instanceof sqlite3_stmt);
+              affirm(null == x);
+              //outln("TRACE_ROW = "+sqlite3_column_text((sqlite3_stmt)pNative, 0));
+              break;
             case SQLITE_TRACE_CLOSE:
-              // pNative ==> sqlite3
+              affirm(pNative instanceof sqlite3);
               affirm(null == x);
+              break;
+            default:
+              affirm(false /*cannot happen*/);
+              break;
           }
           return 0;
         }
       });
-    execSql(db, "SELECT 1; SELECT 2");
+    execSql(db, "SELECT coalesce(null,null,null,'hi'); SELECT 'world'");
     affirm( 6 == counter.value );
     sqlite3_close_v2(db);
     affirm( 7 == counter.value );

diff --git a/ext/jni/src/org/sqlite/jni/Tracer.java b/ext/jni/src/org/sqlite/jni/Tracer.java
index 52ccfabafc34c6dea0d9c4fef0a45c39adce5072..fa62edbfa77b758583987ea447b53acb0b93dfa1 100644 (file)
--- a/ext/jni/src/org/sqlite/jni/Tracer.java
+++ b/ext/jni/src/org/sqlite/jni/Tracer.java
@@ -18,6 +18,10 @@ package org.sqlite.jni;
 */
 public interface Tracer {
   /**
+     Achtung: this interface is subject to change because the current
+     approach to mapping the passed-in natives back to Java is
+     uncomfortably quirky.
+
      Called by sqlite3 for various tracing operations, as per
      sqlite3_trace_v2(). Note that this interface elides the 2nd
      argument to the native trace callback, as that role is better
@@ -37,10 +41,10 @@ public interface Tracer {
      depends on value of the first argument:
 
      - SQLITE_TRACE_STMT: pNative is a sqlite3_stmt. pX is a string
-       containing the prepared SQL, with one caveat/FIXME: JNI only
-       provides us with the ability to convert that string to MUTF-8,
-       as opposed to standard UTF-8, and is cannot be ruled out that
-       that difference may be significant for certain inputs. The
+       containing the prepared SQL, with one caveat: JNI only provides
+       us with the ability to convert that string to MUTF-8, as
+       opposed to standard UTF-8, and is cannot be ruled out that that
+       difference may be significant for certain inputs. The
        alternative would be that we first convert it to UTF-16 before
        passing it on, but there's no readily-available way to do that
        without calling back into the db to peform the conversion
@@ -54,5 +58,5 @@ public interface Tracer {
 
      - SQLITE_TRACE_CLOSE: pNative is a sqlite3. pX is null.
   */
-  int xCallback(int traceFlag, long pNative, Object pX);
+  int xCallback(int traceFlag, Object pNative, Object pX);
 }

diff --git a/manifest b/manifest
index 82ad29b0658e380d4bd39e73b53986c9376cde20..dcd27fee56dcf1333e5b9913825fb7fbff0d8bf8 100644 (file)
--- a/manifest
+++ b/manifest
@@ -1,5 +1,5 @@
-C Add\ssome\sJNI-internal\smetrics,\saccessible\svia\spassing\s-v\swhen\srunning\sTester1.java.\sDocument\san\sOpenJDK\sbug\swhich\sleads\sto\sincorrect\s-Xlint:jni\swarnings.
-D 2023-07-31T12:10:32.812
+C Experimentally\schange\sthe\sJNI\ssqlite3_trace_v2()\scallback\stype\sto\shave\smore\sconvenient\saccess\sto\sthe\scurrent\sJava-side\ssqlite3_stmt\sat\sthe\scost\sof\ssome\suncomfortably\sfiddly\scurrent-statement\stracking\sin\sthe\sJNI\slayer.\sSubject\sto\schange.
+D 2023-07-31T13:52:46.522
 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1
 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea
 F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724
@@ -232,7 +232,7 @@ F ext/icu/icu.c c074519b46baa484bb5396c7e01e051034da8884bad1a1cb7f09bbe6be3f0282
 F ext/icu/sqliteicu.h fa373836ed5a1ee7478bdf8a1650689294e41d0c89c1daab26e9ae78a32075a8
 F ext/jni/GNUmakefile 3d1f106e7a08bb54279c12979b31492b3dea702a732eab445dbc765120995182
 F ext/jni/README.md c0e6e80935e7761acead89b69c87765b23a6bcb2858c321c3d05681fd338292a
-F ext/jni/src/c/sqlite3-jni.c 7be564e523b576b130d930ed7ad3b389cf22372689101766c3a032166f4438a5
+F ext/jni/src/c/sqlite3-jni.c 4d680b84d7def871186e0787cbaa14347952c1c4ea05530440bb237a7a97886c
 F ext/jni/src/c/sqlite3-jni.h 74aaf87e77f99857aa3afc013517c934cbc2c16618c83d8f5d6294351bc8e7b1
 F ext/jni/src/org/sqlite/jni/BusyHandler.java 1b1d3e5c86cd796a0580c81b6af6550ad943baa25e47ada0dcca3aff3ebe978c
 F ext/jni/src/org/sqlite/jni/Collation.java 8dffbb00938007ad0967b2ab424d3c908413af1bbd3d212b9c9899910f1218d1
@@ -244,8 +244,8 @@ F ext/jni/src/org/sqlite/jni/ProgressHandler.java 5979450e996416d28543f1d42634d3
 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 b522c6456ab66026af5c487e4ac40410be36374d0550c2a03ea28421c4d39029
-F ext/jni/src/org/sqlite/jni/Tester1.java 13ca6badf8e79c39fe52c00306f55a1f55b6d504d8991132eca842e08eb2dc1e
-F ext/jni/src/org/sqlite/jni/Tracer.java c2fe1eba4a76581b93b375a7b95ab1919e5ae60accfb06d6beb067b033e9bae1
+F ext/jni/src/org/sqlite/jni/Tester1.java ee7ad9a45a282b12a5c2c5ab5f6fdb14a398f854f29cdeef457c81cceeddad16
+F ext/jni/src/org/sqlite/jni/Tracer.java a5cece9f947b0af27669b8baec300b6dd7ff859c3e6a6e4a1bd8b50f9714775d
 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 600c3ddc1ac28ee8f58669fb435fd0d21f2972c652039361fde907d4fe44eb58
@@ -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 9faca5d9ed4a749421e08bd1da8b7672c0fd31366124fdb613c46e19dece0fc1
-R 2f7ebe9a2d19552b91a45c55eec1bac7
+P a5d68a6b64abe3c2dfc3a32157f70fd8a4ad89feef2510b3bbb2d86b325d51ae
+R cb629a46215e7de4fde069b749d0908e
 U stephan
-Z 7f375306dd5a29931e39a5081c815b0c
+Z 8e74ea4537f0e72a50fc3ee99cfef378
 # Remove this line to create a well-formed Fossil manifest.

diff --git a/manifest.uuid b/manifest.uuid
index a908ec37fdee9be12e61c5f373e977ac37e552d3..bd9b7db410b1fe7e6deb6a43bbfb799afc671e5b 100644 (file)
--- a/manifest.uuid
+++ b/manifest.uuid
@@ -1 +1 @@
-a5d68a6b64abe3c2dfc3a32157f70fd8a4ad89feef2510b3bbb2d86b325d51ae
\ No newline at end of file
+459db332af6ea358b42bac096b9d26f1045b9ec32fad8463bca06807b2396b2c
\ No newline at end of file