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

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;
     }
   }
 

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

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){

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) );

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.

manifest.uuid

@@ -1 +1 @@
-44b4df01ff86841fb85b6295cbada422c6ba8a32a420a2e840e2d607b6c90164
+efbc82b218d26b7ca9b881da69d5fd14d22b5211fbd85a835da50e5bfde3d160