Skip to content

Commit 5b5ff6d

Browse files
xihuan_mstrscottme
authored andcommitted
[SPARK-54753][SQL] fix memory leak of ArtifactManager
### What changes were proposed in this pull request? As stated in https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/lang/ref/Cleaner.html **The cleaning action could be a lambda but all too easily will capture the object reference, by referring to fields of the object being cleaned, preventing the object from becoming phantom reachable. Using a static nested class, as above, will avoid accidentally retaining the object reference.** For more details, and the test and analysis are in https://issues.apache.org/jira/browse/SPARK-54753 <img width="1462" height="559" alt="image" src="https://github.com/user-attachments/assets/83de9e8e-8f63-41fe-8318-b1cea6a1de9c" /> After running with Spark 4.0.1, the ArtififactManager is leaked, its referenced SessionState/SparkSession is as well leaked. ### Why are the changes needed? use a separate class to ref the cleanup state ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? with test program in https://issues.apache.org/jira/browse/SPARK-54753, and use Visual VM to monitor the memory usage ### Was this patch authored or co-authored using generative AI tooling? No cc dongjoon-hyun pranavdev022 hvanhovell vicennial HyukjinKwon Closes #53591 from scottme/master. Lead-authored-by: xihuan_mstr <[email protected]> Co-authored-by: scottme <[email protected]> Signed-off-by: Hyukjin Kwon <[email protected]>
1 parent 2ab68d1 commit 5b5ff6d

File tree

2 files changed

+9
-2
lines changed

2 files changed

+9
-2
lines changed

sql/connect/client/jvm/src/test/scala/org/apache/spark/sql/connect/client/CheckConnectJvmClientCompatibility.scala

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,8 @@ object CheckConnectJvmClientCompatibility {
234234
"org.apache.spark.sql.artifact.ArtifactManager$"),
235235
ProblemFilters.exclude[MissingClassProblem](
236236
"org.apache.spark.sql.artifact.ArtifactManager$SparkContextResourceType$"),
237+
ProblemFilters.exclude[MissingClassProblem](
238+
"org.apache.spark.sql.artifact.ArtifactManager$StateCleanupRunner"),
237239
ProblemFilters.exclude[MissingClassProblem](
238240
"org.apache.spark.sql.artifact.RefCountedCacheId"),
239241

sql/core/src/main/scala/org/apache/spark/sql/artifact/ArtifactManager.scala

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -422,8 +422,7 @@ class ArtifactManager(session: SparkSession) extends AutoCloseable with Logging
422422
artifactPath)
423423
// Ensure that no reference to `this` is captured/help by the cleanup lambda
424424
private def getCleanable: Cleaner.Cleanable = cleaner.register(
425-
this,
426-
() => ArtifactManager.cleanUpGlobalResources(cleanUpStateForGlobalResources)
425+
this, new StateCleanupRunner(cleanUpStateForGlobalResources)
427426
)
428427
private var cleanable = getCleanable
429428

@@ -529,6 +528,12 @@ object ArtifactManager extends Logging {
529528
val JAR, FILE, ARCHIVE = Value
530529
}
531530

531+
private class StateCleanupRunner(cleanupState: ArtifactStateForCleanup) extends Runnable {
532+
override def run(): Unit = {
533+
ArtifactManager.cleanUpGlobalResources(cleanupState)
534+
}
535+
}
536+
532537
// Shared cleaner instance
533538
private val cleaner: Cleaner = Cleaner.create()
534539

0 commit comments

Comments
 (0)