From: stephan Date: Mon, 13 Nov 2023 23:11:10 +0000 (+0000) Subject: JNI: add sqlite3_column_nio_buffer() and sqlite3_value_nio_buffer() using an only... X-Git-Tag: version-3.45.0~168 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=ce2edab088b68c928f388d83493ac9c7b812224c;p=thirdparty%2Fsqlite.git JNI: add sqlite3_column_nio_buffer() and sqlite3_value_nio_buffer() using an only-slightly roundabout approach to creating properly-sized ByteBuffer objects. FossilOrigin-Name: efbc82b218d26b7ca9b881da69d5fd14d22b5211fbd85a835da50e5bfde3d160 --- diff --git a/ext/jni/src/c/sqlite3-jni.c b/ext/jni/src/c/sqlite3-jni.c index e04b3ab5f3..4028505a87 100644 --- a/ext/jni/src/c/sqlite3-jni.c +++ b/ext/jni/src/c/sqlite3-jni.c @@ -15,13 +15,14 @@ /* ** If you found this comment by searching the code for -** CallStaticObjectMethod then you're the victim of an OpenJDK bug: +** CallStaticObjectMethod because it appears in console output then +** you're probably the victim of an OpenJDK bug: ** ** https://bugs.openjdk.org/browse/JDK-8130659 ** -** It's known to happen with OpenJDK v8 but not with v19. -** -** This code does not use JNI's CallStaticObjectMethod(). +** It's known to happen with OpenJDK v8 but not with v19. It was +** triggered by this code long before it made any use of +** CallStaticObjectMethod(). */ /* @@ -664,6 +665,7 @@ struct S3JniGlobalType { ByteBuffer is available (which we determine during static init). */ jclass cByteBuffer /* global ref to java.nio.ByteBuffer */; + jmethodID byteBufferAlloc/* ByteBuffer.allocateDirect() */; } g; /* ** The list of Java-side auto-extensions @@ -1093,6 +1095,47 @@ static jstring s3jni_text16_to_jstring(JNIEnv * const env, const void * const p, return rv; } +/* +** Creates a new ByteBuffer instance with a capacity of n. assert()s +** that SJG.g.cByteBuffer is not 0 and n>0. +*/ +static jobject s3jni__new_ByteBuffer(JNIEnv * const env, int n){ + jobject rv = 0; + assert( SJG.g.cByteBuffer ); + assert( SJG.g.byteBufferAlloc ); + assert( n > 0 ); + rv = (*env)->CallStaticObjectMethod(env, SJG.g.cByteBuffer, + SJG.g.byteBufferAlloc, (jint)n); + S3JniIfThrew { + S3JniExceptionReport; + S3JniExceptionClear; + } + s3jni_oom_check( rv ); + return rv; +} + +/* +** If n>0 and sqlite3_jni_supports_nio() is true then this creates a +** new ByteBuffer object and copies n bytes from p to it. Returns NULL +** if n is 0, sqlite3_jni_supports_nio() is false, or on allocation +** error (unless fatal alloc failures are enabled). +*/ +static jobject s3jni__blob_to_ByteBuffer(JNIEnv * const env, + const void * p, int n){ + jobject rv = NULL; + assert( n >= 0 ); + if( 0==n || !SJG.g.cByteBuffer ){ + return NULL; + } + rv = s3jni__new_ByteBuffer(env, n); + if( rv ){ + void * tgt = (*env)->GetDirectBufferAddress(env, rv); + memcpy(tgt, p, (size_t)n); + } + return rv; +} + + /* ** Requires jx to be a Throwable. Calls its toString() method and ** returns its value converted to a UTF-8 string. The caller owns the @@ -1936,7 +1979,7 @@ static void udf_unargs(JNIEnv *env, jobject jCx, int argc, jobjectArray jArgv){ NativePointerHolder_set(S3JniNph(sqlite3_context), jCx, 0); for( ; i < argc; ++i ){ jobject jsv = (*env)->GetObjectArrayElement(env, jArgv, i); - assert(jsv); + assert(jsv && "Someone illegally modified a UDF argument array."); NativePointerHolder_set(S3JniNph(sqlite3_value), jsv, 0); } } @@ -3022,6 +3065,21 @@ S3JniApi(sqlite3_column_java_object(),jobject,1column_1java_1object)( return rv; } +S3JniApi(sqlite3_value_nio_buffer(),jobject,1column_1nio_1buffer)( + JniArgsEnvClass, jobject jStmt, jint ndx +){ + sqlite3_stmt * const stmt = PtrGet_sqlite3_stmt(jStmt); + jobject rv = 0; + if( stmt ){ + const void * const p = sqlite3_column_blob(stmt, (int)ndx); + if( p ){ + const int n = sqlite3_column_bytes(stmt, (int)ndx); + rv = s3jni__blob_to_ByteBuffer(env, p, n); + } + } + return rv; +} + S3JniApi(sqlite3_column_text(),jbyteArray,1column_1text)( JniArgsEnvClass, jobject jpStmt, jint ndx ){ @@ -5090,6 +5148,21 @@ S3JniApi(sqlite3_value_java_object(),jobject,1value_1java_1object)( : 0; } +S3JniApi(sqlite3_value_nio_buffer(),jobject,1value_1nio_1buffer)( + JniArgsEnvClass, jobject jVal +){ + sqlite3_value * const sv = PtrGet_sqlite3_value(jVal); + jobject rv = 0; + if( sv ){ + const void * const p = sqlite3_value_blob(sv); + if( p ){ + const int n = sqlite3_value_bytes(sv); + rv = s3jni__blob_to_ByteBuffer(env, p, n); + } + } + return rv; +} + S3JniApi(sqlite3_value_text(),jbyteArray,1value_1text)( JniArgsEnvClass, jlong jpSVal ){ @@ -6096,10 +6169,15 @@ Java_org_sqlite_jni_capi_CApi_init(JniArgsEnvClass){ unsigned char buf[16] = {0}; jobject bb = (*env)->NewDirectByteBuffer(env, buf, 16); if( bb ){ - SJG.g.cByteBuffer = (*env)->GetObjectClass(env, bb); + SJG.g.cByteBuffer = S3JniRefGlobal((*env)->GetObjectClass(env, bb)); + SJG.g.byteBufferAlloc = (*env)->GetStaticMethodID( + env, SJG.g.cByteBuffer, "allocateDirect", "(I)Ljava/nio/ByteBuffer;" + ); + S3JniExceptionIsFatal("Error getting ByteBuffer.allocateDirect() method."); S3JniUnrefLocal(bb); }else{ SJG.g.cByteBuffer = 0; + SJG.g.byteBufferAlloc = 0; } } diff --git a/ext/jni/src/c/sqlite3-jni.h b/ext/jni/src/c/sqlite3-jni.h index bf1cc56d1e..c9034dbeed 100644 --- a/ext/jni/src/c/sqlite3-jni.h +++ b/ext/jni/src/c/sqlite3-jni.h @@ -1105,6 +1105,14 @@ JNIEXPORT jint JNICALL Java_org_sqlite_jni_capi_CApi_sqlite3_1column_1bytes16 JNIEXPORT jint JNICALL Java_org_sqlite_jni_capi_CApi_sqlite3_1column_1count (JNIEnv *, jclass, jlong); +/* + * Class: org_sqlite_jni_capi_CApi + * Method: sqlite3_column_database_name + * Signature: (JI)Ljava/lang/String; + */ +JNIEXPORT jstring JNICALL Java_org_sqlite_jni_capi_CApi_sqlite3_1column_1database_1name + (JNIEnv *, jclass, jlong, jint); + /* * Class: org_sqlite_jni_capi_CApi * Method: sqlite3_column_decltype @@ -1155,11 +1163,11 @@ JNIEXPORT jstring JNICALL Java_org_sqlite_jni_capi_CApi_sqlite3_1column_1name /* * Class: org_sqlite_jni_capi_CApi - * Method: sqlite3_column_database_name - * Signature: (JI)Ljava/lang/String; + * Method: sqlite3_column_nio_buffer + * Signature: (Lorg/sqlite/jni/capi/sqlite3_stmt;I)Ljava/nio/ByteBuffer; */ -JNIEXPORT jstring JNICALL Java_org_sqlite_jni_capi_CApi_sqlite3_1column_1database_1name - (JNIEnv *, jclass, jlong, jint); +JNIEXPORT jobject JNICALL Java_org_sqlite_jni_capi_CApi_sqlite3_1column_1nio_1buffer + (JNIEnv *, jclass, jobject, jint); /* * Class: org_sqlite_jni_capi_CApi @@ -2105,6 +2113,14 @@ JNIEXPORT jlong JNICALL Java_org_sqlite_jni_capi_CApi_sqlite3_1value_1int64 JNIEXPORT jobject JNICALL Java_org_sqlite_jni_capi_CApi_sqlite3_1value_1java_1object (JNIEnv *, jclass, jlong); +/* + * Class: org_sqlite_jni_capi_CApi + * Method: sqlite3_value_nio_buffer + * Signature: (Lorg/sqlite/jni/capi/sqlite3_value;)Ljava/nio/ByteBuffer; + */ +JNIEXPORT jobject JNICALL Java_org_sqlite_jni_capi_CApi_sqlite3_1value_1nio_1buffer + (JNIEnv *, jclass, jobject); + /* * Class: org_sqlite_jni_capi_CApi * Method: sqlite3_value_nochange diff --git a/ext/jni/src/org/sqlite/jni/capi/CApi.java b/ext/jni/src/org/sqlite/jni/capi/CApi.java index d3d434f7d6..cd2a09e8ce 100644 --- a/ext/jni/src/org/sqlite/jni/capi/CApi.java +++ b/ext/jni/src/org/sqlite/jni/capi/CApi.java @@ -318,10 +318,6 @@ public final class CApi { buffers and the only such class offered by Java is (apparently) ByteBuffer. - Design note: there are no sqlite3_column_nio_buffer() and - sqlite3_value_nio_buffer() counterparts because the ByteBuffer - interface does not enable sensible implementations of those. - @see https://docs.oracle.com/javase/8/docs/api/java/nio/Buffer.html */ public static native int sqlite3_bind_nio_buffer( @@ -646,6 +642,15 @@ public final class CApi { return sqlite3_column_count(stmt.getNativePointer()); } + private static native String sqlite3_column_database_name(@NotNull long ptrToStmt, int ndx); + + /** + Only available if built with SQLITE_ENABLE_COLUMN_METADATA. + */ + public static String sqlite3_column_database_name(@NotNull sqlite3_stmt stmt, int ndx){ + return sqlite3_column_database_name(stmt.getNativePointer(), ndx); + } + private static native String sqlite3_column_decltype(@NotNull long ptrToStmt, int ndx); public static String sqlite3_column_decltype(@NotNull sqlite3_stmt stmt, int ndx){ @@ -700,14 +705,15 @@ public final class CApi { return sqlite3_column_name(stmt.getNativePointer(), ndx); } - private static native String sqlite3_column_database_name(@NotNull long ptrToStmt, int ndx); - /** - Only available if built with SQLITE_ENABLE_COLUMN_METADATA. + A variant of sqlite3_column_blob() which returns the blob as a + ByteBuffer object. Returns null if its argument is null, if + sqlite3_jni_supports_nio() is false, or if sqlite3_column_blob() + would return null for the same inputs. */ - public static String sqlite3_column_database_name(@NotNull sqlite3_stmt stmt, int ndx){ - return sqlite3_column_database_name(stmt.getNativePointer(), ndx); - } + public static native java.nio.ByteBuffer sqlite3_column_nio_buffer( + @NotNull sqlite3_stmt stmt, int ndx + ); private static native String sqlite3_column_origin_name(@NotNull long ptrToStmt, int ndx); @@ -1418,6 +1424,15 @@ public final class CApi { If the C API was built with SQLITE_ENABLE_PREUPDATE_HOOK defined, this acts as a proxy for C's sqlite3_preupdate_new(), else it returns SQLITE_MISUSE with no side effects. + + WARNING: client code _must not_ hold a reference to the returned + sqlite3_value object beyond the scope of the preupdate hook in + which this function is called. Doing so will leave the client + holding a stale pointer, the address of which could point to + anything at all after the pre-update hook is complete. This API + has no way to record such objects and clear/invalidate them at + the end of a pre-update hook. We "could" add infrastructure to do + so, but would require significant levels of bookkeeping. */ public static int sqlite3_preupdate_new(@NotNull sqlite3 db, int col, @NotNull OutputPointer.sqlite3_value out){ @@ -1441,6 +1456,9 @@ public final class CApi { If the C API was built with SQLITE_ENABLE_PREUPDATE_HOOK defined, this acts as a proxy for C's sqlite3_preupdate_old(), else it returns SQLITE_MISUSE with no side effects. + + WARNING: see warning in sqlite3_preupdate_new() regarding the + potential for stale sqlite3_value handles. */ public static int sqlite3_preupdate_old(@NotNull sqlite3 db, int col, @NotNull OutputPointer.sqlite3_value out){ @@ -2110,6 +2128,16 @@ public final class CApi { return type.isInstance(o) ? (T)o : null; } + /** + A variant of sqlite3_column_blob() which returns the blob as a + ByteBuffer object. Returns null if its argument is null, if + sqlite3_jni_supports_nio() is false, or if sqlite3_value_blob() + would return null for the same input. + */ + public static native java.nio.ByteBuffer sqlite3_value_nio_buffer( + @NotNull sqlite3_value v + ); + private static native int sqlite3_value_nochange(@NotNull long ptrToValue); public static int sqlite3_value_nochange(@NotNull sqlite3_value v){ diff --git a/ext/jni/src/org/sqlite/jni/capi/Tester1.java b/ext/jni/src/org/sqlite/jni/capi/Tester1.java index b50e3eaac1..9e9f9a8843 100644 --- a/ext/jni/src/org/sqlite/jni/capi/Tester1.java +++ b/ext/jni/src/org/sqlite/jni/capi/Tester1.java @@ -494,6 +494,7 @@ public class Tester1 implements Runnable { stmt = prepare(db, "SELECT a FROM t ORDER BY a DESC;"); StringBuilder sbuf = new StringBuilder(); n = 0; + final boolean tryNio = sqlite3_jni_supports_nio(); while( SQLITE_ROW == sqlite3_step(stmt) ){ final sqlite3_value sv = sqlite3_value_dup(sqlite3_column_value(stmt,0)); final String txt = sqlite3_column_text16(stmt, 0); @@ -508,6 +509,15 @@ public class Tester1 implements Runnable { StandardCharsets.UTF_8)) ); affirm( txt.length() == sqlite3_value_bytes16(sv)/2 ); affirm( txt.equals(sqlite3_value_text16(sv)) ); + if( tryNio ){ + java.nio.ByteBuffer bu = sqlite3_value_nio_buffer(sv); + byte ba[] = sqlite3_value_blob(sv); + affirm( ba.length == bu.capacity() ); + int i = 0; + for( byte b : ba ){ + affirm( b == bu.get(i++) ); + } + } sqlite3_value_free(sv); ++n; } @@ -570,6 +580,10 @@ public class Tester1 implements Runnable { /* TODO: these tests need to be much more extensive to check the begin/end range handling. */ + java.nio.ByteBuffer zeroCheck = + java.nio.ByteBuffer.allocateDirect(0); + affirm( null != zeroCheck ); + zeroCheck = null; sqlite3 db = createNewDb(); execSql(db, "CREATE TABLE t(a)"); @@ -590,11 +604,17 @@ public class Tester1 implements Runnable { int total = 0; affirm( SQLITE_ROW == sqlite3_step(stmt) ); byte blob[] = sqlite3_column_blob(stmt, 0); + java.nio.ByteBuffer nioBlob = + sqlite3_column_nio_buffer(stmt, 0); affirm(3 == blob.length); + affirm(blob.length == nioBlob.capacity()); + affirm(blob.length == nioBlob.limit()); int i = 0; for(byte b : blob){ affirm( i<=3 ); - affirm(b == buf.get(1 + i++)); + affirm(b == buf.get(1 + i)); + affirm(b == nioBlob.get(i)); + ++i; total += b; } affirm( SQLITE_DONE == sqlite3_step(stmt) ); diff --git a/manifest b/manifest index c74882a340..af3d7483a2 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C JNI:\sadd\ssqlite3_result_nio_buffer()\sand\stests.\sDiscover\sthat\swe\scannot\screate\ssensible\ssqlite3_column_nio_buffer()\sor\ssqlite3_value_nio_buffer()\scounterparts\sbecause\sof\sByteBuffer\sinterface\slimitations. -D 2023-11-13T18:35:37.387 +C JNI:\sadd\ssqlite3_column_nio_buffer()\sand\ssqlite3_value_nio_buffer()\susing\san\sonly-slightly\sroundabout\sapproach\sto\screating\sproperly-sized\sByteBuffer\sobjects. +D 2023-11-13T23:11:10.170 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724 @@ -241,8 +241,8 @@ F ext/icu/sqliteicu.h fa373836ed5a1ee7478bdf8a1650689294e41d0c89c1daab26e9ae78a3 F ext/jni/GNUmakefile f2f3a31923293659b95225e932a286af1f2287d75bf88ad6c0fd1b9d9cd020d4 F ext/jni/README.md ef9ac115e97704ea995d743b4a8334e23c659e5534c3b64065a5405256d5f2f4 F ext/jni/jar-dist.make 030aaa4ae71dd86e4ec5e7c1e6cd86f9dfa47c4592c070d2e35157e42498e1fa -F ext/jni/src/c/sqlite3-jni.c 56ddb35bbae71123bedd003ae2bf42adae7d683717bb15ac3ed51d16e2b6c3ff -F ext/jni/src/c/sqlite3-jni.h 1f28a6a2beec13a49efdb42804926d2139f6a786558ea52c121ddf0bf39af59a +F ext/jni/src/c/sqlite3-jni.c c63250298bf0bdc335704d877f559dcdefaa041ba0556022b13599d43badb822 +F ext/jni/src/c/sqlite3-jni.h bd95cd17bfe2c7bd5e865a785bc5ed99591f1a1581c9b1a1b9ba7d65fb41d4cc F ext/jni/src/org/sqlite/jni/annotation/NotNull.java 02091a8112e33389f1c160f506cd413168c8dfacbeda608a4946c6e3557b7d5a F ext/jni/src/org/sqlite/jni/annotation/Nullable.java 0b1879852707f752512d4db9d7edd0d8db2f0c2612316ce1c832715e012ff6ba F ext/jni/src/org/sqlite/jni/annotation/package-info.java 977b374aed9d5853cbf3438ba3b0940abfa2ea4574f702a2448ee143b98ac3ca @@ -251,7 +251,7 @@ F ext/jni/src/org/sqlite/jni/capi/AggregateFunction.java 0b72cdff61533b564d65b63 F ext/jni/src/org/sqlite/jni/capi/AuthorizerCallback.java c045a5b47e02bb5f1af91973814a905f12048c428a3504fbc5266d1c1be3de5a F ext/jni/src/org/sqlite/jni/capi/AutoExtensionCallback.java 74cc4998a73d6563542ecb90804a3c4f4e828cb4bd69e61226d1a51f4646e759 F ext/jni/src/org/sqlite/jni/capi/BusyHandlerCallback.java 7b8e19810c42b0ad21a04b5d8c804b32ee5905d137148703f16a75b612c380ca -F ext/jni/src/org/sqlite/jni/capi/CApi.java 1517e1f5fc53001b2cb7229a427d33747163f8f37348368fe7c3e14edbb9c512 +F ext/jni/src/org/sqlite/jni/capi/CApi.java bf81e122cff8a6d88b5185e57e38f5e2c3387a4e84ee6732aff234da6966d21f F ext/jni/src/org/sqlite/jni/capi/CallbackProxy.java 57e2d275dcebe690b1fc1f3d34eb96879b2d7039bce30b563aee547bf45d8a8b F ext/jni/src/org/sqlite/jni/capi/CollationCallback.java e29bcfc540fdd343e2f5cca4d27235113f2886acb13380686756d5cabdfd065a F ext/jni/src/org/sqlite/jni/capi/CollationNeededCallback.java 5bfa226a8e7a92e804fd52d6e42b4c7b875fa7a94f8e2c330af8cc244a8920ab @@ -269,7 +269,7 @@ F ext/jni/src/org/sqlite/jni/capi/SQLFunction.java 0d1e9afc9ff8a2adb94a155b72385 F ext/jni/src/org/sqlite/jni/capi/SQLTester.java 09bee15aa0eedac68d767ae21d9a6a62a31ade59182a3ccbf036d6463d9e30b1 F ext/jni/src/org/sqlite/jni/capi/ScalarFunction.java 93b9700fca4c68075ccab12fe0fbbc76c91cafc9f368e835b9bd7cd7732c8615 F ext/jni/src/org/sqlite/jni/capi/TableColumnMetadata.java addf120e0e76e5be1ff2260daa7ce305ff9b5fafd64153a7a28e9d8f000a815f -F ext/jni/src/org/sqlite/jni/capi/Tester1.java 4d460ccce50e7ef4ce8e1ab5d46ceca70834a798d8a2233c93f4bede9deb6683 +F ext/jni/src/org/sqlite/jni/capi/Tester1.java 646b75c3ec454a2daa20fef1c15ed76f24882f8e529ff3749779bc2ece3c2820 F ext/jni/src/org/sqlite/jni/capi/TraceV2Callback.java 0a25e117a0daae3394a77f24713e36d7b44c67d6e6d30e9e1d56a63442eef723 F ext/jni/src/org/sqlite/jni/capi/UpdateHookCallback.java c8bdf7848e6599115d601bcc9427ff902cb33129b9be32870ac6808e04b6ae56 F ext/jni/src/org/sqlite/jni/capi/ValueHolder.java 22d365746a78c5cd7ae10c39444eb7bbf1a819aad4bb7eb77b1edc47773a3950 @@ -2139,8 +2139,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 b10ce1ef82d84726fbf6a8f624d6530f84fefb505f7868b4a0ea910fed7a877f -R 2f05bfa531dff8515ddabab7cc0cd5f1 +P 44b4df01ff86841fb85b6295cbada422c6ba8a32a420a2e840e2d607b6c90164 +R 723ebc4fdaf6a9b23550997df97352fc U stephan -Z acc87a8d47162ab6fa5eb71b4d2c3712 +Z bec7a2465a92db0fb274e1f6df96d01f # Remove this line to create a well-formed Fossil manifest. diff --git a/manifest.uuid b/manifest.uuid index b2e5d6bd1c..bcd471c105 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -44b4df01ff86841fb85b6295cbada422c6ba8a32a420a2e840e2d607b6c90164 \ No newline at end of file +efbc82b218d26b7ca9b881da69d5fd14d22b5211fbd85a835da50e5bfde3d160 \ No newline at end of file