Closed snoyberg closed 6 years ago
Note that I'm not particularly happy about the fallback here for older GHCs (base < 4.7), but there's not much choice in the matter. The alternative would be dropping support for building hackage-security with older versions of GHC. I'm not sure if that's an option, but it would be more reliable, especially with third-party libraries potentially defining their own async exception types (such as the async package).
LGTM. catchChecked
was never intended to catch asynchronous exceptions; indeed, it was intended purely to catch checked exceptions, and asynchronous exceptions never are checked. Thanks for the PR!
Agreed on squashing, let me know if you want me to do that, or you want to do it at merge time.
Thanks for merging this and #203 @hvr. We're deciding whether to move ahead with merging https://github.com/commercialhaskell/stack/pull/3865 or wait until these changes are released on Hackage. Do you have an ETA on a Hackage release for these changes?
This commit uses the same async-exception detection mechanism as is used by the safe-exceptions package, via checking if the given exception is cast to a SomeAsyncException. (On older GHCs without SomeAsyncException, it contains a hard-coded list of async exception types.) It then ensures that:
Unfortunately, I don't currently have a reliable test case to ensure that this fixes the problems described in #187. Hopefully with this patch available we can begin testing cabal-install and Stack against the change and see if it resolves the issues.