Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/2670#discussion_r18474437
--- Diff: core/src/main/scala/org/apache/spark/util/Utils.scala ---
@@ -666,15 +673,27 @@ private[spark] object Utils extends Logging {
*/
def deleteRecursively(file: File) {
if (file != null) {
- if (file.isDirectory() && !isSymlink(file)) {
- for (child <- listFilesSafely(file)) {
- deleteRecursively(child)
+ try {
+ if (file.isDirectory && !isSymlink(file)) {
+ var savedIOException: IOException = null
+ for (child <- listFilesSafely(file)) {
+ try {
+ deleteRecursively(child)
+ } catch {
+ // In case of multiple exceptions, only last one will be
thrown
+ case ioe: IOException => savedIOException = ioe
+ }
+ }
+ if (savedIOException != null) {
+ throw savedIOException
+ }
}
- }
- if (!file.delete()) {
- // Delete can also fail if the file simply did not exist
- if (file.exists()) {
- throw new IOException("Failed to delete: " +
file.getAbsolutePath)
+ } finally {
--- End diff --
The problem I was seeing is that an empty directory was listed and returned
`null` for some reason, which generates an `IOException` and then leaves the
parent hanging around even though it's empty. I'm not sure why it was happening
but it was reliable when running `UtilsSuite` in my IDE. Hence I wanted to make
this a little more defensive ... it seems as fine to fail for not listing the
dir or not deleting dir?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
