From d90d2175c8288f1619d167a8a25837ad8337085b Mon Sep 17 00:00:00 2001
From: Holden Karau <holden@pigscanfly.ca>
Date: Mon, 24 Mar 2014 15:16:12 -0700
Subject: [PATCH] Fix fall back with custom class loader and add a test for it

---
 .../java/org/apache/spark/util/ParentClassLoader.java |  2 +-
 .../spark/executor/ExecutorURLClassLoader.scala       |  2 +-
 .../spark/executor/ExecutorURLClassLoaderSuite.scala  |  9 +++++++++
 .../org/apache/spark/repl/ExecutorClassLoader.scala   |  2 +-
 .../apache/spark/repl/ExecutorClassLoaderSuite.scala  | 11 +++++++++--
 5 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/core/src/main/java/org/apache/spark/util/ParentClassLoader.java b/core/src/main/java/org/apache/spark/util/ParentClassLoader.java
index 8417f2281613f..354ea78eec5c7 100644
--- a/core/src/main/java/org/apache/spark/util/ParentClassLoader.java
+++ b/core/src/main/java/org/apache/spark/util/ParentClassLoader.java
@@ -27,7 +27,7 @@ public ParentClassLoader(ClassLoader myParent) {
 
     @Override
     public Class<?> findClass(String name) throws ClassNotFoundException {
-	return super.loadClass(name);
+	return super.findClass(name);
     }
 
     @Override
diff --git a/core/src/main/scala/org/apache/spark/executor/ExecutorURLClassLoader.scala b/core/src/main/scala/org/apache/spark/executor/ExecutorURLClassLoader.scala
index f78986747db17..920b28ec2b364 100644
--- a/core/src/main/scala/org/apache/spark/executor/ExecutorURLClassLoader.scala
+++ b/core/src/main/scala/org/apache/spark/executor/ExecutorURLClassLoader.scala
@@ -50,7 +50,7 @@ private[spark] class ChildExecutorURLClassLoader(urls: Array[URL], parent: Class
       userClassLoader.findClass(name)
     } catch {
       case e: ClassNotFoundException => {
-        parentClassLoader.findClass(name)
+        parentClassLoader.loadClass(name)
       }
     }
   }
diff --git a/core/src/test/scala/org/apache/spark/executor/ExecutorURLClassLoaderSuite.scala b/core/src/test/scala/org/apache/spark/executor/ExecutorURLClassLoaderSuite.scala
index a931ad9c926da..c030a1bbda268 100644
--- a/core/src/test/scala/org/apache/spark/executor/ExecutorURLClassLoaderSuite.scala
+++ b/core/src/test/scala/org/apache/spark/executor/ExecutorURLClassLoaderSuite.scala
@@ -52,4 +52,13 @@ class ExecutorURLClassLoaderSuite extends FunSuite {
     assert(fakeClassVersion === "2")
   }
 
+  test("child first can fail") {
+    val parentLoader = new URLClassLoader(urls2, null)
+    val classLoader = new ChildExecutorURLClassLoader(urls, parentLoader)
+    intercept[java.lang.ClassNotFoundException] {
+      classLoader.loadClass("org.apache.spark.test.FakeClassDoesNotExist").newInstance()
+    }
+  }
+
+
 }
diff --git a/repl/src/main/scala/org/apache/spark/repl/ExecutorClassLoader.scala b/repl/src/main/scala/org/apache/spark/repl/ExecutorClassLoader.scala
index df58742da05b0..88340ff808948 100644
--- a/repl/src/main/scala/org/apache/spark/repl/ExecutorClassLoader.scala
+++ b/repl/src/main/scala/org/apache/spark/repl/ExecutorClassLoader.scala
@@ -59,7 +59,7 @@ extends ClassLoader {
 
   override def findClass(name: String): Class[_] = {
     userClassPathFirst match {
-      case true => findClassLocally(name).getOrElse(parentLoader.findClass(name))
+      case true => findClassLocally(name).getOrElse(parentLoader.loadClass(name))
       case false => {
         try {
           parentLoader.loadClass(name)
diff --git a/repl/src/test/scala/org/apache/spark/repl/ExecutorClassLoaderSuite.scala b/repl/src/test/scala/org/apache/spark/repl/ExecutorClassLoaderSuite.scala
index e60c20b7522ea..4874b7931829b 100644
--- a/repl/src/test/scala/org/apache/spark/repl/ExecutorClassLoaderSuite.scala
+++ b/repl/src/test/scala/org/apache/spark/repl/ExecutorClassLoaderSuite.scala
@@ -27,8 +27,8 @@ class ExecutorClassLoaderSuite extends FunSuite {
   val spark_home = sys.env.get("SPARK_HOME").orElse(sys.props.get("spark.home")).get
   val url1 = "file://" + spark_home + "/core/src/test/resources/classes1/"
   val urls2 = List(new File(spark_home + "/core/src/test/resources/classes2/").toURI.toURL).toArray
+
   test("child first") {
-    println("url1 "+url1)
     val parentLoader = new URLClassLoader(urls2, null)
     val classLoader = new ExecutorClassLoader(url1, parentLoader, true)
     val fakeClass = classLoader.loadClass("org.apache.spark.test.FakeClass2").newInstance()
@@ -45,7 +45,6 @@ class ExecutorClassLoaderSuite extends FunSuite {
   }
 
   test("child first can fall back") {
-    println("url1 "+url1)
     val parentLoader = new URLClassLoader(urls2, null)
     val classLoader = new ExecutorClassLoader(url1, parentLoader, true)
     val fakeClass = classLoader.loadClass("org.apache.spark.test.FakeClass3").newInstance()
@@ -53,4 +52,12 @@ class ExecutorClassLoaderSuite extends FunSuite {
     assert(fakeClassVersion === "2")
   }
 
+  test("child first can fail") {
+    val parentLoader = new URLClassLoader(urls2, null)
+    val classLoader = new ExecutorClassLoader(url1, parentLoader, true)
+    intercept[java.lang.ClassNotFoundException] {
+      classLoader.loadClass("org.apache.spark.test.FakeClassDoesNotExist").newInstance()
+    }
+  }
+
 }