From: stephan Date: Sat, 5 Aug 2023 21:35:58 +0000 (+0000) Subject: Refactor the per-JNIEnv cache from a fixed-size static array to a linked list of... X-Git-Tag: version-3.43.0~47^2~94 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=28e95830ada662822066b9175fb734b355221806;p=thirdparty%2Fsqlite.git Refactor the per-JNIEnv cache from a fixed-size static array to a linked list of dynamically-allocated entries. Uncache all per-db state (which is necessarily JNIEnv-specific) when the corresponding JNIEnv is uncached. FossilOrigin-Name: 9dd8b78419e19e88bc3fbff9bf200390b146b2461af2bb6b93d8467036619e33 --- diff --git a/ext/jni/src/c/sqlite3-jni.c b/ext/jni/src/c/sqlite3-jni.c index 0c9862d402..56b86c2577 100644 --- a/ext/jni/src/c/sqlite3-jni.c +++ b/ext/jni/src/c/sqlite3-jni.c @@ -293,24 +293,13 @@ static const struct { enum { /** - Size of the per-JNIEnv cache. We have no way of knowing how many - distinct JNIEnv's will be used in any given run, but know that it - will normally be only 1. Perhaps (just speculating) different - threads use separate JNIEnvs? If that's the case, we don't(?) - have enough info to evict from the cache when those JNIEnvs - expire. - - If this ever fills up, we can refactor this to dynamically - allocate them. - */ - JNIEnvCache_SIZE = 10, - /** - Need enough space for (only) the library's NativePointerHolder - types, a fixed count known at build-time. If we add more than this - a fatal error will be triggered with a reminder to increase this. - This value needs to be exactly the number of entries in the - S3ClassNames object. The S3ClassNames entries are the keys for - this particular cache. + Size of the NativePointerHolder cache. Need enough space for + (only) the library's NativePointerHolder types, a fixed count + known at build-time. If we add more than this a fatal error will + be triggered with a reminder to increase this. This value needs + to be exactly the number of entries in the S3ClassNames + object. The S3ClassNames entries are the keys for this particular + cache. */ NphCache_SIZE = sizeof(S3ClassNames) / sizeof(char const *) }; @@ -370,12 +359,8 @@ struct JNIEnvCacheLine { jfieldID fidB; } jPhraseIter; #endif -#if 0 - /* TODO: refactor this cache as a linked list with malloc()'d entries, - rather than a fixed-size array in S3Global.envCache */ JNIEnvCacheLine * pPrev /* Previous entry in the linked list */; JNIEnvCacheLine * pNext /* Next entry in the linked list */; -#endif /** TODO: NphCacheLine *pNphHit; to help fast-track cache lookups, update this to point to the @@ -384,41 +369,12 @@ struct JNIEnvCacheLine { */ struct NphCacheLine nph[NphCache_SIZE]; }; -typedef struct JNIEnvCache JNIEnvCache; -struct JNIEnvCache { - struct JNIEnvCacheLine lines[JNIEnvCache_SIZE]; - unsigned int used; -}; static void NphCacheLine_clear(JNIEnv * const env, NphCacheLine * const p){ UNREF_G(p->klazz); memset(p, 0, sizeof(NphCacheLine)); } -static void JNIEnvCacheLine_clear(JNIEnvCacheLine * const p){ - JNIEnv * const env = p->env; - if(env){ - int i; - UNREF_G(p->globalClassObj); - UNREF_G(p->globalClassLong); -#ifdef SQLITE_ENABLE_FTS5 - UNREF_G(p->jFtsExt); - UNREF_G(p->jPhraseIter.klazz); -#endif - for( i = 0; i < NphCache_SIZE; ++i ){ - NphCacheLine_clear(env, &p->nph[i]); - } - memset(p, 0, sizeof(JNIEnvCacheLine)); - } -} - -static void JNIEnvCache_clear(JNIEnvCache * const p){ - unsigned int i = 0; - for( ; i < p->used; ++i ){ - JNIEnvCacheLine_clear( &p->lines[i] ); - } -} - /** State for various hook callbacks. */ typedef struct JniHookState JniHookState; struct JniHookState{ @@ -476,7 +432,10 @@ static struct { JNIEnv when necessary. */ JavaVM * jvm; - struct JNIEnvCache envCache; + struct { + JNIEnvCacheLine * aHead /* Linked list of in-use instances */; + JNIEnvCacheLine * aFree /* Linked list of free instances */; + } envCache; struct { PerDbStateJni * aUsed /* Linked list of in-use instances */; PerDbStateJni * aFree /* Linked list of free instances */; @@ -512,11 +471,6 @@ static void * s3jni_malloc(JNIEnv * const env, size_t n){ return rv; } -static void s3jni_free(void * const p){ - if(p) sqlite3_free(p); -} - - /* ** This function is NOT part of the sqlite3 public API. It is strictly ** for use by the sqlite project's own Java/JNI bindings. @@ -575,38 +529,183 @@ static void s3jni_call_xDestroy(JNIEnv * const env, jobject jObj, jclass klazz){ /** Fetches the S3Global.envCache row for the given env, allocing a row if needed. When a row is allocated, its state is initialized - insofar as possible. Calls (*env)->FatalError() if the cache - fills up. That's hypothetically possible but "shouldn't happen." - If it does, we can dynamically allocate these instead. + insofar as possible. Calls (*env)->FatalError() if allocation of + an entry fails. That's hypothetically possible but "shouldn't happen." */ FIXME_THREADING -static struct JNIEnvCacheLine * S3Global_env_cache(JNIEnv * const env){ - struct JNIEnvCacheLine * row = 0; - int i = 0; - for( ; i < JNIEnvCache_SIZE; ++i ){ - row = &S3Global.envCache.lines[i]; - if(row->env == env){ +static JNIEnvCacheLine * S3Global_JNIEnvCache_cache(JNIEnv * const env){ + struct JNIEnvCacheLine * row = S3Global.envCache.aHead; + for( ; row; row = row->pNext ){ + if( row->env == env ){ ++S3Global.metrics.envCacheHits; return row; } - else if(!row->env) break; } ++S3Global.metrics.envCacheMisses; - if(i == JNIEnvCache_SIZE){ - (*env)->FatalError(env, "Maintenance required: JNIEnvCache is full."); - return NULL; + row = S3Global.envCache.aFree; + if( row ){ + assert(!row->pPrev); + S3Global.envCache.aFree = row->pNext; + if( row->pNext ) row->pNext->pPrev = 0; + }else{ + row = sqlite3_malloc(sizeof(JNIEnvCacheLine)); + if( !row ){ + (*env)->FatalError(env, "Maintenance required: JNIEnvCache is full.") + /* Does not return, but cc doesn't know that */; + return NULL; + } } - memset(row, 0, sizeof(JNIEnvCacheLine)); + memset(row, 0, sizeof(*row)); + row->pNext = S3Global.envCache.aHead; + if(row->pNext) row->pNext->pPrev = row; + S3Global.envCache.aHead = row; row->env = env; row->globalClassObj = REF_G((*env)->FindClass(env,"java/lang/Object")); + EXCEPTION_IS_FATAL("Error getting reference to Object class."); row->globalClassLong = REF_G((*env)->FindClass(env,"java/lang/Long")); + EXCEPTION_IS_FATAL("Error getting reference to Long class."); row->ctorLong1 = (*env)->GetMethodID(env, row->globalClassLong, "", "(J)V"); - ++S3Global.envCache.used; - //MARKER(("Added S3Global.envCache entry #%d.\n", S3Global.envCache.used)); + EXCEPTION_IS_FATAL("Error getting reference to Long constructor."); return row; } +/** + Removes any Java references from s and clears its state. If + doXDestroy is true and s->klazz and s->jObj are not NULL, s->jObj's + s is passed to s3jni_call_xDestroy() before any references are + cleared. It is legal to call this when the object has no Java + references. +*/ +static void JniHookState_unref(JNIEnv * const env, JniHookState * const s, int doXDestroy){ + if(doXDestroy && s->klazz && s->jObj){ + s3jni_call_xDestroy(env, s->jObj, s->klazz); + } + UNREF_G(s->jObj); + UNREF_G(s->klazz); + memset(s, 0, sizeof(*s)); +} + +/** + Clears s's state and moves it to the free-list. +*/ +FIXME_THREADING +static void PerDbStateJni_set_aside(PerDbStateJni * const s){ + if(s){ + JNIEnv * const env = s->env; + assert(s->pDb && "Else this object is already in the free-list."); + //MARKER(("state@%p for db@%p setting aside\n", s, s->pDb)); + assert(s->pPrev != s); + assert(s->pNext != s); + assert(s->pPrev ? (s->pPrev!=s->pNext) : 1); + if(s->pNext) s->pNext->pPrev = s->pPrev; + if(s->pPrev) s->pPrev->pNext = s->pNext; + else if(S3Global.perDb.aUsed == s){ + assert(!s->pPrev); + S3Global.perDb.aUsed = s->pNext; + } +#define UNHOOK(MEMBER,XDESTROY) JniHookState_unref(env, &s->MEMBER, XDESTROY) + UNHOOK(trace, 0); + UNHOOK(progress, 0); + UNHOOK(commitHook, 0); + UNHOOK(rollbackHook, 0); + UNHOOK(updateHook, 0); + UNHOOK(collation, 1); + UNHOOK(collationNeeded, 1); + UNHOOK(busyHandler, 1); +#undef UNHOOK + UNREF_G(s->jDb); +#ifdef SQLITE_ENABLE_FTS5 + UNREF_G(s->jFtsApi); +#endif + memset(s, 0, sizeof(PerDbStateJni)); + s->pNext = S3Global.perDb.aFree; + if(s->pNext) s->pNext->pPrev = s; + S3Global.perDb.aFree = s; + //MARKER(("%p->pPrev@%p, pNext@%p\n", s, s->pPrev, s->pNext)); + //if(s->pNext) MARKER(("next: %p->pPrev@%p\n", s->pNext, s->pNext->pPrev)); + } +} + +/** + Requires that p has been snipped from any linked list it is + in. Clears all Java refs p holds and zeroes out p. +*/ +static void JNIEnvCacheLine_clear(JNIEnvCacheLine * const p){ + JNIEnv * const env = p->env; + if(env){ + int i; + UNREF_G(p->globalClassObj); + UNREF_G(p->globalClassLong); +#ifdef SQLITE_ENABLE_FTS5 + UNREF_G(p->jFtsExt); + UNREF_G(p->jPhraseIter.klazz); +#endif + for( i = 0; i < NphCache_SIZE; ++i ){ + NphCacheLine_clear(env, &p->nph[i]); + } + memset(p, 0, sizeof(JNIEnvCacheLine)); + } +} + +/** + Cleans up all state in S3Global.perDb for th given JNIEnv. + Results are undefined if a Java-side db uses the API + from the given JNIEnv after this call. +*/ +FIXME_THREADING +static void PerDbStateJni_free_for_env(JNIEnv *env){ + PerDbStateJni * ps = S3Global.perDb.aUsed; + PerDbStateJni * pNext = 0; + for( ; ps; ps = pNext ){ + pNext = ps->pNext; + if(ps->env == env){ + PerDbStateJni * const pPrev = ps->pPrev; + PerDbStateJni_set_aside(ps); + assert(pPrev ? pPrev->pNext!=ps : 1); + pNext = pPrev; + } + } +} + +/** + Uncache any state for the given JNIEnv, clearing all Java + references the cache owns. Returns true if env was cached and false + if it was not found in the cache. + + Also passes env to PerDbStateJni_free_for_env() to free up + what would otherwise be stale references. +*/ +static int S3Global_JNIEnvCache_uncache(JNIEnv * const env){ + struct JNIEnvCacheLine * row = S3Global.envCache.aHead; + for( ; row; row = row->pNext ){ + if( row->env == env ){ + break; + } + } + if( !row ) return 0; + if( row->pNext ) row->pNext->pPrev = row->pPrev; + if( row->pPrev ) row->pPrev->pNext = row->pNext; + if( S3Global.envCache.aHead == row ){ + assert( !row->pPrev ); + S3Global.envCache.aHead = row->pNext; + } + JNIEnvCacheLine_clear(row); + assert( !row->pNext ); + assert( !row->pPrev ); + row->pNext = S3Global.envCache.aFree; + if( row->pNext ) row->pNext->pPrev = row; + S3Global.envCache.aFree = row; + PerDbStateJni_free_for_env(env); + return 1; +} + +static void S3Global_JNIEnvCache_clear(void){ + while( S3Global.envCache.aHead ){ + S3Global_JNIEnvCache_uncache( S3Global.envCache.aHead->env ); + } +} + /** Searches the NativePointerHolder cache for the given combination. If it finds one, it returns it as-is. If it doesn't AND the cache @@ -640,7 +739,7 @@ static struct NphCacheLine * S3Global_nph_cache(JNIEnv * const env, const char * looking up class objects can be expensive, so they should be cached as well. */ - struct JNIEnvCacheLine * const envRow = S3Global_env_cache(env); + struct JNIEnvCacheLine * const envRow = S3Global_JNIEnvCache_cache(env); struct NphCacheLine * freeSlot = 0; struct NphCacheLine * cacheLine = 0; int i; @@ -787,63 +886,6 @@ static PerDbStateJni * PerDbStateJni_alloc(JNIEnv * const env, sqlite3 *pDb, job return rv; } -/** - Removes any Java references from s and clears its state. If - doXDestroy is true and s->klazz and s->jObj are not NULL, s->jObj's - s is passed to s3jni_call_xDestroy() before any references are - cleared. It is legal to call this when the object has no Java - references. -*/ -static void JniHookState_unref(JNIEnv * const env, JniHookState * const s, int doXDestroy){ - if(doXDestroy && s->klazz && s->jObj){ - s3jni_call_xDestroy(env, s->jObj, s->klazz); - } - UNREF_G(s->jObj); - UNREF_G(s->klazz); - memset(s, 0, sizeof(*s)); -} - -/** - Clears s's state and moves it to the free-list. -*/ -FIXME_THREADING -static void PerDbStateJni_set_aside(PerDbStateJni * const s){ - if(s){ - JNIEnv * const env = s->env; - assert(s->pDb && "Else this object is already in the free-list."); - //MARKER(("state@%p for db@%p setting aside\n", s, s->pDb)); - assert(s->pPrev != s); - assert(s->pNext != s); - assert(s->pPrev ? (s->pPrev!=s->pNext) : 1); - if(s->pNext) s->pNext->pPrev = s->pPrev; - if(s->pPrev) s->pPrev->pNext = s->pNext; - else if(S3Global.perDb.aUsed == s){ - assert(!s->pPrev); - S3Global.perDb.aUsed = s->pNext; - } -#define UNHOOK(MEMBER,XDESTROY) JniHookState_unref(env, &s->MEMBER, XDESTROY) - UNHOOK(trace, 0); - UNHOOK(progress, 0); - UNHOOK(commitHook, 0); - UNHOOK(rollbackHook, 0); - UNHOOK(updateHook, 0); - UNHOOK(collation, 1); - UNHOOK(collationNeeded, 1); - UNHOOK(busyHandler, 1); -#undef UNHOOK - UNREF_G(s->jDb); -#ifdef SQLITE_ENABLE_FTS5 - UNREF_G(s->jFtsApi); -#endif - memset(s, 0, sizeof(PerDbStateJni)); - s->pNext = S3Global.perDb.aFree; - if(s->pNext) s->pNext->pPrev = s; - S3Global.perDb.aFree = s; - //MARKER(("%p->pPrev@%p, pNext@%p\n", s, s->pPrev, s->pNext)); - //if(s->pNext) MARKER(("next: %p->pPrev@%p\n", s->pNext, s->pNext->pPrev)); - } -} - static void PerDbStateJni_dump(PerDbStateJni *s){ MARKER(("PerDbStateJni->env @ %p\n", s->env)); MARKER(("PerDbStateJni->pDb @ %p\n", s->pDb)); @@ -894,28 +936,6 @@ static PerDbStateJni * PerDbStateJni_for_db(JNIEnv * const env, jobject jDb, return s; } -/** - Cleans up and frees all state in S3Global.perDb. -*/ -FIXME_THREADING -static void PerDbStateJni_free_all(void){ - PerDbStateJni * ps = S3Global.perDb.aUsed; - PerDbStateJni * pSNext = 0; - for( ; ps; ps = pSNext ){ - pSNext = ps->pNext; - PerDbStateJni_set_aside(ps); - assert(pSNext ? !pSNext->pPrev : 1); - } - assert( 0==S3Global.perDb.aUsed ); - ps = S3Global.perDb.aFree; - S3Global.perDb.aFree = 0; - pSNext = 0; - for( ; ps; ps = pSNext ){ - pSNext = ps->pNext; - s3jni_free(pSNext); - } -} - /** Requires that jCx be a Java-side sqlite3_context wrapper for pCx. @@ -1322,7 +1342,7 @@ static int udf_args(JNIEnv *env, *jArgv = 0; if(!jcx) goto error_oom; ja = (*env)->NewObjectArray(env, argc, - S3Global_env_cache(env)->globalClassObj, + S3Global_JNIEnvCache_cache(env)->globalClassObj, NULL); if(!ja) goto error_oom; for(i = 0; i < argc; ++i){ @@ -2015,7 +2035,7 @@ JDECL(jint,1finalize)(JENV_JSELF, jobject jpStmt){ int rc = 0; sqlite3_stmt * const pStmt = PtrGet_sqlite3_stmt(jpStmt); if( pStmt ){ - JNIEnvCacheLine * const jc = S3Global_env_cache(env); + JNIEnvCacheLine * const jc = S3Global_JNIEnvCache_cache(env); jobject const pPrev = stmt_set_current(jc, jpStmt); rc = sqlite3_finalize(pStmt); setNativePointer(env, jpStmt, 0, S3ClassNames.sqlite3_stmt); @@ -2086,7 +2106,7 @@ static jint sqlite3_jni_prepare_v123(int prepVersion, JNIEnv * const env, jclass sqlite3_stmt * pStmt = 0; const char * zTail = 0; jbyte * const pBuf = JBA_TOC(baSql); - JNIEnvCacheLine * const jc = S3Global_env_cache(env); + JNIEnvCacheLine * const jc = S3Global_JNIEnvCache_cache(env); jobject const pOldStmt = stmt_set_current(jc, jOutStmt); int rc = SQLITE_ERROR; assert(prepVersion==1 || prepVersion==2 || prepVersion==3); @@ -2181,7 +2201,7 @@ 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); + JNIEnvCacheLine * const jc = S3Global_JNIEnvCache_cache(env); jobject const pPrev = stmt_set_current(jc, jpStmt); rc = sqlite3_reset(pStmt); (void)stmt_set_current(jc, pPrev); @@ -2374,11 +2394,9 @@ JDECL(void,1set_1last_1insert_1rowid)(JENV_JSELF, jobject jpDb, jlong rowId){ } JDECL(jint,1shutdown)(JENV_JSELF){ - PerDbStateJni_free_all(); - JNIEnvCache_clear(&S3Global.envCache); - /* Do not clear S3Global.jvm or the global refs to specific classes: - it's legal to call sqlite3_initialize() again to restart the - lib. */ + S3Global_JNIEnvCache_clear(); + /* Do not clear S3Global.jvm: it's legal to call + sqlite3_initialize() again to restart the lib. */ return sqlite3_shutdown(); } @@ -2386,7 +2404,7 @@ JDECL(jint,1step)(JENV_JSELF,jobject jStmt){ int rc = SQLITE_MISUSE; sqlite3_stmt * const pStmt = PtrGet_sqlite3_stmt(jStmt); if(pStmt){ - JNIEnvCacheLine * const jc = S3Global_env_cache(env); + JNIEnvCacheLine * const jc = S3Global_JNIEnvCache_cache(env); jobject const jPrevStmt = stmt_set_current(jc, jStmt); rc = sqlite3_step(pStmt); (void)stmt_set_current(jc, jPrevStmt); @@ -2400,7 +2418,7 @@ static int s3jni_trace_impl(unsigned traceflag, void *pC, void *pP, void *pX){ 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); + JNIEnvCacheLine * const jc = S3Global_JNIEnvCache_cache(env); int rc; switch(traceflag){ case SQLITE_TRACE_STMT: @@ -2660,7 +2678,6 @@ JDECL(void,1do_1something_1for_1developer)(JENV_JSELF){ SO(JniHookState); SO(PerDbStateJni); SO(S3Global); - SO(JNIEnvCache); SO(S3ClassNames); printf("\t(^^^ %u NativePointerHolder subclasses)\n", (unsigned)(sizeof(S3ClassNames) / sizeof(const char *))); @@ -2782,7 +2799,7 @@ static inline jobject new_fts5_api_wrapper(JNIEnv * const env, fts5_api *sv){ instance, or NULL on OOM. */ static jobject s3jni_getFts5ExensionApi(JNIEnv * const env){ - JNIEnvCacheLine * const row = S3Global_env_cache(env); + JNIEnvCacheLine * const row = S3Global_JNIEnvCache_cache(env); if( !row->jFtsExt ){ row->jFtsExt = new_NativePointerHolder_object(env, S3ClassNames.Fts5ExtensionApi, s3jni_ftsext()); @@ -3056,7 +3073,7 @@ JDECLFtsXA(jint,xPhraseFirst)(JENV_JSELF,jobject jCtx, jint iPhrase, jobject jIter, jobject jOutCol, jobject jOutOff){ Fts5ExtDecl; - JNIEnvCacheLine * const jc = S3Global_env_cache(env); + JNIEnvCacheLine * const jc = S3Global_JNIEnvCache_cache(env); Fts5PhraseIter iter; int rc, iCol = 0, iOff = 0; s3jni_phraseIter_init(env, jc, jIter); @@ -3073,7 +3090,7 @@ JDECLFtsXA(jint,xPhraseFirst)(JENV_JSELF,jobject jCtx, jint iPhrase, JDECLFtsXA(jint,xPhraseFirstColumn)(JENV_JSELF,jobject jCtx, jint iPhrase, jobject jIter, jobject jOutCol){ Fts5ExtDecl; - JNIEnvCacheLine * const jc = S3Global_env_cache(env); + JNIEnvCacheLine * const jc = S3Global_JNIEnvCache_cache(env); Fts5PhraseIter iter; int rc, iCol = 0; s3jni_phraseIter_init(env, jc, jIter); @@ -3089,7 +3106,7 @@ JDECLFtsXA(jint,xPhraseFirstColumn)(JENV_JSELF,jobject jCtx, jint iPhrase, JDECLFtsXA(void,xPhraseNext)(JENV_JSELF,jobject jCtx, jobject jIter, jobject jOutCol, jobject jOutOff){ Fts5ExtDecl; - JNIEnvCacheLine * const jc = S3Global_env_cache(env); + JNIEnvCacheLine * const jc = S3Global_JNIEnvCache_cache(env); Fts5PhraseIter iter; int iCol = 0, iOff = 0; if(!jc->jPhraseIter.klazz) return /*SQLITE_MISUSE*/; @@ -3104,7 +3121,7 @@ JDECLFtsXA(void,xPhraseNext)(JENV_JSELF,jobject jCtx, jobject jIter, JDECLFtsXA(void,xPhraseNextColumn)(JENV_JSELF,jobject jCtx, jobject jIter, jobject jOutCol){ Fts5ExtDecl; - JNIEnvCacheLine * const jc = S3Global_env_cache(env); + JNIEnvCacheLine * const jc = S3Global_JNIEnvCache_cache(env); Fts5PhraseIter iter; int iCol = 0; if(!jc->jPhraseIter.klazz) return /*SQLITE_MISUSE*/; @@ -3158,7 +3175,7 @@ static int s3jni_xQueryPhrase(const Fts5ExtensionApi *xapi, JDECLFtsXA(jint,xQueryPhrase)(JENV_JSELF,jobject jFcx, jint iPhrase, jobject jCallback){ Fts5ExtDecl; - JNIEnvCacheLine * const jc = S3Global_env_cache(env); + JNIEnvCacheLine * const jc = S3Global_JNIEnvCache_cache(env); struct s3jni_xQueryPhraseState s; jclass klazz = jCallback ? (*env)->GetObjectClass(env, jCallback) : NULL; if( !klazz ){ @@ -3247,7 +3264,7 @@ static jint s3jni_fts5_xTokenize(JENV_JSELF, const char *zClassName, jint tokFlags, jobject jFcx, jbyteArray jbaText, jobject jCallback){ Fts5ExtDecl; - JNIEnvCacheLine * const jc = S3Global_env_cache(env); + JNIEnvCacheLine * const jc = S3Global_JNIEnvCache_cache(env); struct s3jni_xQueryPhraseState s; int rc = 0; jbyte * const pText = JBA_TOC(jbaText); @@ -3327,32 +3344,7 @@ JDECLFtsXA(jobject,xUserData)(JENV_JSELF,jobject jFcx){ */ JNIEXPORT jboolean JNICALL Java_org_sqlite_jni_SQLite3Jni_uncacheJniEnv(JNIEnv * const env, jclass self){ - struct JNIEnvCacheLine * row = 0; - int i; - for( i = 0; i < JNIEnvCache_SIZE; ++i ){ - row = &S3Global.envCache.lines[i]; - if(row->env == env){ - break; - } - } - if( i==JNIEnvCache_SIZE ){ - //MARKER(("The given JNIEnv is not currently cached.\n")); - return JNI_FALSE; - } - //MARKER(("Uncaching S3Global.envCache entry #%d.\n", i)); - assert(S3Global.envCache.used >= i); - JNIEnvCacheLine_clear(row); - /** - Move all entries down one slot. memmove() would be faster. We'll - eventually turn this cache into a dynamically-allocated linked - list, anyway, so this part will go away. - */ - for( ++i ; i < JNIEnvCache_SIZE; ++i ){ - S3Global.envCache.lines[i-i] = S3Global.envCache.lines[i]; - } - memset(&S3Global.envCache.lines[i], 0, sizeof(JNIEnvCacheLine)); - --S3Global.envCache.used; - return JNI_TRUE; + return S3Global_JNIEnvCache_uncache(env) ? JNI_TRUE : JNI_FALSE; } @@ -3413,13 +3405,18 @@ Java_org_sqlite_jni_SQLite3Jni_init(JNIEnv * const env, jclass self, jclass klaz //jclass const klazz = (*env)->GetObjectClass(env, sJni); const ConfigFlagEntry * pConfFlag; memset(&S3Global, 0, sizeof(S3Global)); - (void)S3Global_env_cache(env); - assert( 1 == S3Global.envCache.used ); - assert( env == S3Global.envCache.lines[0].env ); - assert( 0 != S3Global.envCache.lines[0].globalClassObj ); if( (*env)->GetJavaVM(env, &S3Global.jvm) ){ (*env)->FatalError(env, "GetJavaVM() failure shouldn't be possible."); + return; + } + (void)S3Global_JNIEnvCache_cache(env); + if( !S3Global.envCache.aHead ){ + (*env)->FatalError(env, "Could not allocate JNIEnv-specific cache."); + return; } + assert( 1 == S3Global.metrics.envCacheMisses ); + assert( env == S3Global.envCache.aHead->env ); + assert( 0 != S3Global.envCache.aHead->globalClassObj ); for( pConfFlag = &aLimits[0]; pConfFlag->zName; ++pConfFlag ){ char const * zSig = (JTYPE_BOOL == pConfFlag->jtype) ? "Z" : "I"; diff --git a/ext/jni/src/org/sqlite/jni/SQLite3Jni.java b/ext/jni/src/org/sqlite/jni/SQLite3Jni.java index 0320b102fe..9c8e1321c4 100644 --- a/ext/jni/src/org/sqlite/jni/SQLite3Jni.java +++ b/ext/jni/src/org/sqlite/jni/SQLite3Jni.java @@ -79,9 +79,22 @@ public final class SQLite3Jni { library again after that "should" re-initialize the cache on demand, but that's untested. + This call forcibly wipes out all cached information for the + current JNIEnv, a side-effect of which is that behavior is + undefined if any database objects are (A) still active at the + time it is called _and_ (B) calls are subsequently made into the + library with such a database. Doing so will, at best, lead to a + crash. It worst, it will lead to the db possibly misbehaving + because some of its Java-bound state has been cleared. There is + no immediate harm in (A) so long as condition (B) is not met. + This process does _not_ actually close any databases or finalize + any prepared statements. For proper library behavior, and to + avoid C-side leaks, be sure to close them before calling this + function. + Calling this from the main application thread is not strictly required but is "polite." Additional threads must call this - before ending or they will leak cache entries in the C memory, + before ending or they will leak cache entries in the C heap, which in turn may keep numerous Java-side global references active. diff --git a/manifest b/manifest index 295725bace..9898624e78 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Add\sSQLite3Jni.uncacheJniEnv(),\sa\sway\sfor\sJava\sthreads\sto\sclear\stheir\sthread-specific\scached\sstate\sfrom\sthe\sJNI\sbindings\swhen\sthey're\sabout\sto\sterminate\s(or\sare\sotherwise\sdone\susing\sthe\slibrary). -D 2023-08-05T20:19:45.125 +C Refactor\sthe\sper-JNIEnv\scache\sfrom\sa\sfixed-size\sstatic\sarray\sto\sa\slinked\slist\sof\sdynamically-allocated\sentries.\sUncache\sall\sper-db\sstate\s(which\sis\snecessarily\sJNIEnv-specific)\swhen\sthe\scorresponding\sJNIEnv\sis\suncached. +D 2023-08-05T21:35:58.833 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 bb4cd99bd8da534215cb6d278f05a626283eb5d2e8aebdb4d35e548637d35a9a F ext/jni/README.md 6ff7e1f4100dee980434a6ee37a199b653bceec62e233a6e2ccde6e7ae0c58bf -F ext/jni/src/c/sqlite3-jni.c 17389f38639294b6ace3030e04a91f8b2e938e191f4c853075d4f55e94665a0c +F ext/jni/src/c/sqlite3-jni.c a894cb1b6a7479d376d98f7df0e29a713d2b6b6cbc0d4543505b90eb38593592 F ext/jni/src/c/sqlite3-jni.h cd9b6367a260f55a14833861ceb1d87cb729c5414ba5d26fbb8854b0f22c7249 F ext/jni/src/org/sqlite/jni/BusyHandler.java 1b1d3e5c86cd796a0580c81b6af6550ad943baa25e47ada0dcca3aff3ebe978c F ext/jni/src/org/sqlite/jni/Collation.java 8dffbb00938007ad0967b2ab424d3c908413af1bbd3d212b9c9899910f1218d1 @@ -249,7 +249,7 @@ F ext/jni/src/org/sqlite/jni/OutputPointer.java 013f2b5fe569d0585a695f5cfa605a3b F ext/jni/src/org/sqlite/jni/ProgressHandler.java 5979450e996416d28543f1d42634d308439565a99332a8bd84e424af667116cc F ext/jni/src/org/sqlite/jni/RollbackHook.java b04c8abcc6ade44a8a57129e33765793f69df0ba909e49ba18d73f4268d92564 F ext/jni/src/org/sqlite/jni/SQLFunction.java 09ce81c1c637e31c3a830d4c859cce95d65f5e02ff45f8bd1985b3479381bc46 -F ext/jni/src/org/sqlite/jni/SQLite3Jni.java 213e7dfae620767deb421020cd705a131fd8ad3774e2bc9461eab46d12bf240c +F ext/jni/src/org/sqlite/jni/SQLite3Jni.java f8fec24eea4ff9a62467648f377391506883eff4bedfc50d69d8cb3dc78d1ab9 F ext/jni/src/org/sqlite/jni/Tester1.java 868b5ea60b788a43f8b15c1b642015341fed8856abb1bb74e2eb4845ade50a4e F ext/jni/src/org/sqlite/jni/TesterFts5.java cf2d687baafffdeba219b77cf611fd47a0556248820ea794ae3e8259bfbdc5ee F ext/jni/src/org/sqlite/jni/Tracer.java a5cece9f947b0af27669b8baec300b6dd7ff859c3e6a6e4a1bd8b50f9714775d @@ -2081,8 +2081,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 7d4ac44ec419ed0474bdb9d237b97660cf0d8faba8fe686f6a914d7bc04dfa3b -R 4e66fd3de8c1a8416110ff65fc063c39 +P 7468f8761bece58f7ced3d112bbe2fb454432d9c54c9b96cedb5a15bc2926d0f +R c28f39c95821b6209d9e0ded55b1b7ca U stephan -Z de9fd1b4a7e3cbc6e1092ca5c70548cf +Z 6d5410b49d781e986fb3ebb97da80215 # Remove this line to create a well-formed Fossil manifest. diff --git a/manifest.uuid b/manifest.uuid index 5d1a062dd0..27cf161b3c 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -7468f8761bece58f7ced3d112bbe2fb454432d9c54c9b96cedb5a15bc2926d0f \ No newline at end of file +9dd8b78419e19e88bc3fbff9bf200390b146b2461af2bb6b93d8467036619e33 \ No newline at end of file