JNI code reorgs and simplify the failing-alloc interface a bit.

FossilOrigin-Name: deed5797de65a25896e991a441f0d05eb92662536296485920fb081e84ad5d32

diff --git a/ext/jni/GNUmakefile b/ext/jni/GNUmakefile
index dd8df07bc4d3af33776409c085fbd70f738d4720..5f7038f051a15169416fe16918630d4cdec18570 100644 (file)
--- 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 c210d298a0752fc41c5de20885b9834f91fc5d53..d219039d303d5ad9f2c77192fcbd501444bc89d6 100644 (file)
--- 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<T>, 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 3e1f47374099222c7cc9813de56a03b7227f3f12..04b1dafb4778dfa1c6cc77daf2a407b1e31b50ac 100644 (file)
--- 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 128decbd671a9ea87340577e7ecd245fc7db25cc..666e786af9ac79bde546129cd50f300d6ee7855f 100644 (file)
--- a/manifest.uuid
+++ b/manifest.uuid
@@ -1 +1 @@
-1ff78582bfd934e0c76464b5f23ed9bf09a3491b145e0ca34acb6e59c4f53995
\ No newline at end of file
+deed5797de65a25896e991a441f0d05eb92662536296485920fb081e84ad5d32
\ No newline at end of file