Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/6918#issuecomment-113734145
I think that my concern was that wrapping a Throwable with a cause and
re-throwing it might not be safe in general because catchers higher up the
stack would then receive a different exception than the original Throwable,
which might lead to problems if the throwable was an Error that's treated
specially (e.g. ThreadDeath).
I suppose one approach would be to not re-wrap the Throwable when
re-throwing. but AFAIK the compiler will complain if you re-throw `Throwable`
in a method that doesn't declare that it `throws Throwable`. I think there's a
reference for this in Guava's Throwables documentation:
https://code.google.com/p/guava-libraries/wiki/ThrowablesExplained. It looks
like this is a Java 6-ism, though, since Java 7 seems to allow this type of
re-throwing:
https://stackoverflow.com/questions/7428966/how-does-the-correct-rethrow-code-look-like-for-this-example
It looks like Java 7 provides us with a nice way to handle this with
try-with-resources (see
https://stuartmarks.wordpress.com/2011/07/21/a-new-java-exception-handling-idiom/),
but I don't think we can use this here as long as we want to backport this fix
to 1.4.1, which still must build for Java 6.
I think that's why I originally decided to go with the `finally` block,
although I guess I should have added more try blocks to guard against errors
during the cleanup itself.
---
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 [email protected] or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
