diff --git a/ext/jni/GNUmakefile b/ext/jni/GNUmakefile index dd8df07bc4..5f7038f051 100644 --- a/ext/jni/GNUmakefile +++ b/ext/jni/GNUmakefile @@ -182,7 +182,7 @@ SQLITE_OPT = \ -DSQLITE_TEMP_STORE=2 \ -DSQLITE_USE_URI=1 \ -DSQLITE_C=$(sqlite3.c) \ - -DSQLITE_JNI_FATAL_OOM=1 \ + -DSQLITE_JNI_FATAL_OOM=0 \ -DSQLITE_DEBUG SQLITE_OPT += -g -DDEBUG -UNDEBUG diff --git a/ext/jni/src/c/sqlite3-jni.c b/ext/jni/src/c/sqlite3-jni.c index c210d298a0..d219039d30 100644 --- a/ext/jni/src/c/sqlite3-jni.c +++ b/ext/jni/src/c/sqlite3-jni.c @@ -216,19 +216,73 @@ } /* -** Helpers for extracting pointers from jobjects, noting that we rely -** on the corresponding Java interfaces having already done the -** type-checking. Don't use these in contexts where that's not the -** case. +** Declares local var env = s3jni_env(). All JNI calls involve a +** JNIEnv somewhere, always named env, and many of our macros assume +** env is in scope. */ -#define PtrGet_T(T,OBJ) NativePointerHolder_get(env, OBJ, &S3NphRefs.T) -#define PtrGet_sqlite3(OBJ) PtrGet_T(sqlite3, OBJ) -#define PtrGet_sqlite3_stmt(OBJ) PtrGet_T(sqlite3_stmt, OBJ) -#define PtrGet_sqlite3_value(OBJ) PtrGet_T(sqlite3_value, OBJ) -#define PtrGet_sqlite3_context(OBJ) PtrGet_T(sqlite3_context, OBJ) +#define S3JniDeclLocal_env JNIEnv * const env = s3jni_env() + +/* Fail fatally with an OOM message. */ +static inline void s3jni_oom(JNIEnv * const env){ + (*env)->FatalError(env, "SQLite3 JNI is out of memory.") /* does not return */; +} + +/* +** sqlite3_malloc() proxy which fails fatally on OOM. This should +** only be used for routines which manage global state and have no +** recovery strategy for OOM. For sqlite3 API which can reasonably +** return SQLITE_NOMEM, s3jni_malloc() should be used instead. +*/ +static void * s3jni_malloc_or_die(JNIEnv * const env, size_t n){ + void * const rv = sqlite3_malloc(n); + if( n && !rv ) s3jni_oom(env); + return rv; +} + +/* +** Works like sqlite3_malloc() unless built with SQLITE_JNI_FATAL_OOM, +** in which case it calls s3jni_oom() on OOM. +*/ +#ifdef SQLITE_JNI_FATAL_OOM +#define s3jni_malloc(SIZE) s3jni_malloc_or_die(env, SIZE) +#else +#define s3jni_malloc(SIZE) sqlite3_malloc(((void)env,(SIZE))) +#endif + +/* +** Works like sqlite3_realloc() unless built with SQLITE_JNI_FATAL_OOM, +** in which case it calls s3jni_oom() on OOM. +*/ +#ifdef SQLITE_JNI_FATAL_OOM +static void * s3jni_realloc_or_die(JNIEnv * const env, void * p, size_t n){ + void * const rv = sqlite3_realloc(p, (int)n); + if( n && !rv ) s3jni_oom(env); + return rv; +} +#define s3jni_realloc(MEM,SIZE) s3jni_realloc_or_die(env, (MEM), (SIZE)) +#else +#define s3jni_realloc(MEM,SIZE) sqlite3_realloc((MEM), ((void)env, (SIZE))) +#endif + +/* Fail fatally if !EXPR. */ +#define s3jni_oom_fatal(EXPR) if( !(EXPR) ) s3jni_oom(env) +/* Maybe fail fatally if !EXPR. */ +#ifdef SQLITE_JNI_FATAL_OOM +#define s3jni_oom_check s3jni_oom_fatal +#else +#define s3jni_oom_check(EXPR) +#endif + /* Helpers for Java value reference management. */ -static inline jobject s3jni_ref_global(JNIEnv * const env, jobject const v){ - return v ? (*env)->NewGlobalRef(env, v) : NULL; +static jobject s3jni_ref_global(JNIEnv * const env, jobject const v){ + jobject const rv = v ? (*env)->NewGlobalRef(env, v) : NULL; + s3jni_oom_fatal( v ? !!rv : 1 ); + return rv; +} +static jobject s3jni_ref_local(JNIEnv * const env, jobject const v){ + jobject const rv = v ? (*env)->NewLocalRef(env, v) : NULL; + s3jni_oom_fatal( v ? !!rv : 1 ); + return rv; } static inline void s3jni_unref_global(JNIEnv * const env, jobject const v){ if( v ) (*env)->DeleteGlobalRef(env, v); @@ -237,7 +291,7 @@ static inline void s3jni_unref_local(JNIEnv * const env, jobject const v){ if( v ) (*env)->DeleteLocalRef(env, v); } #define S3JniRefGlobal(VAR) s3jni_ref_global(env, (VAR)) -#define S3JniRefLocal(VAR) (*env)->NewLocalRef(env, (VAR)) +#define S3JniRefLocal(VAR) s3jni_ref_local(env, (VAR)) #define S3JniUnrefGlobal(VAR) s3jni_unref_global(env, (VAR)) #define S3JniUnrefLocal(VAR) s3jni_unref_local(env, (VAR)) @@ -472,7 +526,7 @@ struct S3JniUdf { ** else it isn't. */ #ifdef SQLITE_DEBUG -# define S3JNI_METRICS_MUTEX 1 +# define S3JNI_METRICS_MUTEX SQLITE_THREADSAFE #else # define S3JNI_METRICS_MUTEX 0 #endif @@ -552,16 +606,16 @@ struct S3JniGlobalType { org.sqlite.jni.auto_extension objects). */ struct { - S3JniAutoExtension *pExt /* The auto-extension list. It is + S3JniAutoExtension *aExt /* The auto-extension list. It is maintained such that all active entries are in the first contiguous nExt array elements. */; - int nAlloc /* number of entries allocated for pExt, + int nAlloc /* number of entries allocated for aExt, as distinct from the number of active entries. */; - int nExt /* number of active entries in pExt, all in the + int nExt /* number of active entries in aExt, all in the first nExt'th array elements. */; - sqlite3_mutex * mutex /* mutex for manipulation/traversal of pExt */; + sqlite3_mutex * mutex /* mutex for manipulation/traversal of aExt */; const void * locker /* object on whose behalf the mutex is held. Only for sanity checking in debug builds. */; } autoExt; @@ -618,6 +672,26 @@ struct S3JniGlobalType { static S3JniGlobalType S3JniGlobal = {}; #define SJG S3JniGlobal +/* +** Helpers for extracting pointers from jobjects, noting that we rely +** on the corresponding Java interfaces having already done the +** type-checking. OBJ must be a jobject referring to a +** NativePointerHolder, where T matches PtrGet_T. Don't use these +** in contexts where that's not the case. Note that these aren't +** type-safe in the strictest sense: +** +** sqlite3 * s = PtrGet_sqlite3_stmt(...) +** +** will work, despite the incorrect macro name, so long as the +** argument is a Java sqlite3 object, as this operation only has void +** pointers to work with. +*/ +#define PtrGet_T(T,OBJ) NativePointerHolder_get(env, OBJ, &S3NphRefs.T) +#define PtrGet_sqlite3(OBJ) PtrGet_T(sqlite3, OBJ) +#define PtrGet_sqlite3_stmt(OBJ) PtrGet_T(sqlite3_stmt, OBJ) +#define PtrGet_sqlite3_value(OBJ) PtrGet_T(sqlite3_value, OBJ) +#define PtrGet_sqlite3_context(OBJ) PtrGet_T(sqlite3_context, OBJ) + /* Increments *p, possibly protected by a mutex. */ #ifndef SQLITE_JNI_ENABLE_METRICS #define s3jni_incr(PTR) @@ -706,20 +780,6 @@ static void s3jni_incr( volatile unsigned int * const p ){ #define S3JniMutex_S3JniDb_leave #endif -/* Fail fatally with an OOM message. */ -static inline void s3jni_oom(JNIEnv * const env){ - (*env)->FatalError(env, "Out of memory.") /* does not return */; -} - -/* Fail fatally if !EXPR. */ -#define s3jni_oom_fatal(EXPR) if( !(EXPR) ) s3jni_oom(env) -/* Maybe fail fatally if !EXPR. */ -#ifdef SQLITE_JNI_FATAL_OOM -#define s3jni_oom_check s3jni_oom_fatal -#else -#define s3jni_oom_check(EXPR) -#endif - /* Helpers for jstring and jbyteArray. */ static const char * s3jni__jstring_to_mutf8_bytes(JNIEnv * const env, jstring v ){ const char *z = v ? (*env)->GetStringUTFChars(env, v, NULL) : 0; @@ -740,43 +800,6 @@ static jbyte * s3jni__jbytearray_bytes(JNIEnv * const env, jbyteArray jBA ){ #define s3jni_jbytearray_release(jByteArray,jBytes) \ if( jBytes ) (*env)->ReleaseByteArrayElements(env, jByteArray, jBytes, JNI_ABORT) -/* -** sqlite3_malloc() proxy which fails fatally on OOM. This should -** only be used for routines which manage global state and have no -** recovery strategy for OOM. For sqlite3 API which can reasonably -** return SQLITE_NOMEM, s3jni_malloc() should be used instead. -*/ -static void * s3jni_malloc_or_die(JNIEnv * const env, size_t n){ - void * const rv = sqlite3_malloc(n); - if( n && !rv ) s3jni_oom(env); - return rv; -} - -/* -** Works like sqlite3_malloc() unless built with SQLITE_JNI_FATAL_OOM, -** in which case it calls s3jni_oom() on OOM. -*/ -static void * s3jni_malloc(JNIEnv * const env, size_t n){ - void * const rv = sqlite3_malloc(n); -#ifdef SQLITE_JNI_FATAL_OOM - if( n && !rv ) s3jni_oom(env); -#endif - return rv; -} - -/* -** Works like sqlite3_realloc() unless built with SQLITE_JNI_FATAL_OOM, -** in which case it calls s3jni_oom() on OOM. -*/ -static void * s3jni_realloc(JNIEnv * const env, void * p, size_t n){ - void * const rv = sqlite3_realloc(p, (int)n); -#ifdef SQLITE_JNI_FATAL_OOM - if( n && !rv ) s3jni_oom(env); -#endif - return rv; -} - - /* ** Returns the current JNIEnv object. Fails fatally if it cannot find ** the object. @@ -790,8 +813,6 @@ static JNIEnv * s3jni_env(void){ } return env; } -/* Declares local var env = s3jni_env(). */ -#define S3JniDeclLocal_env JNIEnv * const env = s3jni_env() /* ** Fetches the S3JniGlobal.envCache row for the given env, allocing a @@ -948,7 +969,7 @@ static char * s3jni_jstring_to_utf8(JNIEnv * const env, } nBa = (*env)->GetArrayLength(env, jba); if( nLen ) *nLen = (int)nBa; - rv = s3jni_malloc( env, nBa + 1 ); + rv = s3jni_malloc( nBa + 1 ); if( rv ){ (*env)->GetByteArrayRegion(env, jba, 0, nBa, (jbyte*)rv); rv[nBa] = 0; @@ -1286,7 +1307,7 @@ static S3JniDb * S3JniDb_alloc(JNIEnv * const env, jobject jDb){ } s3jni_incr( &SJG.metrics.nPdbRecycled ); }else{ - rv = s3jni_malloc(env, sizeof(S3JniDb)); + rv = s3jni_malloc( sizeof(S3JniDb)); if( rv ){ memset(rv, 0, sizeof(S3JniDb)); s3jni_incr( &SJG.metrics.nPdbAlloc ); @@ -1545,7 +1566,7 @@ static void CollationState_xDestroy(void *pArg){ S3JniDeclLocal_env; S3JniMutex_S3JniDb_enter; - S3JniHook_unref( s3jni_env(), &ps->hooks.collation, 1 ); + S3JniHook_unref( env, &ps->hooks.collation, 1 ); S3JniMutex_S3JniDb_leave; } @@ -1627,7 +1648,7 @@ static S3JniUdf * S3JniUdf_alloc(JNIEnv * const env, jobject jObj){ } S3JniMutex_Global_leave; if( !s ){ - s = s3jni_malloc(env, sizeof(*s)); + s = s3jni_malloc( sizeof(*s)); s3jni_incr(&SJG.metrics.nUdfAlloc); } if( s ){ @@ -2007,8 +2028,8 @@ static int s3jni_run_java_auto_extensions(sqlite3 *pDb, const char **pzErr, if( i >= SJG.autoExt.nExt ){ go = 0; }else{ - ax.jObj = S3JniRefLocal(SJG.autoExt.pExt[i].jObj); - ax.midFunc = SJG.autoExt.pExt[i].midFunc; + ax.jObj = S3JniRefLocal(SJG.autoExt.aExt[i].jObj); + ax.midFunc = SJG.autoExt.aExt[i].midFunc; } S3JniMutex_Ext_leave; if( ax.jObj ){ @@ -2041,7 +2062,7 @@ S3JniApi(sqlite3_auto_extension(),jint,1auto_1extension)( S3JniMutex_Ext_enter; for( i = 0; i < SJG.autoExt.nExt; ++i ){ /* Look for match or first empty slot. */ - ax = &SJG.autoExt.pExt[i]; + ax = &SJG.autoExt.aExt[i]; if( ax->jObj && (*env)->IsSameObject(env, ax->jObj, jAutoExt) ){ S3JniMutex_Ext_leave; return 0 /* this as a no-op. */; @@ -2052,16 +2073,16 @@ S3JniApi(sqlite3_auto_extension(),jint,1auto_1extension)( if( SJG.autoExt.nExt == SJG.autoExt.nAlloc ){ unsigned n = 1 + SJG.autoExt.nAlloc; S3JniAutoExtension * const aNew = - s3jni_realloc( env, SJG.autoExt.pExt, n * sizeof(*ax) ); + s3jni_realloc( SJG.autoExt.aExt, n * sizeof(*ax) ); if( !aNew ){ rc = SQLITE_NOMEM; }else{ - SJG.autoExt.pExt = aNew; + SJG.autoExt.aExt = aNew; ++SJG.autoExt.nAlloc; } } if( 0==rc ){ - ax = &SJG.autoExt.pExt[SJG.autoExt.nExt]; + 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) ); @@ -2250,15 +2271,15 @@ S3JniApi(sqlite3_cancel_auto_extension(),jboolean,1cancel_1auto_1extension)( S3JniMutex_Ext_enter; /* This algo mirrors the one in the core. */ for( i = SJG.autoExt.nExt-1; i >= 0; --i ){ - ax = &SJG.autoExt.pExt[i]; + ax = &SJG.autoExt.aExt[i]; if( ax->jObj && (*env)->IsSameObject(env, ax->jObj, jAutoExt) ){ S3JniAutoExtension_clear(env, ax); /* Move final entry into this slot. */ --SJG.autoExt.nExt; - *ax = SJG.autoExt.pExt[SJG.autoExt.nExt]; - memset(&SJG.autoExt.pExt[SJG.autoExt.nExt], 0, + *ax = SJG.autoExt.aExt[SJG.autoExt.nExt]; + memset(&SJG.autoExt.aExt[SJG.autoExt.nExt], 0, sizeof(S3JniAutoExtension)); - assert(! SJG.autoExt.pExt[SJG.autoExt.nExt].jObj ); + assert(! SJG.autoExt.aExt[SJG.autoExt.nExt].jObj ); rc = JNI_TRUE; break; } @@ -2430,7 +2451,6 @@ S3JniApi(sqlite3_column_value(),jobject,1column_1value)( return new_sqlite3_value_wrapper(env, sv); } - static int s3jni_commit_rollback_hook_impl(int isCommit, S3JniDb * const ps){ S3JniDeclLocal_env; @@ -3432,7 +3452,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.pExt[i] ); + S3JniAutoExtension_clear( env, &SJG.autoExt.aExt[i] ); } SJG.autoExt.nExt = 0; S3JniMutex_Ext_leave; @@ -4140,7 +4160,7 @@ static void Fts5JniAux_xDestroy(void *p){ } static Fts5JniAux * Fts5JniAux_alloc(JNIEnv * const env, jobject jObj){ - Fts5JniAux * s = s3jni_malloc(env, sizeof(Fts5JniAux)); + Fts5JniAux * s = s3jni_malloc( sizeof(Fts5JniAux)); if( s ){ jclass klazz; @@ -4565,7 +4585,7 @@ JniDeclFtsXA(int,xSetAuxdata)(JniArgsEnvObj,jobject jCtx, jobject jAux){ int rc; S3JniFts5AuxData * pAux; - pAux = s3jni_malloc(env, sizeof(*pAux)); + pAux = s3jni_malloc( sizeof(*pAux)); if( !pAux ){ if( jAux ){ /* Emulate how xSetAuxdata() behaves when it cannot alloc @@ -4721,7 +4741,7 @@ static void SQLTester_dup_func( S3JniDeclLocal_env; ++p->nDup; - if( n>0 && (pOut = s3jni_malloc( env, (n+16)&~7 ))!=0 ){ + if( n>0 && (pOut = s3jni_malloc( (n+16)&~7 ))!=0 ){ pOut[0] = 0x2bbf4b7c; z = (char*)&pOut[1]; memcpy(z, sqlite3_value_text(argv[0]), n); diff --git a/manifest b/manifest index 3e1f473740..04b1dafb47 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Apply\sthe\sJNI\sOOM\schecks\sto\smemory\sreturned\sby\sJDK\sAPIs,\sas\sdistinct\sfrom\sour\sAPIs. -D 2023-08-26T22:34:26.393 +C JNI\scode\sreorgs\sand\ssimplify\sthe\sfailing-alloc\sinterface\sa\sbit. +D 2023-08-27T07:26:33.055 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 c1893766d909a9748151a0d52a3c18eb6aa37d2aaba187eb5fe45df344f96d2a +F ext/jni/GNUmakefile 4e60cdca419ac6783719da98379480b6f04d5d1b5fa1408c46fcb0c32565c571 F ext/jni/README.md 1332b1fa27918bd5d9ca2d0d4f3ac3a6ab86b9e3699dc5bfe32904a027f3d2a9 F ext/jni/jar-dist.make 030aaa4ae71dd86e4ec5e7c1e6cd86f9dfa47c4592c070d2e35157e42498e1fa -F ext/jni/src/c/sqlite3-jni.c 9440a767fd1019cffec368b3542af0c0a8c7c6da1a7c6f285dc19e07f670ced8 +F ext/jni/src/c/sqlite3-jni.c 98c483b32aaec515573d0cb1293010713954639f9279309a50cd4189eaf118c8 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 daede0f801f59d6501a863c4688e4635b34171e98b56b8ab4432c779113f1997 -R 6bd1f56d9b8c83901a03e2a9ce4434ac +P 1ff78582bfd934e0c76464b5f23ed9bf09a3491b145e0ca34acb6e59c4f53995 +R 5568414903d2c4b26699319473e5e738 U stephan -Z 362c439d994aeafc4f637309d1fc43bd +Z 3cd78dd857d13aa12cd9cebc268e8f6f # Remove this line to create a well-formed Fossil manifest. diff --git a/manifest.uuid b/manifest.uuid index 128decbd67..666e786af9 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -1ff78582bfd934e0c76464b5f23ed9bf09a3491b145e0ca34acb6e59c4f53995 \ No newline at end of file +deed5797de65a25896e991a441f0d05eb92662536296485920fb081e84ad5d32 \ No newline at end of file