From 60aca33a8b8f8d8364ecaa6565128fc4c1f298fd Mon Sep 17 00:00:00 2001 From: stephan Date: Sun, 27 Aug 2023 08:10:59 +0000 Subject: [PATCH] Factor out a superfluous JNI class. Doc and code style cleanups. FossilOrigin-Name: 0f37f27148dfa93ecc42381ad3455a9059285d1af2df027429044942dc4d861b --- ext/jni/GNUmakefile | 5 +- ext/jni/src/c/sqlite3-jni.c | 113 +++++++++++++++++++----------------- manifest | 14 ++--- manifest.uuid | 2 +- 4 files changed, 70 insertions(+), 64 deletions(-) diff --git a/ext/jni/GNUmakefile b/ext/jni/GNUmakefile index 5f7038f051..f2252bc624 100644 --- a/ext/jni/GNUmakefile +++ b/ext/jni/GNUmakefile @@ -165,6 +165,7 @@ $(sqlite3.h): $(sqlite3.c): $(sqlite3.h) opt.threadsafe ?= 1 +opt.oom ?= 0 SQLITE_OPT = \ -DSQLITE_ENABLE_RTREE \ -DSQLITE_ENABLE_EXPLAIN_COMMENTS \ @@ -182,7 +183,7 @@ SQLITE_OPT = \ -DSQLITE_TEMP_STORE=2 \ -DSQLITE_USE_URI=1 \ -DSQLITE_C=$(sqlite3.c) \ - -DSQLITE_JNI_FATAL_OOM=0 \ + -DSQLITE_JNI_FATAL_OOM=$(opt.oom) \ -DSQLITE_DEBUG SQLITE_OPT += -g -DDEBUG -UNDEBUG @@ -252,7 +253,7 @@ test-sqllog: $(test.deps) $(bin.java) $(test.main.flags) -sqllog test-mt: $(test.deps) @echo "Testing in multi-threaded mode:"; - $(bin.java) $(test.main.flags) -t 5 -r 20 -shuffle $(test.flags) + $(bin.java) $(test.main.flags) -t 11 -r 50 -shuffle $(test.flags) test: test-one test-mt tests: test test-sqllog diff --git a/ext/jni/src/c/sqlite3-jni.c b/ext/jni/src/c/sqlite3-jni.c index d219039d30..5adc2f626e 100644 --- a/ext/jni/src/c/sqlite3-jni.c +++ b/ext/jni/src/c/sqlite3-jni.c @@ -480,13 +480,12 @@ struct S3JniEnv { }; /* -** State for proxying sqlite3_auto_extension() in Java. +** State for proxying sqlite3_auto_extension() in Java. This was +** initially a separate class from S3JniHook and now the older name is +** retained for readability in the APIs which use this, as well as for +** its better code-searchability. */ -typedef struct S3JniAutoExtension S3JniAutoExtension; -struct S3JniAutoExtension { - jobject jObj /* Java object */; - jmethodID midFunc /* xEntryPoint() callback */; -}; +typedef S3JniHook S3JniAutoExtension; /* ** Type IDs for SQL function categories. @@ -1366,15 +1365,10 @@ static S3JniDb * S3JniDb_get(JNIEnv * const env, jobject jDb, sqlite3 *pDb){ #define S3JniDb_from_c(sqlite3Ptr) S3JniDb_get(env,0,(sqlite3Ptr)) /* -** Unref any Java-side state in ax and zero out ax. +** Unref any Java-side state in (S3JniAutoExtension*) AX and zero out +** AX. */ -static void S3JniAutoExtension_clear(JNIEnv * const env, - S3JniAutoExtension * const ax){ - if( ax->jObj ){ - S3JniUnrefGlobal(ax->jObj); - memset(ax, 0, sizeof(*ax)); - } -} +#define S3JniAutoExtension_clear(AX) S3JniHook_unref(env, AX, 0); /* ** Initializes a pre-allocated S3JniAutoExtension object. Returns @@ -1388,12 +1382,12 @@ static int S3JniAutoExtension_init(JNIEnv *const env, jclass const klazz = (*env)->GetObjectClass(env, jAutoExt); S3JniMutex_Ext_assertLocker; - ax->midFunc = (*env)->GetMethodID(env, klazz, "call", - "(Lorg/sqlite/jni/sqlite3;)I"); + ax->midCallback = (*env)->GetMethodID(env, klazz, "call", + "(Lorg/sqlite/jni/sqlite3;)I"); S3JniUnrefLocal(klazz); S3JniExceptionWarnIgnore; - if( !ax->midFunc ){ - S3JniAutoExtension_clear(env, ax); + if( !ax->midCallback ){ + S3JniAutoExtension_clear(ax); return SQLITE_ERROR; } ax->jObj = S3JniRefGlobal(jAutoExt); @@ -1629,12 +1623,23 @@ static inline jobject new_sqlite3_value_wrapper(JNIEnv * const env, sqlite3_valu return new_NativePointerHolder_object(env, &S3NphRefs.sqlite3_value, sv); } +/* Helper typedefs for UDF callback types. */ typedef void (*udf_xFunc_f)(sqlite3_context*,int,sqlite3_value**); typedef void (*udf_xStep_f)(sqlite3_context*,int,sqlite3_value**); typedef void (*udf_xFinal_f)(sqlite3_context*); /*typedef void (*udf_xValue_f)(sqlite3_context*);*/ /*typedef void (*udf_xInverse_f)(sqlite3_context*,int,sqlite3_value**);*/ +/* +** Allocate a new S3JniUdf (User-defined Function) and associate it +** with the SQLFunction-type jObj. Returns NULL on OOM. If the +** returned object's type==UDF_UNKNOWN_TYPE then the type of UDF was +** not unambiguously detected based on which callback members it has, +** which falls into the category of user error. +** +** The caller must arrange for the returned object to eventually be +** passed to S3JniUdf_free(). +*/ static S3JniUdf * S3JniUdf_alloc(JNIEnv * const env, jobject jObj){ S3JniUdf * s = 0; @@ -1660,15 +1665,18 @@ static S3JniUdf * S3JniUdf_alloc(JNIEnv * const env, jobject jObj){ memset(s, 0, sizeof(*s)); s->jObj = S3JniRefGlobal(jObj); -#define FGET(FuncName,FuncType,Field) \ - s->Field = (*env)->GetMethodID(env, klazz, FuncName, FuncType); \ + +#define FGET(FuncName,FuncSig,Field) \ + s->Field = (*env)->GetMethodID(env, klazz, FuncName, FuncSig); \ if( !s->Field ) (*env)->ExceptionClear(env) + FGET("xFunc", zFSI, jmidxFunc); FGET("xStep", zFSI, jmidxStep); FGET("xFinal", zFV, jmidxFinal); FGET("xValue", zFV, jmidxValue); FGET("xInverse", zFSI, jmidxInverse); #undef FGET + S3JniUnrefLocal(klazz); if( s->jmidxFunc ) s->type = UDF_SCALAR; else if( s->jmidxStep && s->jmidxFinal ){ @@ -1683,6 +1691,7 @@ static S3JniUdf * S3JniUdf_alloc(JNIEnv * const env, jobject jObj){ static void S3JniUdf_free(S3JniUdf * s){ S3JniDeclLocal_env; + s3jni_call_xDestroy(env, s->jObj); S3JniUnrefGlobal(s->jObj); sqlite3_free(s->zFuncName); @@ -1698,13 +1707,13 @@ static void S3JniUdf_finalizer(void * s){ S3JniUdf_free((S3JniUdf*)s); } -/** - Helper for processing args to UDF handlers - with signature (sqlite3_context*,int,sqlite3_value**). +/* +** Helper for processing args to UDF handlers with signature +** (sqlite3_context*,int,sqlite3_value**). */ typedef struct { - jobject jcx; - jobjectArray jargv; + jobject jcx /* sqlite3_context object */; + jobjectArray jargv /* sqlite3_value[] */; } udf_jargs; /** @@ -1778,10 +1787,7 @@ static int udf_report_exception(JNIEnv * const env, int translateToErr, rc = SQLITE_NOMEM; } }else{ - MARKER(("Client-defined SQL function %s.%s() threw. " - "It should not do that.\n", - zFuncName ? zFuncName : "", zFuncType)); - (*env)->ExceptionDescribe( env ); + S3JniExceptionWarnCallbackThrew("Client-defined SQL function"); S3JniExceptionClear; } S3JniUnrefLocal(ex); @@ -1793,10 +1799,8 @@ static int udf_report_exception(JNIEnv * const env, int translateToErr, ** UDF, calls it, and returns 0 on success. */ static int udf_xFSI(sqlite3_context* const pCx, int argc, - sqlite3_value** const argv, - S3JniUdf * const s, - jmethodID xMethodID, - const char * const zFuncType){ + sqlite3_value** const argv, S3JniUdf * const s, + jmethodID xMethodID, const char * const zFuncType){ S3JniDeclLocal_env; udf_jargs args = {0,0}; int rc = udf_args(env, pCx, argc, argv, &args.jcx, &args.jargv); @@ -1824,16 +1828,17 @@ static int udf_xFV(sqlite3_context* cx, S3JniUdf * s, jobject jcx = new_sqlite3_context_wrapper(env, cx); int rc = 0; int const isFinal = 'F'==zFuncType[1]/*xFinal*/; - if( !jcx ){ + if( jcx ){ + (*env)->CallVoidMethod(env, s->jObj, xMethodID, jcx); + S3JniIfThrew{ + rc = udf_report_exception(env, isFinal, cx, s->zFuncName, + zFuncType); + } + S3JniUnrefLocal(jcx); + }else{ if( isFinal ) sqlite3_result_error_nomem(cx); - return SQLITE_NOMEM; - } - (*env)->CallVoidMethod(env, s->jObj, xMethodID, jcx); - S3JniIfThrew{ - rc = udf_report_exception(env, isFinal, cx, s->zFuncName, - zFuncType); + rc = SQLITE_NOMEM; } - S3JniUnrefLocal(jcx); return rc; } @@ -1981,7 +1986,6 @@ WRAP_INT_SVALUE(1value_1type, sqlite3_value_type) #undef WRAP_MUTF8_VOID #undef WRAP_STR_STMT_INT - S3JniApi(sqlite3_aggregate_context(),jlong,1aggregate_1context)( JniArgsEnvClass, jobject jCx, jboolean initialize ){ @@ -1994,7 +1998,6 @@ S3JniApi(sqlite3_aggregate_context(),jlong,1aggregate_1context)( return (jlong)p; } - /* Central auto-extension handler. */ static int s3jni_run_java_auto_extensions(sqlite3 *pDb, const char **pzErr, const struct sqlite3_api_routines *ignored){ @@ -2009,15 +2012,16 @@ static int s3jni_run_java_auto_extensions(sqlite3 *pDb, const char **pzErr, jc = S3JniEnv_get(env); ps = jc->pdbOpening; if( !ps ){ - MARKER(("Unexpected arrival of null S3JniDb in auto-extension runner.\n")); - *pzErr = sqlite3_mprintf("Unexpected arrival of null S3JniDb in auto-extension runner."); + *pzErr = sqlite3_mprintf("Unexpected arrival of null S3JniDb in " + "auto-extension runner."); return SQLITE_ERROR; } jc->pdbOpening = 0; assert( !ps->pDb && "it's still being opened" ); ps->pDb = pDb; assert( ps->jDb ); - NativePointerHolder_set(env, &S3NphRefs.sqlite3, ps->jDb, pDb); + NativePointerHolder_set(env, &S3NphRefs.sqlite3, ps->jDb, pDb) + /* As of here, the Java/C connection is complete */; for( i = 0; go && 0==rc; ++i ){ S3JniAutoExtension ax = {0,0} /* We need a copy of the auto-extension object, with our own @@ -2029,11 +2033,11 @@ static int s3jni_run_java_auto_extensions(sqlite3 *pDb, const char **pzErr, go = 0; }else{ ax.jObj = S3JniRefLocal(SJG.autoExt.aExt[i].jObj); - ax.midFunc = SJG.autoExt.aExt[i].midFunc; + ax.midCallback = SJG.autoExt.aExt[i].midCallback; } S3JniMutex_Ext_leave; if( ax.jObj ){ - rc = (*env)->CallIntMethod(env, ax.jObj, ax.midFunc, ps->jDb); + rc = (*env)->CallIntMethod(env, ax.jObj, ax.midCallback, ps->jDb); S3JniUnrefLocal(ax.jObj); S3JniIfThrew { jthrowable const ex = (*env)->ExceptionOccurred(env); @@ -2084,8 +2088,8 @@ S3JniApi(sqlite3_auto_extension(),jint,1auto_1extension)( if( 0==rc ){ ax = &SJG.autoExt.aExt[SJG.autoExt.nExt]; rc = S3JniAutoExtension_init(env, ax, jAutoExt); - assert( rc ? (0==ax->jObj && 0==ax->midFunc) - : (0!=ax->jObj && 0!=ax->midFunc) ); + assert( rc ? (0==ax->jObj && 0==ax->midCallback) + : (0!=ax->jObj && 0!=ax->midCallback) ); } } if( 0==rc ){ @@ -2093,7 +2097,7 @@ S3JniApi(sqlite3_auto_extension(),jint,1auto_1extension)( rc = sqlite3_auto_extension( (void(*)(void))s3jni_run_java_auto_extensions ); if( rc ){ assert( ax ); - S3JniAutoExtension_clear(env, ax); + S3JniAutoExtension_clear(ax); } } if( 0==rc ){ @@ -2273,7 +2277,7 @@ S3JniApi(sqlite3_cancel_auto_extension(),jboolean,1cancel_1auto_1extension)( for( i = SJG.autoExt.nExt-1; i >= 0; --i ){ ax = &SJG.autoExt.aExt[i]; if( ax->jObj && (*env)->IsSameObject(env, ax->jObj, jAutoExt) ){ - S3JniAutoExtension_clear(env, ax); + S3JniAutoExtension_clear(ax); /* Move final entry into this slot. */ --SJG.autoExt.nExt; *ax = SJG.autoExt.aExt[SJG.autoExt.nExt]; @@ -3044,7 +3048,8 @@ static int s3jni_open_post(JNIEnv * const env, S3JniEnv * const jc, assert(ps->jDb); if( 0==ps->pDb ){ ps->pDb = *ppDb; - NativePointerHolder_set(env, &S3NphRefs.sqlite3, ps->jDb, *ppDb); + NativePointerHolder_set(env, &S3NphRefs.sqlite3, ps->jDb, *ppDb) + /* As of here, the Java/C connection is complete */; }else{ assert( ps->pDb == *ppDb /* set up via s3jni_run_java_auto_extensions() */); } @@ -3452,7 +3457,7 @@ static void s3jni_reset_auto_extension(JNIEnv *env){ int i; S3JniMutex_Ext_enter; for( i = 0; i < SJG.autoExt.nExt; ++i ){ - S3JniAutoExtension_clear( env, &SJG.autoExt.aExt[i] ); + S3JniAutoExtension_clear( &SJG.autoExt.aExt[i] ); } SJG.autoExt.nExt = 0; S3JniMutex_Ext_leave; diff --git a/manifest b/manifest index 04b1dafb47..7aac56b14d 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C JNI\scode\sreorgs\sand\ssimplify\sthe\sfailing-alloc\sinterface\sa\sbit. -D 2023-08-27T07:26:33.055 +C Factor\sout\sa\ssuperfluous\sJNI\sclass.\sDoc\sand\scode\sstyle\scleanups. +D 2023-08-27T08:10:59.533 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724 @@ -233,10 +233,10 @@ F ext/fts5/tool/showfts5.tcl d54da0e067306663e2d5d523965ca487698e722c F ext/icu/README.txt 7ab7ced8ae78e3a645b57e78570ff589d4c672b71370f5aa9e1cd7024f400fc9 F ext/icu/icu.c c074519b46baa484bb5396c7e01e051034da8884bad1a1cb7f09bbe6be3f0282 F ext/icu/sqliteicu.h fa373836ed5a1ee7478bdf8a1650689294e41d0c89c1daab26e9ae78a32075a8 -F ext/jni/GNUmakefile 4e60cdca419ac6783719da98379480b6f04d5d1b5fa1408c46fcb0c32565c571 +F ext/jni/GNUmakefile 05a756bb7a579b7d6570cb590567e9f0d12270529a2e7e50523284e5a3684838 F ext/jni/README.md 1332b1fa27918bd5d9ca2d0d4f3ac3a6ab86b9e3699dc5bfe32904a027f3d2a9 F ext/jni/jar-dist.make 030aaa4ae71dd86e4ec5e7c1e6cd86f9dfa47c4592c070d2e35157e42498e1fa -F ext/jni/src/c/sqlite3-jni.c 98c483b32aaec515573d0cb1293010713954639f9279309a50cd4189eaf118c8 +F ext/jni/src/c/sqlite3-jni.c 9a07c95d570c1a46fff1db5383d0d247ef946b8c010a8e392045e350bf22e28f F ext/jni/src/c/sqlite3-jni.h a410d05ca47a676b75ff7b8980e75ad604ea15f3c29965f88989703abc2eeaf6 F ext/jni/src/org/sqlite/jni/AggregateFunction.java 0a5a74bea5ee12a99407e9432d0ca393525af912c2b0ca55c7ee5dbd019c00ef F ext/jni/src/org/sqlite/jni/AuthorizerCallback.java c374bb76409cce7a0bdba94877706b59ac6127fa5d9e6af3e8058c99ce99c030 @@ -2103,8 +2103,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 1ff78582bfd934e0c76464b5f23ed9bf09a3491b145e0ca34acb6e59c4f53995 -R 5568414903d2c4b26699319473e5e738 +P deed5797de65a25896e991a441f0d05eb92662536296485920fb081e84ad5d32 +R 1286cf04d67bcef64c621ffe2a87fc44 U stephan -Z 3cd78dd857d13aa12cd9cebc268e8f6f +Z 113b1efbfce3141a69775192a9b6c79b # Remove this line to create a well-formed Fossil manifest. diff --git a/manifest.uuid b/manifest.uuid index 666e786af9..b64486e811 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -deed5797de65a25896e991a441f0d05eb92662536296485920fb081e84ad5d32 \ No newline at end of file +0f37f27148dfa93ecc42381ad3455a9059285d1af2df027429044942dc4d861b \ No newline at end of file -- 2.47.2