Skip to content

Commit a8480f8

Browse files
committed
Fix lost asynchronous exception in asyncFetchPackages (fixes haskell#6322)
The concrete bug is that `try` catches asynchronous exceptions, preventing `withAsync` from interrupting it when its `body` action finishes prematurely. This manifests during `cabal build` when hitting Ctrl-C during download, e.g.: 1. Ctrl-C interrupts a curl process 2. This interrupt is converted into a UserInterrupt exception via the process package's delegate_ctlc functionality. 3. UserInterrupt bubbles up through `fetchPackage`, is caught by the `try` (fine) and writen to the result mvar. Meanwhile, `fetchPackage` continues its loop, starting to fetch the next package. 4. Some `rebuildTarget` is waiting for the interrupted download; `waitAsyncFetchPackage` rethrows UserInterrupt in that thread. 5. UserInterrupt bubbles up through `collectJob`, `execute`, the `body` action. 6. `withAsync`'s finalizer cancels the `fetchPackages` async. 7. The `fetchPackages` async receives the AsyncCancelled exception while fetching the next package (see 3. above). This interrupts the download action (it shouldn't matter whether we happen to be running curl again or not), and AsyncCancelled bubbles up through `fetchPackage`, is caught by the `try` (not fine!) and written to the mvar. But no-one is reading that mvar anymore because the build job already aborted (step 5), and that AsyncCancelled exception is lost. 8. `fetchPackages` keeps doing its thing, exiting eventually. Note that this change affects both instances of `try` in this story: `UserInterrupt` is also an asynchronous exception, so after the change the `UserInterrupt` takes a different path from step 3. That appears to be fine, though.
1 parent b6d0516 commit a8480f8

File tree

2 files changed

+7
-4
lines changed

2 files changed

+7
-4
lines changed

cabal-install/cabal-install.cabal

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,8 @@ library
226226
text >= 1.2.3 && < 1.3,
227227
parsec >= 3.1.13.0 && < 3.2,
228228
regex-base >= 0.94.0.0 && <0.95,
229-
regex-posix >= 0.96.0.0 && <0.97
229+
regex-posix >= 0.96.0.0 && <0.97,
230+
safe-exceptions
230231

231232
if flag(native-dns)
232233
if os(windows)

cabal-install/src/Distribution/Client/FetchUtils.hs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ import Distribution.Client.Utils
5252
( ProgressPhase(..), progressMessage )
5353

5454
import qualified Data.Map as Map
55-
import Control.Exception
55+
import qualified Control.Exception.Safe as SafeExceptions
5656
import Control.Concurrent.Async
5757
import Control.Concurrent.MVar
5858
import System.Directory
@@ -247,8 +247,10 @@ asyncFetchPackages verbosity repoCtxt pkglocs body = do
247247
fetchPackages =
248248
for_ asyncDownloadVars $ \(pkgloc, var) -> do
249249
-- Suppress marking here, because 'withAsync' means
250-
-- that we get nondeterministic interleaving
251-
result <- try $ fetchPackage (verboseUnmarkOutput verbosity)
250+
-- that we get nondeterministic interleaving.
251+
-- It is essential that we don't catch async exceptions here,
252+
-- specifically AsyncCancelled thrown at us from withAsync.
253+
result <- SafeExceptions.try $ fetchPackage (verboseUnmarkOutput verbosity)
252254
repoCtxt pkgloc
253255
putMVar var result
254256

0 commit comments

Comments
 (0)