Github user JeetKunDoug commented on a diff in the pull request: https://github.com/apache/spark/pull/21322#discussion_r188325608 --- Diff: core/src/main/scala/org/apache/spark/storage/memory/MemoryStore.scala --- @@ -384,15 +385,36 @@ private[spark] class MemoryStore( } } + private def maybeReleaseResources(entry: MemoryEntry[_]): Unit = { + entry match { + case SerializedMemoryEntry(buffer, _, _) => buffer.dispose() + case DeserializedMemoryEntry(objs: Array[Any], _, _) => maybeCloseValues(objs) --- End diff -- Actually, digging further, there's other places where we may deserialize an object from the disk store and never put it into the memory store - it seems like punting on a guarantee that your AutoClosable object is closed and making this a best-effort thing when calling `BlockManager.removeBroadcast` (which is how we used it in the case that caused us to put together this PR) may make the most sense. It'll still be better than depending on GC and a finalizer to get the resources cleaned up when the driver can call `Broadcast#destroy` but we can document it as a best practice to have one just in case the close() call doesn't happen due to edge cases.
--- --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org