Improve C-side exception handling from Java-side UDF callbacks.

FossilOrigin-Name: aebbc24afb339ed07b7cd767fbc0d25f3e9c3d9bb5ef3d1c10b04b605c7261bc

ext/jni/README.md

@@ -105,7 +105,9 @@ Client-defined callbacks _must never throw exceptions_ unless _very
 explicitly documented_ as being throw-safe. Exceptions are generally
 reserved for higher-level bindings which are constructed to
 specifically deal with them and ensure that they do not leak C-level
-resources.
+resources. In some cases, callback handlers (see below) are permitted
+to throw, in which cases they get translated to C-level result codes
+and/or messages.
 
 
 Unwieldy Constructs are Re-mapped

ext/jni/src/c/sqlite3-jni.c

@@ -1577,22 +1577,47 @@ error_oom:
   return SQLITE_NOMEM;
 }
 
-static int udf_report_exception(sqlite3_context * cx,
+/*
-                                const char *zFuncName,
+** Must be called immediately after a Java-side UDF callback throws.
-                                const char *zFuncType){
+** If translateToErr is true then it sets the exception's message in
-  int rc;
+** the result error using sqlite3_result_error(). If translateToErr is
-  char * z =
+** false then it emits a warning that the function threw but should
-    sqlite3_mprintf("Client-defined function %s.%s() threw. It should "
+** not do so. In either case, it clears the exception state.
-                    "not do that.",
+**
-                    zFuncName ? zFuncName : "<unnamed>", zFuncType);
+** Returns SQLITE_NOMEM if an allocation fails, else SQLITE_ERROR. In
-  if(z){
+** the latter case it calls sqlite3_result_error_nomem().
+*/
+static int udf_report_exception(JNIEnv * const env, int translateToErr,
+                                sqlite3_context * cx,
+                                const char *zFuncName, const char *zFuncType ){
+  jthrowable const ex = (*env)->ExceptionOccurred(env);
+  int rc = SQLITE_ERROR;
+
+  assert(ex && "This must only be called when a Java exception is pending.");
+  if( translateToErr ){
+    char * zMsg;
+    char * z;
+
+    EXCEPTION_CLEAR;
+    zMsg = s3jni_exception_error_msg(env, ex);
+    z = sqlite3_mprintf("Client-defined SQL function %s.%s() threw: %s",
+                        zFuncName ? zFuncName : "<unnamed>", zFuncType,
+                        zMsg ? zMsg : "Unknown exception" );
+    sqlite3_free(zMsg);
+    if( z ){
       sqlite3_result_error(cx, z, -1);
       sqlite3_free(z);
-    rc = SQLITE_ERROR;
     }else{
       sqlite3_result_error_nomem(cx);
       rc = SQLITE_NOMEM;
     }
+  }else{
+    MARKER(("Client-defined SQL function %s.%s() threw. "
+            "It should not do that.\n",
+            zFuncName ? zFuncName : "<unnamed>", zFuncType));
+    (*env)->ExceptionDescribe( env );
+    EXCEPTION_CLEAR;
+  }
   return rc;
 }
 
@@ -1600,24 +1625,25 @@ static int udf_report_exception(sqlite3_context * cx,
    Sets up the state for calling a Java-side xFunc/xStep/xInverse()
    UDF, calls it, and returns 0 on success.
 */
-static int udf_xFSI(sqlite3_context* pCx, int argc,
+static int udf_xFSI(sqlite3_context* const pCx, int argc,
-                    sqlite3_value** argv,
+                    sqlite3_value** const argv,
-                    S3JniUdf * s,
+                    S3JniUdf * const s,
                     jmethodID xMethodID,
-                    const char * zFuncType){
+                    const char * const zFuncType){
   LocalJniGetEnv;
   udf_jargs args = {0,0};
   int rc = udf_args(env, pCx, argc, argv, &args.jcx, &args.jargv);
-  //MARKER(("%s.%s() pCx = %p\n", s->zFuncName, zFuncType, pCx));
+
-  if(rc) return rc;
   //MARKER(("UDF::%s.%s()\n", s->zFuncName, zFuncType));
+  if(rc) return rc;
   if( UDF_SCALAR != s->type ){
     rc = udf_setAggregateContext(env, args.jcx, pCx, 0);
   }
   if( 0 == rc ){
     (*env)->CallVoidMethod(env, s->jObj, xMethodID, args.jcx, args.jargv);
     IFTHREW{
-      rc = udf_report_exception(pCx, s->zFuncName, zFuncType);
+      rc = udf_report_exception(env, 'F'==zFuncType[1]/*xFunc*/, pCx,
+                                s->zFuncName, zFuncType);
     }
   }
   UNREF_L(args.jcx);
@@ -1635,19 +1661,21 @@ static int udf_xFV(sqlite3_context* cx, S3JniUdf * s,
   LocalJniGetEnv;
   jobject jcx = new_sqlite3_context_wrapper(env, cx);
   int rc = 0;
+  int const isFinal = 'F'==zFuncType[1]/*xFinal*/;
   //MARKER(("%s.%s() cx = %p\n", s->zFuncName, zFuncType, cx));
   if(!jcx){
-    sqlite3_result_error_nomem(cx);
+    if( isFinal ) sqlite3_result_error_nomem(cx);
     return SQLITE_NOMEM;
   }
   //MARKER(("UDF::%s.%s()\n", s->zFuncName, zFuncType));
   if( UDF_SCALAR != s->type ){
-    rc = udf_setAggregateContext(env, jcx, cx, 1);
+    rc = udf_setAggregateContext(env, jcx, cx, isFinal);
   }
   if( 0 == rc ){
     (*env)->CallVoidMethod(env, s->jObj, xMethodID, jcx);
     IFTHREW{
-      rc = udf_report_exception(cx,s->zFuncName, zFuncType);
+      rc = udf_report_exception(env, isFinal, cx, s->zFuncName,
+                                zFuncType);
     }
   }
   UNREF_L(jcx);
@@ -3599,8 +3627,7 @@ static void s3jni_fts5_extension_function(Fts5ExtensionApi const *pApi,
   (*env)->CallVoidMethod(env, pAux->jObj, pAux->jmid,
                          jFXA, jpFts, jpCx, jArgv);
   IFTHREW{
-    EXCEPTION_CLEAR;
+    udf_report_exception(env, 1, pCx, pAux->zFuncName, "xFunction");
-    udf_report_exception(pCx, pAux->zFuncName, "xFunction");
   }
   UNREF_L(jpFts);
   UNREF_L(jpCx);

ext/jni/src/org/sqlite/jni/SQLFunction.java

@@ -98,7 +98,11 @@ public abstract class SQLFunction {
   //! Subclass for creating scalar functions.
   public static abstract class Scalar extends SQLFunction {
 
-    //! As for the xFunc() argument of the C API's sqlite3_create_function()
+    /**
+       As for the xFunc() argument of the C API's
+       sqlite3_create_function().  If this function throws, it is
+       translated into an sqlite3_result_error().
+    */
     public abstract void xFunc(sqlite3_context cx, sqlite3_value[] args);
 
     /**
@@ -116,10 +120,18 @@ public abstract class SQLFunction {
   */
   public static abstract class Aggregate<T> extends SQLFunction {
 
-    //! As for the xStep() argument of the C API's sqlite3_create_function()
+    /**
+       As for the xStep() argument of the C API's
+       sqlite3_create_function().  If this function throws, the
+       exception is not propagated and a warning might be emitted to a
+       debugging channel.
+    */
     public abstract void xStep(sqlite3_context cx, sqlite3_value[] args);
 
-    //! As for the xFinal() argument of the C API's sqlite3_create_function()
+    /**
+       As for the xFinal() argument of the C API's sqlite3_create_function().
+       If this function throws, it is translated into an sqlite3_result_error().
+    */
     public abstract void xFinal(sqlite3_context cx);
 
     /**
@@ -165,10 +177,18 @@ public abstract class SQLFunction {
   */
   public static abstract class Window<T> extends Aggregate<T> {
 
-    //! As for the xInverse() argument of the C API's sqlite3_create_window_function()
+    /**
+       As for the xInverse() argument of the C API's
+       sqlite3_create_window_function(). If this function throws, the
+       exception is not propagated and a warning might be emitted
+       to a debugging channel.
+    */
     public abstract void xInverse(sqlite3_context cx, sqlite3_value[] args);
 
-    //! As for the xValue() argument of the C API's sqlite3_create_window_function()
+    /**
+       As for the xValue() argument of the C API's sqlite3_create_window_function().
+       See xInverse() for the fate of any exceptions this throws.
+    */
     public abstract void xValue(sqlite3_context cx);
   }
 }

ext/jni/src/org/sqlite/jni/Tester1.java

@@ -158,16 +158,22 @@ public class Tester1 implements Runnable {
     execSql(db, true, sql);
   }
 
-  static sqlite3_stmt prepare(sqlite3 db, String sql){
+  static sqlite3_stmt prepare(sqlite3 db, boolean throwOnError, String sql){
     final OutputPointer.sqlite3_stmt outStmt = new OutputPointer.sqlite3_stmt();
     int rc = sqlite3_prepare(db, sql, outStmt);
+    if( throwOnError ){
       affirm( 0 == rc );
+    }
     final sqlite3_stmt rv = outStmt.take();
     affirm( null == outStmt.get() );
+    if( throwOnError ){
       affirm( 0 != rv.getNativePointer() );
+    }
     return rv;
   }
-
+  static sqlite3_stmt prepare(sqlite3 db, String sql){
+    return prepare(db, true, sql);
+  }
   private void showCompileOption(){
     int i = 0;
     String optName;
@@ -598,6 +604,47 @@ public class Tester1 implements Runnable {
     affirm( xDestroyCalled.value );
   }
 
+  private void testUdfThrows(){
+    final sqlite3 db = createNewDb();
+    final ValueHolder<Integer> xFuncAccum = new ValueHolder<>(0);
+
+    SQLFunction funcAgg = new SQLFunction.Aggregate<Integer>(){
+        @Override public void xStep(sqlite3_context cx, sqlite3_value[] args){
+          /** Throwing from here should emit loud noise on stdout or stderr
+              but the exception is supressed because we have no way to inform
+              sqlite about it from these callbacks. */
+          //throw new RuntimeException("Throwing from an xStep");
+        }
+        @Override public void xFinal(sqlite3_context cx){
+          throw new RuntimeException("Throwing from an xFinal");
+        }
+      };
+    int rc = sqlite3_create_function(db, "myagg", 1, SQLITE_UTF8, funcAgg);
+    affirm(0 == rc);
+    affirm(0 == xFuncAccum.value);
+    sqlite3_stmt stmt = prepare(db, "SELECT myagg(1)");
+    rc = sqlite3_step(stmt);
+    sqlite3_finalize(stmt);
+    affirm( 0 != rc );
+    affirm( sqlite3_errmsg(db).indexOf("an xFinal") > 0 );
+
+    SQLFunction funcSc = new SQLFunction.Scalar(){
+        @Override public void xFunc(sqlite3_context cx, sqlite3_value[] args){
+          throw new RuntimeException("Throwing from an xFunc");
+        }
+      };
+    rc = sqlite3_create_function(db, "mysca", 0, SQLITE_UTF8, funcSc);
+    affirm(0 == rc);
+    affirm(0 == xFuncAccum.value);
+    stmt = prepare(db, "SELECT mysca()");
+    rc = sqlite3_step(stmt);
+    sqlite3_finalize(stmt);
+    affirm( 0 != rc );
+    affirm( sqlite3_errmsg(db).indexOf("an xFunc") > 0 );
+
+    sqlite3_close_v2(db);
+  }
+
   private void testUdfJavaObject(){
     final sqlite3 db = createNewDb();
     final ValueHolder<sqlite3> testResult = new ValueHolder<>(db);

manifest

@@ -1,5 +1,5 @@
-C Minor\sTester1.java\scleanups.
+C Improve\sC-side\sexception\shandling\sfrom\sJava-side\sUDF\scallbacks.
-D 2023-08-22T23:00:44.674
+D 2023-08-23T00:17:28.362
 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1
 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea
 F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724
@@ -233,9 +233,9 @@ F ext/icu/README.txt 7ab7ced8ae78e3a645b57e78570ff589d4c672b71370f5aa9e1cd7024f4
 F ext/icu/icu.c c074519b46baa484bb5396c7e01e051034da8884bad1a1cb7f09bbe6be3f0282
 F ext/icu/sqliteicu.h fa373836ed5a1ee7478bdf8a1650689294e41d0c89c1daab26e9ae78a32075a8
 F ext/jni/GNUmakefile a38d7c5ad4ab1e209e2a9352ff06add1d9a0bc666351714bfc8858625330c16b
-F ext/jni/README.md 975b35173debbbf3a4ab7166e14d2ffa2bacff9b6850414f09cc919805e81ba4
+F ext/jni/README.md ddcc6be0c0d65f1e2fd687de9f40d38c82630fd61f83cc9550773caa19dd8be1
 F ext/jni/jar-dist.make 9a03d10dbb5a74c724bfec4b76fd9e4c9865cbbc858d731cb48f38ac897d73a3
-F ext/jni/src/c/sqlite3-jni.c 1064441d33cef541c9c9c84fb5093573a0767bb36563e6ea538c355b6148c4c6
+F ext/jni/src/c/sqlite3-jni.c 8e7dac68246edec8d85e34104d5c749c544ae10c55b1bbf6eeff116447da7f54
 F ext/jni/src/c/sqlite3-jni.h 8b0ab1a3f0f92b75d4ff50db4a88b66a137cfb561268eb15bb3993ed174dbb74
 F ext/jni/src/org/sqlite/jni/Authorizer.java 1308988f7f40579ea0e4deeaec3c6be971630566bd021c31367fe3f5140db892
 F ext/jni/src/org/sqlite/jni/AutoExtension.java 3b62c915e45ce73f63343ca9195ec63592244d616a1908b7587bdd45de1b97dd
@@ -254,9 +254,9 @@ F ext/jni/src/org/sqlite/jni/OutputPointer.java 464ea85c3eba673a7b575545f69fcd8a
 F ext/jni/src/org/sqlite/jni/ProgressHandler.java 6f62053a828a572de809828b1ee495380677e87daa29a1c57a0e2c06b0a131dc
 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 8c1ad92c35bcc1b2f7256cf6e229b31340ed6d1a404d487f0a9adb28ba7fc332
+F ext/jni/src/org/sqlite/jni/SQLFunction.java f697cf2a81c4119f2baf0682af689686f0466f1dd83dba00885f5603e693fe16
 F ext/jni/src/org/sqlite/jni/SQLite3Jni.java 2f36370cfdec01d309720392b2c3e4af6afce0b6ece8188b5c3ed688a5a1e63a
-F ext/jni/src/org/sqlite/jni/Tester1.java 1a9d9a0ce93aa105dc409273bb50bf10f15e8b546d9ef0922b14e69dc3f14107
+F ext/jni/src/org/sqlite/jni/Tester1.java 93bd76f2294fa2f592395c9d823adb38a9be3846bff00debeff02310f4e9e6be
 F ext/jni/src/org/sqlite/jni/TesterFts5.java de095e3b701fba0c56d7b8b2993dc22bcbaa9de8f992904a93729ad729a91576
 F ext/jni/src/org/sqlite/jni/Tracer.java a5cece9f947b0af27669b8baec300b6dd7ff859c3e6a6e4a1bd8b50f9714775d
 F ext/jni/src/org/sqlite/jni/UpdateHook.java e58645a1727f8a9bbe72dc072ec5b40d9f9362cb0aa24acfe93f49ff56a9016d
@@ -2092,8 +2092,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 56b2a077ace6e6ad5834e1a597b710f212a5b7d5db5b9a27a41f2aa0f6952c55
+P 70d936953ba55cfed32efe7e3a9d4b71da9a7ffc8818b6540471e0bf311bc688
-R 786162271678eb8ff4639708ab69b4fa
+R f4b3990a7d714ecf1a549380689ce9aa
 U stephan
-Z 0962b73394fcdbf4989509de32c7e31a
+Z b8494717453324d170d4924e0442cf8e
 # Remove this line to create a well-formed Fossil manifest.

manifest.uuid

@@ -1 +1 @@
-70d936953ba55cfed32efe7e3a9d4b71da9a7ffc8818b6540471e0bf311bc688
+aebbc24afb339ed07b7cd767fbc0d25f3e9c3d9bb5ef3d1c10b04b605c7261bc