fix: propagate the Spark task ClassLoader to JVM UDF calls - #5282
Conversation
A UDF whose closure was captured by a class from a user jar (--jars /
spark.jars) failed at execution time with:
java.lang.ClassCastException: cannot assign instance of
java.lang.invoke.SerializedLambda to field ScalaUDF.f of type
scala.Function1
at CometScalaUDFCodegen.lookupOrCompile
That is a masked ClassNotFoundException. When the deserializing loader
cannot resolve a lambda's capturing class, ObjectInputStream records the
CNFE against the handle, skips SerializedLambda.readResolve, and the raw
SerializedLambda then fails the field-type check in
defaultCheckFieldValues.
The loader was wrong because CometUdfBridge.evaluate is invoked from a
Tokio worker, which attaches to the JVM through JNI and therefore has no
context ClassLoader. lookupOrCompile fell back to
classOf[Expression].getClassLoader, which never holds user jars. Spark
installs the executor's user ClassLoader on task threads only, so plans
whose stage leaf is a native scan (CometNativeScan,
CometIcebergNativeScan) hit this on every UDF call.
Capture the task thread's context ClassLoader in CometExecIterator and
thread it to the bridge the same way TaskContext already is, then install
it on the calling thread for the duration of the call. This also covers
Class.forName for user-supplied CometUDF implementations, which had the
same latent bug, and any user code inside a UDF that reads the context
ClassLoader.
No behavior change. - Drop the `taskClassLoader` field and its fallback in CometExecIterator. The fallback was unreachable (Executor installs a non-null context ClassLoader before the task body runs, and the class already requires a live TaskContext), and it fell back to the very ClassLoader that caused the bug. Pass the loader inline; the null-guards in the bridge and on the Rust side already serve the no-TaskContext driver path. - Keep the rationale in one place, on CometUdfBridge.evaluate, and cross-reference it from the other five hops instead of restating it. Record why the loader is installed per call rather than once per attached worker: the Tokio runtime is process-global, so one worker interleaves work from different task attempts and, under Spark Connect, from sessions with different artifact ClassLoaders. - Collapse the paired task_context / class_loader debug_asserts into one, matching the comment that already treats them as a single invariant. - Restore the context ClassLoader unconditionally: a no-op when nothing was installed, and it also undoes any change the user function made to a worker that outlives the call. - Resolve CometUDF classes through the existing ClassLoaders.loadClass helper rather than hand-rolling the same lookup. - Test suite: use a classes directory instead of building a jar (a directory is a valid classpath entry), an intersection cast instead of a nested interface, and pass javac only scala-library instead of the whole test classpath. Merge the two identical query tests into one parameterized over the native-scan leaf, load the function once, and register the temp tree for deletion on exit.
mbutrovich
left a comment
There was a problem hiding this comment.
Thanks @andygrove! Test suggestion but I don't consider it blocking:
The PR description credits this fix with also closing a second latent bug: the Class.forName resolution in CometUdfBridge.java for a user-supplied CometUDF implementation shipped in a user jar. The new suite only exercises the ScalaUDF / CometScalaUDFCodegen path (spark.udf.register plus the codegen dispatcher). Is a CometUDF ever loaded from a class name outside Comet's own classpath today? CometScalaUDFCodegen looks like the only class in the codebase ever passed as JvmScalarUdf.class_name (CometScalaUDF.scala:138 is the only setClassName call site), so as far as I can tell nothing currently routes a user-jar class name through that second path. If that's right, would it be worth a comment noting the second fix is defensive for a currently-unreachable path, or is there a way to reach it that a test should pin down now?
comphead
left a comment
There was a problem hiding this comment.
Thanks @andygrove I'm checking this, the PR LGTM overall I just wondering if this class loader tricks can somehow impact if user specify userClassPathFirst
comphead
left a comment
There was a problem hiding this comment.
Thanks @andygrove lets give it a try
Which issue does this PR close?
Closes #5281.
Rationale for this change
A
ScalaUDF(or Java UDF) whose closure was captured by a class from a user jar (--jars/spark.jars) fails at execution time:That
ClassCastExceptionis a maskedClassNotFoundException. When the deserializing classloader cannot resolve a lambda's capturing class,ObjectInputStreamrecords the CNFE against the object handle, therefore skipsSerializedLambda.readResolve(), and the rawSerializedLambdathen fails the field-type check indefaultCheckFieldValues. The CNFE never surfaces.The classloader is wrong because of which thread runs the deserialization.
CometUdfBridge.evaluateis invoked from a Tokio worker, which attaches to the JVM through JNI, and an attached thread has no context classloader.lookupOrCompiletherefore fell back toclassOf[Expression].getClassLoader— the loader that loaded spark-catalyst and Comet, which never contains user jars. Spark installs the executor'sMutableURLClassLoaderon task threads only.Measured by returning thread identity from inside a UDF body:
CometNativeScanThread-28..32(Tokio workers)Executor task launch worker …MutableURLClassLoaderExecutor task launch worker …MutableURLClassLoaderSo any plan whose stage leaf is a native scan (
CometNativeScan,CometIcebergNativeScan) hits this on every UDF call.CometUdfBridgealready propagatesTaskContextacross the JNI boundary for exactly this reason; the classloader was not propagated.This is invisible to Comet's existing test suites because they register UDFs from test classes already on the application classpath.
What changes are included in this PR?
The task thread's context classloader is now propagated to the Tokio worker, following the same path as
TaskContext:CometExecIteratorcapturesThread.currentThread().getContextClassLoader(it is constructed on the Spark task thread) and passes it tocreatePlan.Native.createPlangains aclassLoaderparameter;jni_api.rsholds it as a JNI global ref onExecutionContextand hands it to the planner viawith_class_loader.PhysicalPlannerclones it into everyJvmScalarUdfExpr, under the samedebug_assertinvariant already used fortask_context.JvmScalarUdfExprpasses it through the bridge call; the JNI signature gainsLjava/lang/ClassLoader;.CometUdfBridge.evaluateinstalls it as the calling thread's context classloader for the duration of the call, restoring the prior value infinally— the same save-and-restore shape used forTaskContext.Installing it on the thread rather than passing it down to the deserializer fixes both affected lookups at once: the closure deserialization in
CometScalaUDFCodegen, and theClass.forName(udfClassName)inCometUdfBridge, which had the same latent bug for a user-suppliedCometUDFimplementation shipped in a user jar. It also means user code inside a UDF that relies on the context classloader behaves as it does under Spark's own execution.One line was added to the "Behavior" section of
docs/source/user-guide/latest/scala_java_udfs.md, alongside the existingTaskContext.get()guarantee.How are these changes tested?
New suite
CometScalaUDFClassLoaderSuite, which compiles a class holding a serializablescala.Function1lambda into a jar at test time and wires it in viaspark.executor.extraClassPath— the local-mode equivalent of a--jarssubmission, sinceLocalSchedulerBackendfeeds it into the executor'sMutableURLClassLoader. Four tests:ClassNotFoundExceptionfromclassOf[Expression].getClassLoader(the old fallback), so the fixture reproduces the production classloader topology.SELECT hiddenUdf(s) FROM twith aCometNativeScanleaf.The two new behavioral tests were confirmed to fail without the fix (reverting only the loader install), reporting
MISSING|Thread-53, MISSING|Thread-28, …and naming the Tokio workers.Existing coverage run locally on Spark 4.1 / Scala 2.13:
CometCodegenSuite,CometCodegenHOFSuite,CometCodegenSourceSuite,CometCodegenFuzzSuiteplus the new suite (180 tests), andCometExecSuite(143 tests), which exercises the changedcreatePlansignature broadly.cargo fmt --checkandcargo clippy --all-targets -D warningsare clean.