From: stephan Date: Mon, 14 Aug 2023 17:12:55 +0000 (+0000) Subject: Bring handling of the Java auto-ext handler more in line with the core in terms of... X-Git-Tag: version-3.44.0~305^2~30 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=9019e2e667b297a4c9548ffeb8651d914f70d509;p=thirdparty%2Fsqlite.git Bring handling of the Java auto-ext handler more in line with the core in terms of locking and mutability during traversal. This removes the explicit synchronous requirement from the Java open() and auto-ext bindings. FossilOrigin-Name: 42994b952e092ae4fa319395208622e887387ca3ff8ac57961c824a6c272bf0e --- diff --git a/ext/jni/src/c/sqlite3-jni.c b/ext/jni/src/c/sqlite3-jni.c index d33329575d..37aa06460e 100644 --- a/ext/jni/src/c/sqlite3-jni.c +++ b/ext/jni/src/c/sqlite3-jni.c @@ -354,6 +354,60 @@ struct S3JniNphClass { by the sqlite3_context binding. */; }; +static void S3JniNphClass_clear(JNIEnv * const env, S3JniNphClass * const p){ + UNREF_G(p->klazz); + memset(p, 0, sizeof(S3JniNphClass)); +} + +/** State for various hook callbacks. */ +typedef struct S3JniHook S3JniHook; +struct S3JniHook{ + jobject jObj /* global ref to Java instance */; + jmethodID midCallback /* callback method. Signature depends on + jObj's type */; + jclass klazz /* global ref to jObj's class. Only needed + by hooks which have an xDestroy() method. + We can probably eliminate this and simply + do the class lookup at the same + (deferred) time we do the xDestroy() + lookup. */; +}; + +/** + Per-(sqlite3*) state for various JNI bindings. This state is + allocated as needed, cleaned up in sqlite3_close(_v2)(), and + recycled when possible. It is freed during sqlite3_shutdown(). +*/ +typedef struct S3JniDb S3JniDb; +struct S3JniDb { + JNIEnv *env /* The associated JNIEnv handle */; + sqlite3 *pDb /* The associated db handle */; + jobject jDb /* A global ref of the object which was passed to + sqlite3_open(_v2)(). We need this in order to have + an object to pass to sqlite3_collation_needed()'s + callback, or else we have to dynamically create one + for that purpose, which would be fine except that + it would be a different instance (and maybe even a + different class) than the one the user may expect + to receive. */; + char * zMainDbName /* Holds any string allocated on behave of + SQLITE_DBCONFIG_MAINDBNAME. */; + S3JniHook busyHandler; + S3JniHook collation; + S3JniHook collationNeeded; + S3JniHook commitHook; + S3JniHook progress; + S3JniHook rollbackHook; + S3JniHook trace; + S3JniHook updateHook; + S3JniHook authHook; +#ifdef SQLITE_ENABLE_FTS5 + jobject jFtsApi /* global ref to s3jni_fts5_api_from_db() */; +#endif + S3JniDb * pNext /* Next entry in the available/free list */; + S3JniDb * pPrev /* Previous entry in the available/free list */; +}; + /** Cache for per-JNIEnv data. @@ -380,6 +434,27 @@ struct S3JniEnv { jmethodID ctorStringBA /* the String(byte[],Charset) constructor */; jmethodID stringGetBytes /* the String.getBytes(Charset) method */; } g /* refs to global Java state */; + /** + pdbOpening is used to coordinate the Java/DB connection of a + being-open()'d db in the face of auto-extensions. "The problem" + is that auto-extensions run before we can bind the C db to its + Java representation, but auto-extensions require that binding. We + handle this as follows: + + - In open(), allocate the Java side of that connection and set + pdbOpening to point to that object. Note that it's per-thread, + and we remain in that thread until after the auto-extensions + are run. + + - Call open(), which triggers the auto-extension handler. + That handler uses pdbOpening to connect the native db handle + which it receives with pdbOpening. + + - When open() returns, check whether it invoked the auto-ext + handler. If not, complete the Java/C binding unless open() + returns a NULL db, in which case free pdbOpening. + */ + S3JniDb * pdbOpening; #ifdef SQLITE_ENABLE_FTS5 jobject jFtsExt /* Global ref to Java singleton for the Fts5ExtensionApi instance. */; @@ -397,11 +472,6 @@ struct S3JniEnv { S3JniNphClass nph[NphCache_SIZE]; }; -static void S3JniNphClass_clear(JNIEnv * const env, S3JniNphClass * const p){ - UNREF_G(p->klazz); - memset(p, 0, sizeof(S3JniNphClass)); -} - /* Whether auto extensions are feasible here is currently unknown due to... @@ -422,62 +492,9 @@ static void S3JniNphClass_clear(JNIEnv * const env, S3JniNphClass * const p){ (2). */ typedef struct S3JniAutoExtension S3JniAutoExtension; -typedef void (*S3JniAutoExtension_xEntryPoint)(sqlite3*); struct S3JniAutoExtension { - jobject jObj; - jmethodID midFunc; - S3JniAutoExtension_xEntryPoint xEntryPoint; - S3JniAutoExtension *pNext /* next linked-list entry */; - S3JniAutoExtension *pPrev /* previous linked-list entry */; -}; - -/** State for various hook callbacks. */ -typedef struct S3JniHook S3JniHook; -struct S3JniHook{ - jobject jObj /* global ref to Java instance */; - jmethodID midCallback /* callback method. Signature depends on - jObj's type */; - jclass klazz /* global ref to jObj's class. Only needed - by hooks which have an xDestroy() method. - We can probably eliminate this and simply - do the class lookup at the same - (deferred) time we do the xDestroy() - lookup. */; -}; - -/** - Per-(sqlite3*) state for various JNI bindings. This state is - allocated as needed, cleaned up in sqlite3_close(_v2)(), and - recycled when possible. It is freed during sqlite3_shutdown(). -*/ -typedef struct S3JniDb S3JniDb; -struct S3JniDb { - JNIEnv *env /* The associated JNIEnv handle */; - sqlite3 *pDb /* The associated db handle */; - jobject jDb /* A global ref of the object which was passed to - sqlite3_open(_v2)(). We need this in order to have - an object to pass to sqlite3_collation_needed()'s - callback, or else we have to dynamically create one - for that purpose, which would be fine except that - it would be a different instance (and maybe even a - different class) than the one the user may expect - to receive. */; - char * zMainDbName /* Holds any string allocated on behave of - SQLITE_DBCONFIG_MAINDBNAME. */; - S3JniHook busyHandler; - S3JniHook collation; - S3JniHook collationNeeded; - S3JniHook commitHook; - S3JniHook progress; - S3JniHook rollbackHook; - S3JniHook trace; - S3JniHook updateHook; - S3JniHook authHook; -#ifdef SQLITE_ENABLE_FTS5 - jobject jFtsApi /* global ref to s3jni_fts5_api_from_db() */; -#endif - S3JniDb * pNext /* Next entry in the available/free list */; - S3JniDb * pPrev /* Previous entry in the available/free list */; + jobject jObj /* Java object */; + jmethodID midFunc /* xEntryPoint() callback */; }; /** @@ -530,38 +547,15 @@ static struct { } metrics; /** The list of bound auto-extensions (Java-side: - org.sqlite.jni.AutoExtension objects). Because this data - structure cannot be manipulated during traversal (without adding - more code to deal with it), and to avoid a small window where a - call to sqlite3_reset/clear_auto_extension() could lock the - structure during an open() call, we lock this mutex before - sqlite3_open() is called and unlock it once sqlite3_open() - returns. + org.sqlite.jni.AutoExtension objects). */ struct { - S3JniAutoExtension *pHead /* Head of the auto-extension list */; - /** - pdbOpening is used to coordinate the Java/DB connection of a - being-open()'d db. "The problem" is that auto-extensions run - before we can bind the C db to its Java representation, but - auto-extensions require that binding. We handle this as - follows: - - - At the start of open(), we lock on this->mutex. - - Allocate the Java side of that connection and set pdbOpening - to point to that object. - - Call open(), which triggers the auto-extension handler. - That handler uses pdbOpening to connect the native db handle - which it recieves with pdbOpening. - - Return from open(). - - Clean up and unlock the mutex. - - If open() did not block on a mutex, there would be a race - condition in which two open() calls could set pdbOpening. - */ - S3JniDb * pdbOpening; - sqlite3_mutex * mutex /* mutex for manipulation/traversal of pHead */; - void const * locker /* Mutex is locked on this object's behalf */; + S3JniAutoExtension *pExt /* Head of the auto-extension list */; + int nAlloc /* number of entries allocated for pExt, + as distinct from the number of active + entries. */; + int nExt /* number of active entries in pExt. */; + sqlite3_mutex * mutex /* mutex for manipulation/traversal of pExt */; } autoExt; } S3JniGlobal; @@ -591,18 +585,12 @@ static struct { /*MARKER(("Leaving PerDb mutex@%p %s.\n", env, __func__));*/ \ S3JniGlobal.perDb.locker = 0; \ sqlite3_mutex_leave( S3JniGlobal.perDb.mutex ) -#define MUTEX_ENV_EXT \ +#define MUTEX_EXT_ENTER \ /*MARKER(("Entering autoExt mutex@%p %s.\n", env, __func__));*/ \ sqlite3_mutex_enter( S3JniGlobal.autoExt.mutex ); \ ++S3JniGlobal.metrics.nMutexAutoExt -#define MUTEX_TRY_EXT(FAIL_EXPR) \ - /*MARKER(("Leaving PerDb mutex@%p %s.\n", env, __func__));*/ \ - if( sqlite3_mutex_try( S3JniGlobal.autoExt.mutex ) ){ FAIL_EXPR; } \ - S3JniGlobal.autoExt.locker = env; \ - ++S3JniGlobal.metrics.nMutexAutoExt #define MUTEX_EXT_LEAVE \ - /*MARKER(("Leaving PerDb mutex@%p %s.\n", env, __func__));*/ \ - S3JniGlobal.autoExt.locker = 0; \ + /*MARKER(("Leaving autoExt mutex@%p %s.\n", env, __func__));*/ \ sqlite3_mutex_leave( S3JniGlobal.autoExt.mutex ) #define OOM_CHECK(VAR) if(!(VAR)) s3jni_oom(env) @@ -1241,53 +1229,40 @@ static S3JniDb * S3JniDb_for_db(JNIEnv * const env, jobject jDb, sqlite3 *pDb){ } /** - Unlink ax from S3JniGlobal.autoExt and free it. + Unref any Java-side state in ax. */ -static void S3JniAutoExtension_free(JNIEnv * const env, - S3JniAutoExtension * const ax){ - if( ax ){ - if( ax->pNext ) ax->pNext->pPrev = ax->pPrev; - if( ax == S3JniGlobal.autoExt.pHead ){ - assert( !ax->pNext ); - S3JniGlobal.autoExt.pHead = ax->pNext; - }else if( ax->pPrev ){ - ax->pPrev->pNext = ax->pNext; - } - ax->pNext = ax->pPrev = 0; +static void S3JniAutoExtension_clear(JNIEnv * const env, + S3JniAutoExtension * const ax){ + if( ax->jObj ){ UNREF_G(ax->jObj); - sqlite3_free(ax); + memset(ax, 0, sizeof(*ax)); } } /** - Allocates a new auto extension and plugs it in to S3JniGlobal.autoExt. - Returns 0 on OOM or if there is an error collecting the required - state from jAutoExt (which must be an AutoExtension object). + Initializes a pre-allocated S3JniAutoExtension object. Returns + non-0 if there is an error collecting the required state from + jAutoExt (which must be an AutoExtension object). */ -static S3JniAutoExtension * S3JniAutoExtension_alloc(JNIEnv *const env, - jobject const jAutoExt){ - S3JniAutoExtension * const ax = sqlite3_malloc(sizeof(*ax)); - if( ax ){ - jclass klazz; - memset(ax, 0, sizeof(*ax)); - klazz = (*env)->GetObjectClass(env, jAutoExt); - if(!klazz){ - S3JniAutoExtension_free(env, ax); - return 0; - } - ax->midFunc = (*env)->GetMethodID(env, klazz, "xEntryPoint", - "(Lorg/sqlite/jni/sqlite3;)I"); - if(!ax->midFunc){ - MARKER(("Error getting xEntryPoint(sqlite3) from object.")); - S3JniAutoExtension_free(env, ax); - return 0; - } - ax->jObj = REF_G(jAutoExt); - ax->pNext = S3JniGlobal.autoExt.pHead; - if( ax->pNext ) ax->pNext->pPrev = ax; - S3JniGlobal.autoExt.pHead = ax; +static int S3JniAutoExtension_init(JNIEnv *const env, + S3JniAutoExtension * const ax, + jobject const jAutoExt){ + jclass klazz; + klazz = (*env)->GetObjectClass(env, jAutoExt); + if(!klazz){ + S3JniAutoExtension_clear(env, ax); + return SQLITE_ERROR; + } + ax->midFunc = (*env)->GetMethodID(env, klazz, "xEntryPoint", + "(Lorg/sqlite/jni/sqlite3;)I"); + if(!ax->midFunc){ + MARKER(("Error getting xEntryPoint(sqlite3) from object.")); + S3JniAutoExtension_clear(env, ax); + return SQLITE_ERROR; } - return ax; + UNREF_L(klazz); + ax->jObj = REF_G(jAutoExt); + return 0; } /** @@ -1888,65 +1863,96 @@ static JNIEnv * s3jni_get_env(void){ /* Central auto-extension handler. */ static int s3jni_run_java_auto_extensions(sqlite3 *pDb, const char **pzErr, const struct sqlite3_api_routines *ignored){ - S3JniAutoExtension const * pAX = S3JniGlobal.autoExt.pHead; - int rc; + int rc = 0; + unsigned i, go = 1; JNIEnv * env = 0; - S3JniDb * const ps = S3JniGlobal.autoExt.pdbOpening; - - assert( S3JniGlobal.autoExt.locker ); - assert( S3JniGlobal.autoExt.locker == ps ); - S3JniGlobal.autoExt.pdbOpening = 0; - if( !pAX ){ - return 0; - }else if( S3JniGlobal.autoExt.locker != ps ) { - *pzErr = sqlite3_mprintf("Internal error: unexpected path lead to " - "running an auto-extension."); - return SQLITE_ERROR; - } + S3JniDb * ps; + S3JniEnv * jc; + if( 0==S3JniGlobal.autoExt.nExt ) return 0; env = s3jni_get_env(); + jc = S3JniGlobal_env_cache(env); + ps = jc->pdbOpening; + jc->pdbOpening = 0; + assert( ps && "Unexpected arrival of null S3JniDb in auto-extension runner." ); //MARKER(("auto-extension on open()ing ps@%p db@%p\n", ps, pDb)); - assert( !ps->pDb /* it's still being opened */ ); + assert( !ps->pDb && "it's still being opened" ); ps->pDb = pDb; assert( ps->jDb ); NativePointerHolder_set(env, ps->jDb, pDb, &S3NphRefs.sqlite3); - for( ; pAX; pAX = pAX->pNext ){ - rc = (*env)->CallIntMethod(env, pAX->jObj, pAX->midFunc, ps->jDb); - IFTHREW { - jthrowable const ex = (*env)->ExceptionOccurred(env); - char * zMsg; - EXCEPTION_CLEAR; - zMsg = s3jni_exception_error_msg(env, ex); - UNREF_L(ex); - *pzErr = sqlite3_mprintf("auto-extension threw: %s", zMsg); - sqlite3_free(zMsg); - rc = rc ? rc : SQLITE_ERROR; - break; - }else if( rc ){ - break; + for( i = 0; go && 0==rc; ++i ){ + S3JniAutoExtension const * ax; + MUTEX_EXT_ENTER; + if( i >= S3JniGlobal.autoExt.nAlloc ){ + ax = 0; + go = 0; + }else{ + ax = &S3JniGlobal.autoExt.pExt[i]; + } + MUTEX_EXT_LEAVE; + if( ax && ax->jObj ){ + rc = (*env)->CallIntMethod(env, ax->jObj, ax->midFunc, ps->jDb); + IFTHREW { + jthrowable const ex = (*env)->ExceptionOccurred(env); + char * zMsg; + EXCEPTION_CLEAR; + zMsg = s3jni_exception_error_msg(env, ex); + UNREF_L(ex); + *pzErr = sqlite3_mprintf("auto-extension threw: %s", zMsg); + sqlite3_free(zMsg); + rc = rc ? rc : SQLITE_ERROR; + } } } return rc; } FIXME_THREADING(autoExt) -JDECL(jint,1auto_1extension)(JENV_OSELF, jobject jAutoExt){ +JDECL(jint,1auto_1extension)(JENV_CSELF, jobject jAutoExt){ static int once = 0; + int i; S3JniAutoExtension * ax; + int rc = 0; + int firstEmptySlot = -1; if( !jAutoExt ) return SQLITE_MISUSE; - MUTEX_ENV_EXT; - if( 0==once && ++once ){ - sqlite3_auto_extension( (void(*)(void))s3jni_run_java_auto_extensions ); - } - ax = S3JniGlobal.autoExt.pHead; - for( ; ax; ax = ax->pNext ){ - if( (*env)->IsSameObject(env, ax->jObj, jAutoExt) ){ - return 0 /* C API treats this as a no-op. */; + MUTEX_EXT_ENTER; + for( i = 0; i < S3JniGlobal.autoExt.nAlloc; ++i ){ + /* Look for match or first empty slot. */ + ax = &S3JniGlobal.autoExt.pExt[i]; + if( ax->jObj && (*env)->IsSameObject(env, ax->jObj, jAutoExt) ){ + break /* this as a no-op. */; + }else if( !ax->jObj && firstEmptySlot<0 ){ + firstEmptySlot = (int)i; + } + } + if(i == S3JniGlobal.autoExt.nAlloc ){ + if( firstEmptySlot >= 0 ){ + ax = &S3JniGlobal.autoExt.pExt[firstEmptySlot]; + rc = S3JniAutoExtension_init(env, ax, jAutoExt); + }else{ + unsigned n = 1 + S3JniGlobal.autoExt.nAlloc; + S3JniAutoExtension * const aNew = + sqlite3_realloc( S3JniGlobal.autoExt.pExt, + n * sizeof(S3JniAutoExtension) ); + if( !aNew ){ + rc = SQLITE_NOMEM; + }else{ + S3JniGlobal.autoExt.pExt = aNew; + ax = &S3JniGlobal.autoExt.pExt[S3JniGlobal.autoExt.nAlloc]; + ++S3JniGlobal.autoExt.nAlloc; + rc = S3JniAutoExtension_init(env, ax, jAutoExt); + assert( rc ? 0==ax->jObj : 0!=ax->jObj ); + } + } + } + if( 0==rc ){ + ++S3JniGlobal.autoExt.nExt; + if( 0==once && ++once ){ + sqlite3_auto_extension( (void(*)(void))s3jni_run_java_auto_extensions ); } } - ax = S3JniAutoExtension_alloc(env, jAutoExt); MUTEX_EXT_LEAVE; - return ax ? 0 : SQLITE_NOMEM; + return rc; } FIXME_THREADING(S3JniEnv) @@ -2087,14 +2093,37 @@ FIXME_THREADING(autoExt) JDECL(jboolean,1cancel_1auto_1extension)(JENV_CSELF, jobject jAutoExt){ S3JniAutoExtension * ax; jboolean rc = JNI_FALSE; - MUTEX_ENV_EXT; - for( ax = S3JniGlobal.autoExt.pHead; ax; ax = ax->pNext ){ - if( (*env)->IsSameObject(env, ax->jObj, jAutoExt) ){ - S3JniAutoExtension_free(env, ax); + int i; + MUTEX_EXT_ENTER; +#if 1 + for( i = 0; i < S3JniGlobal.autoExt.nAlloc; ++i ){ + ax = &S3JniGlobal.autoExt.pExt[i]; + if( ax->jObj && (*env)->IsSameObject(env, ax->jObj, jAutoExt) ){ + S3JniAutoExtension_clear(env, ax); + /* Move final entry into this slot. */ + *ax = S3JniGlobal.autoExt.pExt[S3JniGlobal.autoExt.nAlloc - 1]; + memset(&S3JniGlobal.autoExt.pExt[S3JniGlobal.autoExt.nAlloc - 1], 0, + sizeof(S3JniAutoExtension)); + --S3JniGlobal.autoExt.nExt; rc = JNI_TRUE; break; } } +#else + /* Why does this impl lead to an invalid unref error? */ + for( i = S3JniGlobal.autoExt.nAlloc-1; i <= 0; --i ){ + ax = &S3JniGlobal.autoExt.pExt[i]; + if( ax->jObj && (*env)->IsSameObject(env, ax->jObj, jAutoExt) ){ + S3JniAutoExtension_clear(env, ax); + /* Move final entry into this slot. */ + *ax = S3JniGlobal.autoExt.pExt[S3JniGlobal.autoExt.nAlloc - 1]; + memset(&S3JniGlobal.autoExt.pExt[S3JniGlobal.autoExt.nAlloc - 1], 0, + sizeof(S3JniAutoExtension)); + rc = JNI_TRUE; + break; + } + } +#endif MUTEX_EXT_LEAVE; return rc; } @@ -2646,14 +2675,6 @@ static int s3jni_open_pre(JNIEnv * const env, S3JniEnv **jc, jstring jDbName, char **zDbName, S3JniDb ** ps, jobject *jDb){ int rc = 0; - MUTEX_TRY_EXT(return SQLITE_BUSY) - /* we don't wait forever here because it could lead to a deadlock - if an auto-extension opens a database. Without a mutex, that - situation leads to infinite recursion and stack overflow, which - is infinitely easier to track down from client code. Note that - we rely on the Java methods for open() and auto-extension - handling to be synchronized so that this BUSY cannot be - triggered by a race condition with the auto-ext functions. */; *jc = S3JniGlobal_env_cache(env); if(!*jc){ rc = SQLITE_NOMEM; @@ -2673,17 +2694,12 @@ static int s3jni_open_pre(JNIEnv * const env, S3JniEnv **jc, } *ps = S3JniDb_alloc(env, 0, *jDb); if(*ps){ - S3JniGlobal.autoExt.pdbOpening = *ps; - S3JniGlobal.autoExt.locker = *ps; + (*jc)->pdbOpening = *ps; }else{ rc = SQLITE_NOMEM; } //MARKER(("pre-open ps@%p\n", *ps)); end: - if(rc){ - MUTEX_EXT_LEAVE; - /* Else remain in autoExt.mutex until s3jni_open_post(). */ - } return rc; } @@ -2698,11 +2714,11 @@ end: Returns theRc. */ -static int s3jni_open_post(JNIEnv * const env, S3JniDb * ps, - sqlite3 **ppDb, jobject jOut, int theRc){ +static int s3jni_open_post(JNIEnv * const env, S3JniEnv * const jc, + S3JniDb * ps, sqlite3 **ppDb, + jobject jOut, int theRc){ //MARKER(("post-open() ps@%p db@%p\n", ps, *ppDb)); - assert( S3JniGlobal.autoExt.locker == ps ); - S3JniGlobal.autoExt.pdbOpening = 0; + jc->pdbOpening = 0; if(*ppDb){ assert(ps->jDb); if( 0==ps->pDb ){ @@ -2718,7 +2734,6 @@ static int s3jni_open_post(JNIEnv * const env, S3JniDb * ps, ps = 0; } OutputPointer_set_sqlite3(env, jOut, ps ? ps->jDb : 0); - MUTEX_EXT_LEAVE /* locked in s3jni_open_pre() */; return theRc; } @@ -2728,17 +2743,15 @@ JDECL(jint,1open)(JENV_CSELF, jstring strName, jobject jOut){ jobject jDb = 0; S3JniDb * ps = 0; S3JniEnv * jc = 0; - S3JniDb * const prevOpening = S3JniGlobal.autoExt.pdbOpening; - int rc= s3jni_open_pre(env, &jc, strName, &zName, &ps, &jDb); + int rc = s3jni_open_pre(env, &jc, strName, &zName, &ps, &jDb); if( 0==rc ){ rc = sqlite3_open(zName, &pOut); //MARKER(("env=%p, *env=%p\n", env, *env)); //MARKER(("open() ps@%p db@%p\n", ps, pOut)); - rc = s3jni_open_post(env, ps, &pOut, jOut, rc); + rc = s3jni_open_post(env, jc, ps, &pOut, jOut, rc); assert(rc==0 ? pOut!=0 : 1); sqlite3_free(zName); } - S3JniGlobal.autoExt.pdbOpening = prevOpening; return (jint)rc; } @@ -2750,7 +2763,6 @@ JDECL(jint,1open_1v2)(JENV_CSELF, jstring strName, S3JniDb * ps = 0; S3JniEnv * jc = 0; char *zVfs = 0; - S3JniDb * const prevOpening = S3JniGlobal.autoExt.pdbOpening; int rc = s3jni_open_pre(env, &jc, strName, &zName, &ps, &jDb); if( 0==rc && strVfs ){ zVfs = s3jni_jstring_to_utf8(jc, strVfs, 0); @@ -2764,8 +2776,7 @@ JDECL(jint,1open_1v2)(JENV_CSELF, jstring strName, //MARKER(("open_v2() ps@%p db@%p\n", ps, pOut)); /*MARKER(("zName=%s, zVfs=%s, pOut=%p, flags=%d, nrc=%d\n", zName, zVfs, pOut, (int)flags, nrc));*/ - rc = s3jni_open_post(env, ps, &pOut, jOut, rc); - S3JniGlobal.autoExt.pdbOpening = prevOpening; + rc = s3jni_open_post(env, jc, ps, &pOut, jOut, rc); assert(rc==0 ? pOut!=0 : 1); sqlite3_free(zName); sqlite3_free(zVfs); @@ -2905,10 +2916,12 @@ JDECL(jint,1reset)(JENV_CSELF, jobject jpStmt){ } JDECL(void,1reset_1auto_1extension)(JENV_CSELF){ - MUTEX_ENV_EXT; - while( S3JniGlobal.autoExt.pHead ){ - S3JniAutoExtension_free(env, S3JniGlobal.autoExt.pHead); + int i; + MUTEX_EXT_ENTER; + for( i = 0; i < S3JniGlobal.autoExt.nAlloc; ++i ){ + S3JniAutoExtension_clear( env, &S3JniGlobal.autoExt.pExt[i] ); } + S3JniGlobal.autoExt.nExt = 0; MUTEX_EXT_LEAVE; } @@ -3517,7 +3530,7 @@ JDECL(void,1do_1something_1for_1developer)(JENV_CSELF){ printf("\tJNIEnv cache %u misses, %u hits\n", S3JniGlobal.metrics.envCacheMisses, S3JniGlobal.metrics.envCacheHits); - printf("Mutex entry:\n\t%u env\n\t%u perDb\n\t%u autoExt (mostly via open[_v2]())\n", + printf("Mutex entry:\n\t%u env\n\t%u perDb\n\t%u autoExt\n", S3JniGlobal.metrics.nMutexEnv, S3JniGlobal.metrics.nMutexPerDb, S3JniGlobal.metrics.nMutexAutoExt); diff --git a/ext/jni/src/org/sqlite/jni/SQLite3Jni.java b/ext/jni/src/org/sqlite/jni/SQLite3Jni.java index 073a79835f..0833c1d4a3 100644 --- a/ext/jni/src/org/sqlite/jni/SQLite3Jni.java +++ b/ext/jni/src/org/sqlite/jni/SQLite3Jni.java @@ -178,26 +178,17 @@ public final class SQLite3Jni { not have access to the sqlite3_api object which native auto-extensions do. - - If an auto-extension opens a db from the same thread, opening - will fail with SQLITE_BUSY. The alternative would be endless - recursion into the auto-extension. - - - The list of auto-extensions must not be manipulated from within - an auto-extension. Auto extensions can neither be added, - removed, nor cleared while one registered with this function is - running. Attempting to do so may lead to a deadlock. + - If the list of auto-extensions is manipulated from an + auto-extension, it is undefined which, if any, auto-extensions + will subsequently execute for the current database (it depends + on multiple factors). See the AutoExtension class docs for more information. Achtung: it is as yet unknown whether auto extensions registered from one JNIEnv (thread) can be safely called from another. - - Design note: this family of methods is synchronized in order to - help avoid a small race condition where an in-progress - sqlite3_reset_auto_extension() or sqlite3_cancel_auto_extension() - could cause sqlite3_open() to fail with SQLITE_BUSY. */ - public static synchronized native int sqlite3_auto_extension(@NotNull AutoExtension callback); + public static native int sqlite3_auto_extension(@NotNull AutoExtension callback); public static int sqlite3_bind_blob( @NotNull sqlite3_stmt stmt, int ndx, @Nullable byte[] data @@ -293,12 +284,7 @@ public final class SQLite3Jni { @NotNull sqlite3 db, int ms ); - /** - Works like the C API except that it returns false, without side - effects, if auto extensions are currently running. (The JNI-level - list of extensions cannot be manipulated while it is being traversed.) - */ - public static synchronized native boolean sqlite3_cancel_auto_extension( + public static native boolean sqlite3_cancel_auto_extension( @NotNull AutoExtension ax ); @@ -595,16 +581,13 @@ public final class SQLite3Jni { Recall that even if opening fails, the output pointer might be non-null. Any error message about the failure will be in that object and it is up to the caller to sqlite3_close() that - db handle. Passing a null to sqlite3_close() is legal. - - Design note: this method is synchronized in order to help - alleviate a race condition involving auto-extensions. + db handle. */ - public static synchronized native int sqlite3_open( + public static native int sqlite3_open( @Nullable String filename, @NotNull OutputPointer.sqlite3 ppDb ); - public static synchronized native int sqlite3_open_v2( + public static native int sqlite3_open_v2( @Nullable String filename, @NotNull OutputPointer.sqlite3 ppDb, int flags, @Nullable String zVfs ); @@ -728,7 +711,7 @@ public final class SQLite3Jni { extensions are currently running. (The JNI-level list of extensions cannot be manipulated while it is being traversed.) */ - public static synchronized native void sqlite3_reset_auto_extension(); + public static native void sqlite3_reset_auto_extension(); public static native void sqlite3_result_double( @NotNull sqlite3_context cx, double v diff --git a/manifest b/manifest index 8cb273b6f5..611838b934 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C More\swork\son\sthe\sJNI-specific\smutexes.\sRework\sthe\sNativePointerHolder\scache\slookup\sto\sbe\sslightly\ssimpler\sand\sO(1)\sinstead\sof\sO(N). -D 2023-08-14T13:27:40.885 +C Bring\shandling\sof\sthe\sJava\sauto-ext\shandler\smore\sin\sline\swith\sthe\score\sin\sterms\sof\slocking\sand\smutability\sduring\straversal.\sThis\sremoves\sthe\sexplicit\ssynchronous\srequirement\sfrom\sthe\sJava\sopen()\sand\sauto-ext\sbindings. +D 2023-08-14T17:12:55.531 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724 @@ -234,7 +234,7 @@ F ext/icu/sqliteicu.h fa373836ed5a1ee7478bdf8a1650689294e41d0c89c1daab26e9ae78a3 F ext/jni/GNUmakefile a9e11b92e620058558cbc1a2d49f8ec53c78d6a989b9db0b7d0b649b9f174881 F ext/jni/README.md 7a614a2fa6c561205f7a53fd8626cf93a7b5711ff454fc1814517f796df398eb F ext/jni/jar-dist.make f90a553203a57934bf275bed86479485135a52f48ac5c1cfe6499ae07b0b35a4 -F ext/jni/src/c/sqlite3-jni.c 0ca77c27d05b677191f105bc6f8570c916b78991a809d44566c60cfc16b50612 +F ext/jni/src/c/sqlite3-jni.c 4b93d970b142e62712f2cbdde01e1c5ed78af5f306238efad0e53276f26f1211 F ext/jni/src/c/sqlite3-jni.h f10d2f38720687c70ecdd5e44f6e8db98efee2caa05fc86b2d9e0c76e6cc0a18 F ext/jni/src/org/sqlite/jni/Authorizer.java 1308988f7f40579ea0e4deeaec3c6be971630566bd021c31367fe3f5140db892 F ext/jni/src/org/sqlite/jni/AutoExtension.java 18e83f6f463e306df60b2dceb65247d32af1f78af4bbbae9155411a8c6cdb093 @@ -254,7 +254,7 @@ F ext/jni/src/org/sqlite/jni/ProgressHandler.java 6f62053a828a572de809828b1ee495 F ext/jni/src/org/sqlite/jni/ResultCode.java ba701f20213a5f259e94cfbfdd36eb7ac7ce7797f2c6c7fca2004ff12ce20f86 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 cd0627b5317435f9a6c72247915f9e32d6e8c225fd6f0db2c66b4a7f0b4e5601 +F ext/jni/src/org/sqlite/jni/SQLite3Jni.java 5897c1d11f6c780825c7ac739270365e6312990195fc135fc6b02d5536dbae18 F ext/jni/src/org/sqlite/jni/Tester1.java 368e836d943d9e882d2a217d0f582ed4141d164f174bebc50715acd57549a09b F ext/jni/src/org/sqlite/jni/TesterFts5.java 59e22dd24af033ea8827d36225a2f3297908fb6af8818ead8850c6c6847557b1 F ext/jni/src/org/sqlite/jni/Tracer.java a5cece9f947b0af27669b8baec300b6dd7ff859c3e6a6e4a1bd8b50f9714775d @@ -2091,8 +2091,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 b62d93258b6a661f3a9b61468b3b641c14faf2d2196f78aca95fe14de43c9444 -R 640b629cd539e67d64418aa6a08729eb +P c84ded0e59aea4861d72b53b4b40cf580747c0f6ca58c334a996f1a825276cb5 +R 49ea94318acd6a04e4782997e6036b52 U stephan -Z db03beab64a32e82d4161306677f6282 +Z f760a4c17dc36361fbad3526fa5aa4c4 # Remove this line to create a well-formed Fossil manifest. diff --git a/manifest.uuid b/manifest.uuid index 0f159ac8d9..a0b706b5cf 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -c84ded0e59aea4861d72b53b4b40cf580747c0f6ca58c334a996f1a825276cb5 \ No newline at end of file +42994b952e092ae4fa319395208622e887387ca3ff8ac57961c824a6c272bf0e \ No newline at end of file