Skip to content

Commit 11e5c37

Browse files
JoshRosenrxin
authored andcommitted
[SPARK-8962] Add Scalastyle rule to ban direct use of Class.forName; fix existing uses
This pull request adds a Scalastyle regex rule which fails the style check if `Class.forName` is used directly. `Class.forName` always loads classes from the default / system classloader, but in a majority of cases, we should be using Spark's own `Utils.classForName` instead, which tries to load classes from the current thread's context classloader and falls back to the classloader which loaded Spark when the context classloader is not defined. <!-- Reviewable:start --> [<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/apache/spark/7350) <!-- Reviewable:end --> Author: Josh Rosen <[email protected]> Closes apache#7350 from JoshRosen/ban-Class.forName and squashes the following commits: e3e96f7 [Josh Rosen] Merge remote-tracking branch 'origin/master' into ban-Class.forName c0b7885 [Josh Rosen] Hopefully fix the last two cases d707ba7 [Josh Rosen] Fix uses of Class.forName that I missed in my first cleanup pass 046470d [Josh Rosen] Merge remote-tracking branch 'origin/master' into ban-Class.forName 62882ee [Josh Rosen] Fix uses of Class.forName or add exclusion. d9abade [Josh Rosen] Add stylechecker rule to ban uses of Class.forName
1 parent 740b034 commit 11e5c37

File tree

49 files changed

+117
-84
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

49 files changed

+117
-84
lines changed

core/src/main/scala/org/apache/spark/Logging.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ private object Logging {
159159
try {
160160
// We use reflection here to handle the case where users remove the
161161
// slf4j-to-jul bridge order to route their logs to JUL.
162-
val bridgeClass = Class.forName("org.slf4j.bridge.SLF4JBridgeHandler")
162+
val bridgeClass = Utils.classForName("org.slf4j.bridge.SLF4JBridgeHandler")
163163
bridgeClass.getMethod("removeHandlersForRootLogger").invoke(null)
164164
val installed = bridgeClass.getMethod("isInstalled").invoke(null).asInstanceOf[Boolean]
165165
if (!installed) {

core/src/main/scala/org/apache/spark/SparkContext.scala

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1968,7 +1968,7 @@ class SparkContext(config: SparkConf) extends Logging with ExecutorAllocationCli
19681968
for (className <- listenerClassNames) {
19691969
// Use reflection to find the right constructor
19701970
val constructors = {
1971-
val listenerClass = Class.forName(className)
1971+
val listenerClass = Utils.classForName(className)
19721972
listenerClass.getConstructors.asInstanceOf[Array[Constructor[_ <: SparkListener]]]
19731973
}
19741974
val constructorTakingSparkConf = constructors.find { c =>
@@ -2503,7 +2503,7 @@ object SparkContext extends Logging {
25032503
"\"yarn-standalone\" is deprecated as of Spark 1.0. Use \"yarn-cluster\" instead.")
25042504
}
25052505
val scheduler = try {
2506-
val clazz = Class.forName("org.apache.spark.scheduler.cluster.YarnClusterScheduler")
2506+
val clazz = Utils.classForName("org.apache.spark.scheduler.cluster.YarnClusterScheduler")
25072507
val cons = clazz.getConstructor(classOf[SparkContext])
25082508
cons.newInstance(sc).asInstanceOf[TaskSchedulerImpl]
25092509
} catch {
@@ -2515,7 +2515,7 @@ object SparkContext extends Logging {
25152515
}
25162516
val backend = try {
25172517
val clazz =
2518-
Class.forName("org.apache.spark.scheduler.cluster.YarnClusterSchedulerBackend")
2518+
Utils.classForName("org.apache.spark.scheduler.cluster.YarnClusterSchedulerBackend")
25192519
val cons = clazz.getConstructor(classOf[TaskSchedulerImpl], classOf[SparkContext])
25202520
cons.newInstance(scheduler, sc).asInstanceOf[CoarseGrainedSchedulerBackend]
25212521
} catch {
@@ -2528,8 +2528,7 @@ object SparkContext extends Logging {
25282528

25292529
case "yarn-client" =>
25302530
val scheduler = try {
2531-
val clazz =
2532-
Class.forName("org.apache.spark.scheduler.cluster.YarnScheduler")
2531+
val clazz = Utils.classForName("org.apache.spark.scheduler.cluster.YarnScheduler")
25332532
val cons = clazz.getConstructor(classOf[SparkContext])
25342533
cons.newInstance(sc).asInstanceOf[TaskSchedulerImpl]
25352534

@@ -2541,7 +2540,7 @@ object SparkContext extends Logging {
25412540

25422541
val backend = try {
25432542
val clazz =
2544-
Class.forName("org.apache.spark.scheduler.cluster.YarnClientSchedulerBackend")
2543+
Utils.classForName("org.apache.spark.scheduler.cluster.YarnClientSchedulerBackend")
25452544
val cons = clazz.getConstructor(classOf[TaskSchedulerImpl], classOf[SparkContext])
25462545
cons.newInstance(scheduler, sc).asInstanceOf[CoarseGrainedSchedulerBackend]
25472546
} catch {

core/src/main/scala/org/apache/spark/SparkEnv.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ object SparkEnv extends Logging {
261261

262262
// Create an instance of the class with the given name, possibly initializing it with our conf
263263
def instantiateClass[T](className: String): T = {
264-
val cls = Class.forName(className, true, Utils.getContextOrSparkClassLoader)
264+
val cls = Utils.classForName(className)
265265
// Look for a constructor taking a SparkConf and a boolean isDriver, then one taking just
266266
// SparkConf, then one taking no arguments
267267
try {

core/src/main/scala/org/apache/spark/api/r/RBackendHandler.scala

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import io.netty.channel.{ChannelHandlerContext, SimpleChannelInboundHandler}
2626

2727
import org.apache.spark.Logging
2828
import org.apache.spark.api.r.SerDe._
29+
import org.apache.spark.util.Utils
2930

3031
/**
3132
* Handler for RBackend
@@ -88,21 +89,6 @@ private[r] class RBackendHandler(server: RBackend)
8889
ctx.close()
8990
}
9091

91-
// Looks up a class given a class name. This function first checks the
92-
// current class loader and if a class is not found, it looks up the class
93-
// in the context class loader. Address [SPARK-5185]
94-
def getStaticClass(objId: String): Class[_] = {
95-
try {
96-
val clsCurrent = Class.forName(objId)
97-
clsCurrent
98-
} catch {
99-
// Use contextLoader if we can't find the JAR in the system class loader
100-
case e: ClassNotFoundException =>
101-
val clsContext = Class.forName(objId, true, Thread.currentThread().getContextClassLoader)
102-
clsContext
103-
}
104-
}
105-
10692
def handleMethodCall(
10793
isStatic: Boolean,
10894
objId: String,
@@ -113,7 +99,7 @@ private[r] class RBackendHandler(server: RBackend)
11399
var obj: Object = null
114100
try {
115101
val cls = if (isStatic) {
116-
getStaticClass(objId)
102+
Utils.classForName(objId)
117103
} else {
118104
JVMObjectTracker.get(objId) match {
119105
case None => throw new IllegalArgumentException("Object not found " + objId)

core/src/main/scala/org/apache/spark/broadcast/BroadcastManager.scala

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import java.util.concurrent.atomic.AtomicLong
2222
import scala.reflect.ClassTag
2323

2424
import org.apache.spark._
25+
import org.apache.spark.util.Utils
2526

2627
private[spark] class BroadcastManager(
2728
val isDriver: Boolean,
@@ -42,7 +43,7 @@ private[spark] class BroadcastManager(
4243
conf.get("spark.broadcast.factory", "org.apache.spark.broadcast.TorrentBroadcastFactory")
4344

4445
broadcastFactory =
45-
Class.forName(broadcastFactoryClass).newInstance.asInstanceOf[BroadcastFactory]
46+
Utils.classForName(broadcastFactoryClass).newInstance.asInstanceOf[BroadcastFactory]
4647

4748
// Initialize appropriate BroadcastFactory and BroadcastObject
4849
broadcastFactory.initialize(isDriver, conf, securityManager)

core/src/main/scala/org/apache/spark/deploy/SparkHadoopUtil.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ class SparkHadoopUtil extends Logging {
178178

179179
private def getFileSystemThreadStatisticsMethod(methodName: String): Method = {
180180
val statisticsDataClass =
181-
Class.forName("org.apache.hadoop.fs.FileSystem$Statistics$StatisticsData")
181+
Utils.classForName("org.apache.hadoop.fs.FileSystem$Statistics$StatisticsData")
182182
statisticsDataClass.getDeclaredMethod(methodName)
183183
}
184184

@@ -356,7 +356,7 @@ object SparkHadoopUtil {
356356
System.getProperty("SPARK_YARN_MODE", System.getenv("SPARK_YARN_MODE")))
357357
if (yarnMode) {
358358
try {
359-
Class.forName("org.apache.spark.deploy.yarn.YarnSparkHadoopUtil")
359+
Utils.classForName("org.apache.spark.deploy.yarn.YarnSparkHadoopUtil")
360360
.newInstance()
361361
.asInstanceOf[SparkHadoopUtil]
362362
} catch {

core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -624,7 +624,7 @@ object SparkSubmit {
624624
var mainClass: Class[_] = null
625625

626626
try {
627-
mainClass = Class.forName(childMainClass, true, loader)
627+
mainClass = Utils.classForName(childMainClass)
628628
} catch {
629629
case e: ClassNotFoundException =>
630630
e.printStackTrace(printStream)

core/src/main/scala/org/apache/spark/deploy/SparkSubmitArguments.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -576,7 +576,7 @@ private[deploy] class SparkSubmitArguments(args: Seq[String], env: Map[String, S
576576
System.setSecurityManager(sm)
577577

578578
try {
579-
Class.forName(mainClass).getMethod("main", classOf[Array[String]])
579+
Utils.classForName(mainClass).getMethod("main", classOf[Array[String]])
580580
.invoke(null, Array(HELP))
581581
} catch {
582582
case e: InvocationTargetException =>

core/src/main/scala/org/apache/spark/deploy/history/HistoryServer.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ object HistoryServer extends Logging {
228228

229229
val providerName = conf.getOption("spark.history.provider")
230230
.getOrElse(classOf[FsHistoryProvider].getName())
231-
val provider = Class.forName(providerName)
231+
val provider = Utils.classForName(providerName)
232232
.getConstructor(classOf[SparkConf])
233233
.newInstance(conf)
234234
.asInstanceOf[ApplicationHistoryProvider]

core/src/main/scala/org/apache/spark/deploy/master/Master.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ private[master] class Master(
172172
new FileSystemRecoveryModeFactory(conf, SerializationExtension(actorSystem))
173173
(fsFactory.createPersistenceEngine(), fsFactory.createLeaderElectionAgent(this))
174174
case "CUSTOM" =>
175-
val clazz = Class.forName(conf.get("spark.deploy.recoveryMode.factory"))
175+
val clazz = Utils.classForName(conf.get("spark.deploy.recoveryMode.factory"))
176176
val factory = clazz.getConstructor(classOf[SparkConf], classOf[Serialization])
177177
.newInstance(conf, SerializationExtension(actorSystem))
178178
.asInstanceOf[StandaloneRecoveryModeFactory]

0 commit comments

Comments
 (0)