spark git commit: [SPARK-21170][CORE] Utils.tryWithSafeFinallyAndFailureCallbacks throws IllegalArgumentException: Self-suppression not permitted

2017-07-01 Thread srowen 
Repository: spark
Updated Branches:
  refs/heads/branch-2.2 85fddf406 -> 6fd39ea1c


[SPARK-21170][CORE] Utils.tryWithSafeFinallyAndFailureCallbacks throws 
IllegalArgumentException: Self-suppression not permitted

## What changes were proposed in this pull request?

Not adding the exception to the suppressed if it is the same instance as 
originalThrowable.

## How was this patch tested?

Added new tests to verify this, these tests fail without source code changes 
and passes with the change.

Author: Devaraj K 

Closes #18384 from devaraj-kavali/SPARK-21170.

(cherry picked from commit 6beca9ce94f484de2f9ffb946bef8334781b3122)
Signed-off-by: Sean Owen 


Project: http://git-wip-us.apache.org/repos/asf/spark/repo
Commit: http://git-wip-us.apache.org/repos/asf/spark/commit/6fd39ea1
Tree: http://git-wip-us.apache.org/repos/asf/spark/tree/6fd39ea1
Diff: http://git-wip-us.apache.org/repos/asf/spark/diff/6fd39ea1

Branch: refs/heads/branch-2.2
Commit: 6fd39ea1c9dbf68763cb394a28d8a13c116341df
Parents: 85fddf4
Author: Devaraj K 
Authored: Sat Jul 1 15:53:49 2017 +0100
Committer: Sean Owen 
Committed: Sat Jul 1 15:54:18 2017 +0100

--
 .../scala/org/apache/spark/util/Utils.scala | 30 +++
 .../org/apache/spark/util/UtilsSuite.scala  | 88 +++-
 2 files changed, 99 insertions(+), 19 deletions(-)
--


http://git-wip-us.apache.org/repos/asf/spark/blob/6fd39ea1/core/src/main/scala/org/apache/spark/util/Utils.scala
--
diff --git a/core/src/main/scala/org/apache/spark/util/Utils.scala 
b/core/src/main/scala/org/apache/spark/util/Utils.scala
index 67497bb..999486c 100644
--- a/core/src/main/scala/org/apache/spark/util/Utils.scala
+++ b/core/src/main/scala/org/apache/spark/util/Utils.scala
@@ -1345,14 +1345,10 @@ private[spark] object Utils extends Logging {
   try {
 finallyBlock
   } catch {
-case t: Throwable =>
-  if (originalThrowable != null) {
-originalThrowable.addSuppressed(t)
-logWarning(s"Suppressing exception in finally: " + t.getMessage, t)
-throw originalThrowable
-  } else {
-throw t
-  }
+case t: Throwable if (originalThrowable != null && originalThrowable 
!= t) =>
+  originalThrowable.addSuppressed(t)
+  logWarning(s"Suppressing exception in finally: ${t.getMessage}", t)
+  throw originalThrowable
   }
 }
   }
@@ -1384,22 +1380,20 @@ private[spark] object Utils extends Logging {
   catchBlock
 } catch {
   case t: Throwable =>
-originalThrowable.addSuppressed(t)
-logWarning(s"Suppressing exception in catch: " + t.getMessage, t)
+if (originalThrowable != t) {
+  originalThrowable.addSuppressed(t)
+  logWarning(s"Suppressing exception in catch: ${t.getMessage}", t)
+}
 }
 throw originalThrowable
 } finally {
   try {
 finallyBlock
   } catch {
-case t: Throwable =>
-  if (originalThrowable != null) {
-originalThrowable.addSuppressed(t)
-logWarning(s"Suppressing exception in finally: " + t.getMessage, t)
-throw originalThrowable
-  } else {
-throw t
-  }
+case t: Throwable if (originalThrowable != null && originalThrowable 
!= t) =>
+  originalThrowable.addSuppressed(t)
+  logWarning(s"Suppressing exception in finally: ${t.getMessage}", t)
+  throw originalThrowable
   }
 }
   }

http://git-wip-us.apache.org/repos/asf/spark/blob/6fd39ea1/core/src/test/scala/org/apache/spark/util/UtilsSuite.scala
--
diff --git a/core/src/test/scala/org/apache/spark/util/UtilsSuite.scala 
b/core/src/test/scala/org/apache/spark/util/UtilsSuite.scala
index 3339d5b..d130a1d 100644
--- a/core/src/test/scala/org/apache/spark/util/UtilsSuite.scala
+++ b/core/src/test/scala/org/apache/spark/util/UtilsSuite.scala
@@ -38,7 +38,7 @@ import org.apache.commons.math3.stat.inference.ChiSquareTest
 import org.apache.hadoop.conf.Configuration
 import org.apache.hadoop.fs.Path
 
-import org.apache.spark.{SparkConf, SparkFunSuite}
+import org.apache.spark.{SparkConf, SparkFunSuite, TaskContext}
 import org.apache.spark.internal.Logging
 import org.apache.spark.network.util.ByteUnit
 
@@ -1025,4 +1025,90 @@ class UtilsSuite extends SparkFunSuite with 
ResetSystemProperties with Logging {
 assert(redactedConf("spark.sensitive.property") === 
Utils.REDACTION_REPLACEMENT_TEXT)
 
   }
+
+  test("tryWithSafeFinally") {
+var e = new Error("Block0")
+val finallyBlockError = new

spark git commit: [SPARK-21170][CORE] Utils.tryWithSafeFinallyAndFailureCallbacks throws IllegalArgumentException: Self-suppression not permitted

2017-07-01 Thread srowen 
Repository: spark
Updated Branches:
  refs/heads/master e0b047eaf -> 6beca9ce9


[SPARK-21170][CORE] Utils.tryWithSafeFinallyAndFailureCallbacks throws 
IllegalArgumentException: Self-suppression not permitted

## What changes were proposed in this pull request?

Not adding the exception to the suppressed if it is the same instance as 
originalThrowable.

## How was this patch tested?

Added new tests to verify this, these tests fail without source code changes 
and passes with the change.

Author: Devaraj K 

Closes #18384 from devaraj-kavali/SPARK-21170.


Project: http://git-wip-us.apache.org/repos/asf/spark/repo
Commit: http://git-wip-us.apache.org/repos/asf/spark/commit/6beca9ce
Tree: http://git-wip-us.apache.org/repos/asf/spark/tree/6beca9ce
Diff: http://git-wip-us.apache.org/repos/asf/spark/diff/6beca9ce

Branch: refs/heads/master
Commit: 6beca9ce94f484de2f9ffb946bef8334781b3122
Parents: e0b047e
Author: Devaraj K 
Authored: Sat Jul 1 15:53:49 2017 +0100
Committer: Sean Owen 
Committed: Sat Jul 1 15:53:49 2017 +0100

--
 .../scala/org/apache/spark/util/Utils.scala | 30 +++
 .../org/apache/spark/util/UtilsSuite.scala  | 88 +++-
 2 files changed, 99 insertions(+), 19 deletions(-)
--


http://git-wip-us.apache.org/repos/asf/spark/blob/6beca9ce/core/src/main/scala/org/apache/spark/util/Utils.scala
--
diff --git a/core/src/main/scala/org/apache/spark/util/Utils.scala 
b/core/src/main/scala/org/apache/spark/util/Utils.scala
index bbb7999..26f61e2 100644
--- a/core/src/main/scala/org/apache/spark/util/Utils.scala
+++ b/core/src/main/scala/org/apache/spark/util/Utils.scala
@@ -1348,14 +1348,10 @@ private[spark] object Utils extends Logging {
   try {
 finallyBlock
   } catch {
-case t: Throwable =>
-  if (originalThrowable != null) {
-originalThrowable.addSuppressed(t)
-logWarning(s"Suppressing exception in finally: " + t.getMessage, t)
-throw originalThrowable
-  } else {
-throw t
-  }
+case t: Throwable if (originalThrowable != null && originalThrowable 
!= t) =>
+  originalThrowable.addSuppressed(t)
+  logWarning(s"Suppressing exception in finally: ${t.getMessage}", t)
+  throw originalThrowable
   }
 }
   }
@@ -1387,22 +1383,20 @@ private[spark] object Utils extends Logging {
   catchBlock
 } catch {
   case t: Throwable =>
-originalThrowable.addSuppressed(t)
-logWarning(s"Suppressing exception in catch: " + t.getMessage, t)
+if (originalThrowable != t) {
+  originalThrowable.addSuppressed(t)
+  logWarning(s"Suppressing exception in catch: ${t.getMessage}", t)
+}
 }
 throw originalThrowable
 } finally {
   try {
 finallyBlock
   } catch {
-case t: Throwable =>
-  if (originalThrowable != null) {
-originalThrowable.addSuppressed(t)
-logWarning(s"Suppressing exception in finally: " + t.getMessage, t)
-throw originalThrowable
-  } else {
-throw t
-  }
+case t: Throwable if (originalThrowable != null && originalThrowable 
!= t) =>
+  originalThrowable.addSuppressed(t)
+  logWarning(s"Suppressing exception in finally: ${t.getMessage}", t)
+  throw originalThrowable
   }
 }
   }

http://git-wip-us.apache.org/repos/asf/spark/blob/6beca9ce/core/src/test/scala/org/apache/spark/util/UtilsSuite.scala
--
diff --git a/core/src/test/scala/org/apache/spark/util/UtilsSuite.scala 
b/core/src/test/scala/org/apache/spark/util/UtilsSuite.scala
index f7bc8f8..4ce143f 100644
--- a/core/src/test/scala/org/apache/spark/util/UtilsSuite.scala
+++ b/core/src/test/scala/org/apache/spark/util/UtilsSuite.scala
@@ -38,7 +38,7 @@ import org.apache.commons.math3.stat.inference.ChiSquareTest
 import org.apache.hadoop.conf.Configuration
 import org.apache.hadoop.fs.Path
 
-import org.apache.spark.{SparkConf, SparkFunSuite}
+import org.apache.spark.{SparkConf, SparkFunSuite, TaskContext}
 import org.apache.spark.internal.Logging
 import org.apache.spark.network.util.ByteUnit
 
@@ -1024,4 +1024,90 @@ class UtilsSuite extends SparkFunSuite with 
ResetSystemProperties with Logging {
 assert(redactedConf("spark.sensitive.property") === 
Utils.REDACTION_REPLACEMENT_TEXT)
 
   }
+
+  test("tryWithSafeFinally") {
+var e = new Error("Block0")
+val finallyBlockError = new Error("Finally Block")
+var isErrorOccurred = false
+// if the try and finally blocks throw different exception instances
+