[SPARK-36627][CORE] Fix java deserialization of proxy classes #33879

fsamuel-bs · 2021-08-31T17:12:48Z

Upstream SPARK-XXXXX ticket and PR link (if not applicable, explain)

https://issues.apache.org/jira/browse/SPARK-36627

What changes were proposed in this pull request?

In JavaSerializer.JavaDeserializationStream we override resolveClass of ObjectInputStream to use the threads' contextClassLoader. However, we do not override resolveProxyClass, which is used when deserializing Java proxy objects, which makes spark use the wrong classloader when deserializing objects, which causes the job to fail with the following exception:

Caused by: org.apache.spark.SparkException: Job aborted due to stage failure: Task 0 in stage 1.0 failed 4 times, most recent failure: Lost task 0.3 in stage 1.0 (TID 4, <host>, executor 1): java.lang.ClassNotFoundException: <class&gt;
	at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:581)
	at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:178)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:522)
	at java.base/java.lang.Class.forName0(Native Method)
	at java.base/java.lang.Class.forName(Class.java:398)
	at java.base/java.io.ObjectInputStream.resolveProxyClass(ObjectInputStream.java:829)
	at java.base/java.io.ObjectInputStream.readProxyDesc(ObjectInputStream.java:1917)
	...
	at org.apache.spark.serializer.JavaDeserializationStream.readObject(JavaSerializer.scala:76)

Why are the changes needed?

Spark deserialization fails with no recourse for the user.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Unit tests.

fsamuel-bs · 2021-09-21T15:44:04Z

@EugenCepoi @squito could any of you take a look at this PR? Pinging you both because you seem to have worked together on https://issues.apache.org/jira/browse/SPARK-8730, #7122, which is similar to this one.

fsamuel-bs · 2021-08-31T17:13:59Z

core/src/main/scala/org/apache/spark/serializer/JavaSerializer.scala

+
+    override def resolveProxyClass(ifaces: Array[String]): Class[_] = {
+        // scalastyle:off classforname
+        val resolved = ifaces.map(iface => Class.forName(iface, false, loader))


We definitely want better exceptions if this fails, but not sure what exactly we'd want here.

2-space indent, and maybe you can use the Utils method in Spark for loading the class?

Utils#classForName uses either getContextOrSparkClassLoader or thread.currentThread().getContextClassLoader which might not be the ones we want depending on the context we're deserializing -- I'd rather use the same classloader we're using above (when deserializing non-proxy java objects).

fsamuel-bs · 2021-08-31T17:21:11Z

core/src/test/scala/org/apache/spark/serializer/JavaSerializerSuite.scala

+      instance.deserialize[ContainsProxyClass](instance.serialize(new ContainsProxyClass()))
+    // enforce class cast
+    obj.getClass
+    assert(classesLoaded.exists(klass => klass.contains("MyInterface")))


This is a very roundabout way of testing this, but I couldn't think of another way of testing it - the way we hit this issue was running spark in YARN, and I don't think we can trivially reproduce it with spark local.

In order to repro exactly what happened in YARN we'd need to make sun.misc.VM.latestUserDefinedLoader() not be able to resolve MyInterface, but I couldn't figure out how to do it.

srowen · 2021-09-26T00:50:44Z

Out of curiosity, where do proxy classes typically come up?

fsamuel-bs · 2021-10-22T09:42:06Z

Sorry for the late response - proxy classes come up when we're building http clients. We usually build them from interfaces (annotated with JaxRs annotations or custom ones) and reference it by its interface. See feign for an example.

srowen

This probably can't 'hurt', right? only should make something possibly work that couldn't before

srowen · 2021-10-22T13:40:20Z

core/src/main/scala/org/apache/spark/serializer/JavaSerializer.scala

+
+    override def resolveProxyClass(ifaces: Array[String]): Class[_] = {
+        // scalastyle:off classforname
+        val resolved = ifaces.map(iface => Class.forName(iface, false, loader))


2-space indent, and maybe you can use the Utils method in Spark for loading the class?

core/src/test/java/org/apache/spark/serializer/ProxySerializerTest.java

srowen · 2021-10-22T13:40:33Z

core/src/test/java/org/apache/spark/serializer/ProxySerializerTest.java

+import java.lang.reflect.Proxy;
+
+class ContainsProxyClass implements Serializable {
+    final MyInterface proxy = (MyInterface) Proxy.newProxyInstance(


Likewise 2-space

fsamuel-bs · 2021-10-27T14:00:58Z

core/src/main/scala/org/apache/spark/serializer/JavaSerializer.scala

-    out.writeInt(counterReset)
-    out.writeBoolean(extraDebugInfo)
-  }
+  override def writeExternal(out: ObjectOutput): Unit =


Ran ./dev/scalafmt, which modified a few more lines.

srowen

Looks OK, let me run tests. If you have a sec, go ahead and copy info from the JIRA into the description above

srowen · 2021-10-27T14:07:45Z

core/src/main/scala/org/apache/spark/serializer/JavaSerializer.scala

@@ -28,8 +28,10 @@ import org.apache.spark.internal.config._
 import org.apache.spark.util.{ByteBufferInputStream, ByteBufferOutputStream, Utils}

 private[spark] class JavaSerializationStream(
-    out: OutputStream, counterReset: Int, extraDebugInfo: Boolean)
-  extends SerializationStream {
+    out: OutputStream,


I think this is fine, if this is what scalafmt does to this file - otherwise I'd say leave it

yep, all was scalafmt

srowen · 2021-10-27T14:09:31Z

Jenkins test this please

SparkQA · 2021-10-27T14:24:16Z

Test build #144660 has finished for PR 33879 at commit 76f4fa6.

This patch fails Java style tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2021-10-27T15:00:40Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49131/

SparkQA · 2021-10-27T15:44:05Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49131/

SparkQA · 2021-10-27T16:54:18Z

Test build #144662 has finished for PR 33879 at commit 76f4fa6.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

srowen · 2021-10-28T13:39:35Z

Jenkins test this please

SparkQA · 2021-10-28T15:03:37Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49188/

SparkQA · 2021-10-28T15:57:49Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49188/

SparkQA · 2021-10-28T17:03:07Z

Test build #144719 has finished for PR 33879 at commit e6b0bf6.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

srowen · 2021-10-28T23:16:04Z

Merged to master

## Upstream SPARK-XXXXX ticket and PR link (if not applicable, explain) https://issues.apache.org/jira/browse/SPARK-36627 ## What changes were proposed in this pull request? In JavaSerializer.JavaDeserializationStream we override resolveClass of ObjectInputStream to use the threads' contextClassLoader. However, we do not override resolveProxyClass, which is used when deserializing Java proxy objects, which makes spark use the wrong classloader when deserializing objects, which causes the job to fail with the following exception: ``` Caused by: org.apache.spark.SparkException: Job aborted due to stage failure: Task 0 in stage 1.0 failed 4 times, most recent failure: Lost task 0.3 in stage 1.0 (TID 4, <host>, executor 1): java.lang.ClassNotFoundException: <class> at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:581) at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:178) at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:522) at java.base/java.lang.Class.forName0(Native Method) at java.base/java.lang.Class.forName(Class.java:398) at java.base/java.io.ObjectInputStream.resolveProxyClass(ObjectInputStream.java:829) at java.base/java.io.ObjectInputStream.readProxyDesc(ObjectInputStream.java:1917) ... at org.apache.spark.serializer.JavaDeserializationStream.readObject(JavaSerializer.scala:76) ``` ### Why are the changes needed? Spark deserialization fails with no recourse for the user. ### Does this PR introduce any user-facing change? No. ### How was this patch tested? Unit tests. Closes apache#33879 from fsamuel-bs/SPARK-36627. Authored-by: Samuel Souza <ssouza@palantir.com> Signed-off-by: Sean Owen <srowen@gmail.com>

fsamuel-bs added 2 commits August 31, 2021 18:06

tests + patch

34427c4

pr number

9329067

github-actions bot added the CORE label Aug 31, 2021

docs

279c063

fsamuel-bs commented Sep 21, 2021

View reviewed changes

srowen requested changes Oct 22, 2021

View reviewed changes

./dev/scalafmt + copyright header

76f4fa6

fsamuel-bs commented Oct 27, 2021

View reviewed changes

srowen reviewed Oct 27, 2021

View reviewed changes

rename file

b34f400

line length

e6b0bf6

srowen closed this in 90fe41b Oct 28, 2021

fsamuel-bs mentioned this pull request Nov 2, 2021

[SPARK-36627][CORE] Fix java deserialization of proxy classes palantir/spark#769

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-36627][CORE] Fix java deserialization of proxy classes #33879

[SPARK-36627][CORE] Fix java deserialization of proxy classes #33879

fsamuel-bs commented Aug 31, 2021 •

edited

Loading

fsamuel-bs commented Sep 21, 2021

fsamuel-bs Aug 31, 2021

srowen Oct 22, 2021

fsamuel-bs Oct 27, 2021

fsamuel-bs Aug 31, 2021

srowen commented Sep 26, 2021

fsamuel-bs commented Oct 22, 2021

srowen left a comment

srowen Oct 22, 2021

srowen Oct 22, 2021

fsamuel-bs Oct 27, 2021

srowen left a comment

srowen Oct 27, 2021

fsamuel-bs Oct 27, 2021

srowen commented Oct 27, 2021

SparkQA commented Oct 27, 2021

SparkQA commented Oct 27, 2021

SparkQA commented Oct 27, 2021

SparkQA commented Oct 27, 2021

srowen commented Oct 28, 2021

SparkQA commented Oct 28, 2021

SparkQA commented Oct 28, 2021

SparkQA commented Oct 28, 2021

srowen commented Oct 28, 2021

[SPARK-36627][CORE] Fix java deserialization of proxy classes #33879

[SPARK-36627][CORE] Fix java deserialization of proxy classes #33879

Conversation

fsamuel-bs commented Aug 31, 2021 • edited Loading

Upstream SPARK-XXXXX ticket and PR link (if not applicable, explain)

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

fsamuel-bs commented Sep 21, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

srowen commented Sep 26, 2021

fsamuel-bs commented Oct 22, 2021

srowen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

srowen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

srowen commented Oct 27, 2021

SparkQA commented Oct 27, 2021

SparkQA commented Oct 27, 2021

SparkQA commented Oct 27, 2021

SparkQA commented Oct 27, 2021

srowen commented Oct 28, 2021

SparkQA commented Oct 28, 2021

SparkQA commented Oct 28, 2021

SparkQA commented Oct 28, 2021

srowen commented Oct 28, 2021

fsamuel-bs commented Aug 31, 2021 •

edited

Loading