Skip to content

Commit 7d0f25c

Browse files
committed
fix: tolerate missing CometUdfBridge class at JVMClasses init
Make comet_udf_bridge an Option in JVMClasses so a missing org.apache.comet.udf.CometUdfBridge class (e.g. shading dropped org.apache.comet.udf.*) no longer crashes executor JVM init. The JVM-UDF dispatch path returns a clear ExecutionError when the bridge is unavailable. Also clarify the FFI lifetime contract on the result import.
1 parent fee5ab2 commit 7d0f25c

2 files changed

Lines changed: 27 additions & 6 deletions

File tree

native/jni-bridge/src/lib.rs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,9 @@ pub struct JVMClasses<'a> {
220220
/// acquire & release native memory.
221221
pub comet_task_memory_manager: CometTaskMemoryManager<'a>,
222222
/// The CometUdfBridge class used to dispatch JVM scalar UDFs.
223-
pub comet_udf_bridge: CometUdfBridge<'a>,
223+
/// `None` if the class is not on the classpath; the JVM-UDF dispatch path
224+
/// reports a clear error rather than crashing executor init.
225+
pub comet_udf_bridge: Option<CometUdfBridge<'a>>,
224226
}
225227

226228
unsafe impl Send for JVMClasses<'_> {}
@@ -291,7 +293,16 @@ impl JVMClasses<'_> {
291293
comet_batch_iterator: CometBatchIterator::new(env).unwrap(),
292294
comet_shuffle_block_iterator: CometShuffleBlockIterator::new(env).unwrap(),
293295
comet_task_memory_manager: CometTaskMemoryManager::new(env).unwrap(),
294-
comet_udf_bridge: CometUdfBridge::new(env).unwrap(),
296+
comet_udf_bridge: {
297+
// Optional: if the bridge class is absent (e.g. comet shading
298+
// dropped org.apache.comet.udf.*), record None and clear the
299+
// pending JVM exception so other JNI calls keep working.
300+
let bridge = CometUdfBridge::new(env).ok();
301+
if env.exception_check() {
302+
env.exception_clear();
303+
}
304+
bridge
305+
},
295306
}
296307
});
297308
}

native/spark-expr/src/jvm_udf/mod.rs

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ use datafusion::common::Result as DFResult;
2929
use datafusion::logical_expr::ColumnarValue;
3030
use datafusion::physical_expr::PhysicalExpr;
3131

32-
use datafusion_comet_jni_bridge::errors::CometError;
32+
use datafusion_comet_jni_bridge::errors::{CometError, ExecutionError};
3333
use datafusion_comet_jni_bridge::JVMClasses;
3434
use jni::objects::{JObject, JValue};
3535

@@ -148,7 +148,14 @@ impl PhysicalExpr for JvmScalarUdfExpr {
148148

149149
// Step 3: attach a JNI env for this thread and call the static bridge method.
150150
JVMClasses::with_env(|env| {
151-
let bridge = &JVMClasses::get().comet_udf_bridge;
151+
let bridge = JVMClasses::get().comet_udf_bridge.as_ref().ok_or_else(|| {
152+
CometError::from(ExecutionError::GeneralError(
153+
"JVM UDF bridge unavailable: org.apache.comet.udf.CometUdfBridge \
154+
class was not found on the JVM classpath. Set \
155+
spark.comet.exec.regexp.engine=rust to disable this path."
156+
.to_string(),
157+
))
158+
})?;
152159

153160
// Build the JVM String for the class name.
154161
let jclass_name = env
@@ -195,8 +202,11 @@ impl PhysicalExpr for JvmScalarUdfExpr {
195202
})?;
196203

197204
// Step 4: import the result from the FFI slots filled by the JVM.
198-
// SAFETY: from_ffi consumes the FFI_ArrowArray's release ownership; arrow-rs sets the
199-
// release callback to None on the moved-from struct, so the Box's subsequent drop is a no-op.
205+
// SAFETY: `*out_array` moves the FFI_ArrowArray out of the Box (the heap
206+
// allocation is freed by the move), and `from_ffi` wraps it in an Arc that
207+
// keeps the JVM-installed release callback alive until the resulting
208+
// ArrayData drops. `out_schema` is borrowed; its release callback runs
209+
// exactly once when the Box drops at end of scope.
200210
let result_data = unsafe { from_ffi(*out_array, &out_schema) }
201211
.map_err(|e| CometError::Arrow { source: e })?;
202212
Ok(ColumnarValue::Array(make_array(result_data)))

0 commit comments

Comments
 (0)