From: stephan Date: Mon, 31 Jul 2023 13:52:46 +0000 (+0000) Subject: Experimentally change the JNI sqlite3_trace_v2() callback type to have more convenien... X-Git-Tag: version-3.43.0~47^2~114 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=db6f0bef9133f012e8a41ab11406cd1d7d819202;p=thirdparty%2Fsqlite.git Experimentally change the JNI sqlite3_trace_v2() callback type to have more convenient access to the current Java-side sqlite3_stmt at the cost of some uncomfortably fiddly current-statement tracking in the JNI layer. Subject to change. FossilOrigin-Name: 459db332af6ea358b42bac096b9d26f1045b9ec32fad8463bca06807b2396b2c --- diff --git a/ext/jni/src/c/sqlite3-jni.c b/ext/jni/src/c/sqlite3-jni.c index 088ae4f88b..ee389d4032 100644 --- 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 c12245f68b..23bd038348 100644 --- 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 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 52ccfabafc..fa62edbfa7 100644 --- 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 82ad29b065..dcd27fee56 100644 --- 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 a908ec37fd..bd9b7db410 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -a5d68a6b64abe3c2dfc3a32157f70fd8a4ad89feef2510b3bbb2d86b325d51ae \ No newline at end of file +459db332af6ea358b42bac096b9d26f1045b9ec32fad8463bca06807b2396b2c \ No newline at end of file