Skip to content

Commit cf3ab76

Browse files
committed
[SPARK-53136][CORE] tryWithResource & tryInitializeResource shall close resource quietly
### What changes were proposed in this pull request? This PR modifies tryWithResource & tryInitializeResource to close the resource quietly ### Why are the changes needed? Avoid unexpected errors, for example We shall see ```scala scala> tryWithResource(spark.getClass.getClassLoader.getResourceAsStream("a"))(_.readAllBytes) java.lang.NullPointerException: Cannot invoke "java.io.InputStream.readAllBytes()" because "x$1" is null at $anonfun$res1$2(<console>:1) at tryWithResource(<console>:3) ... 42 elided ``` instead of ```scala scala> tryWithResource(spark.getClass.getClassLoader.getResourceAsStream("a"))(_.readAllBytes) java.lang.NullPointerException: Cannot invoke "java.io.Closeable.close()" because "resource" is null at tryWithResource(<console>:4) ... 42 elided ``` ### Does this PR introduce _any_ user-facing change? no ### How was this patch tested? new UT ### Was this patch authored or co-authored using generative AI tooling? no Closes #51864 from yaooqinn/SPARK-53136. Authored-by: Kent Yao <[email protected]> Signed-off-by: Kent Yao <[email protected]>
1 parent 971d318 commit cf3ab76

File tree

2 files changed

+24
-2
lines changed

2 files changed

+24
-2
lines changed

common/utils/src/main/scala/org/apache/spark/util/SparkErrorUtils.scala

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,11 @@ private[spark] trait SparkErrorUtils extends Logging {
4747

4848
def tryWithResource[R <: Closeable, T](createResource: => R)(f: R => T): T = {
4949
val resource = createResource
50-
try f.apply(resource) finally resource.close()
50+
try {
51+
f.apply(resource)
52+
} finally {
53+
closeQuietly(resource)
54+
}
5155
}
5256

5357
/**
@@ -61,7 +65,7 @@ private[spark] trait SparkErrorUtils extends Logging {
6165
initialize(resource)
6266
} catch {
6367
case e: Throwable =>
64-
resource.close()
68+
closeQuietly(resource)
6569
throw e
6670
}
6771
}

common/utils/src/test/scala/org/apache/spark/util/SparkErrorUtilsSuite.scala

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717

1818
package org.apache.spark.util
1919

20+
import java.io.{Closeable, IOException}
21+
2022
import scala.annotation.nowarn
2123

2224
import org.scalatest.BeforeAndAfterEach
@@ -60,6 +62,22 @@ class SparkErrorUtilsSuite
6062
assert(SparkErrorUtils.getRootCause(cyclicCause) == cyclicCause.getCause.getCause)
6163
}
6264

65+
test("tryWithResource / tryInitializeResource") {
66+
val closeable = new Closeable {
67+
override def close(): Unit = {
68+
throw new IOException("Catch me if you can")
69+
}
70+
}
71+
val e1 = intercept[IOException] {
72+
SparkErrorUtils.tryWithResource(closeable)(_ => throw new IOException("You got me!"))
73+
}
74+
assert(e1.getMessage === "You got me!")
75+
val e2 = intercept[IOException] {
76+
SparkErrorUtils.tryInitializeResource(closeable)(_ => throw new IOException("You got me!"))
77+
}
78+
assert(e2.getMessage === "You got me!")
79+
}
80+
6381
private def createExceptionWithoutCause: Throwable =
6482
try throw new ExceptionWithoutCause
6583
catch {

0 commit comments

Comments
 (0)