[tor-commits] [tordnsel/master] fix exceptions in TorDNSEL.Control.Concurrent
arlo at torproject.org
arlo at torproject.org
Sat Apr 16 06:08:43 UTC 2016
commit 7494d418a991eb60b4644169310026336068d634
Author: David Kaloper <david at numm.org>
Date: Mon Aug 26 04:10:27 2013 +0200
fix exceptions in TorDNSEL.Control.Concurrent
---
src/TorDNSEL/Control/Concurrent/Link.hs | 7 +-
src/TorDNSEL/Control/Concurrent/Link/Internals.hs | 168 ++++++++++++----------
src/TorDNSEL/Control/Concurrent/Util.hs | 47 +++---
3 files changed, 121 insertions(+), 101 deletions(-)
diff --git a/src/TorDNSEL/Control/Concurrent/Link.hs b/src/TorDNSEL/Control/Concurrent/Link.hs
index 98eda78..2c0a05f 100644
--- a/src/TorDNSEL/Control/Concurrent/Link.hs
+++ b/src/TorDNSEL/Control/Concurrent/Link.hs
@@ -30,15 +30,16 @@ module TorDNSEL.Control.Concurrent.Link (
, withMonitor
, exit
, throwTo
- , throwDynTo
, killThread
, setTrapExit
, unsetTrapExit
- , ExitReason
+ , ExitReason(..)
+ , exitReason
+ , isAbnormal
+ , throwAbnormal
, extractReason
, fromExitSignal
, LinkException(..)
- , showLinkException
) where
import TorDNSEL.Control.Concurrent.Link.Internals
diff --git a/src/TorDNSEL/Control/Concurrent/Link/Internals.hs b/src/TorDNSEL/Control/Concurrent/Link/Internals.hs
index cf83cba..7b614b9 100644
--- a/src/TorDNSEL/Control/Concurrent/Link/Internals.hs
+++ b/src/TorDNSEL/Control/Concurrent/Link/Internals.hs
@@ -1,3 +1,4 @@
+{-# LANGUAGE CPP #-}
{-# OPTIONS_GHC -fno-ignore-asserts #-}
-----------------------------------------------------------------------------
@@ -24,15 +25,18 @@
-- #not-home
module TorDNSEL.Control.Concurrent.Link.Internals where
+import qualified Control.Exception as E
import qualified Control.Concurrent as C
import Control.Concurrent.MVar
(MVar, newMVar, withMVar, modifyMVar, modifyMVar_)
-import qualified Control.Exception as E
+import GHC.Conc (setUncaughtExceptionHandler)
+import System.Exit (ExitCode)
import Control.Monad (unless)
import qualified Data.Foldable as F
import qualified Data.Map as M
import qualified Data.Set as S
-import Data.Dynamic (Dynamic, fromDynamic, toDyn, Typeable)
+-- import Data.Dynamic (Dynamic, fromDynamic, toDyn, Typeable)
+import Data.Typeable (Typeable)
import Data.List (nub)
import Data.Unique (Unique, newUnique)
import System.IO (hPutStrLn, hFlush, stderr)
@@ -46,6 +50,9 @@ import TorDNSEL.Util
newtype ThreadId = Tid Unique
deriving (Eq, Ord)
+-- ( Else an orphaned Show Unique... )
+instance Show ThreadId where show _ = "#<thread>"
+
-- | Return the 'ThreadId' of the calling thread.
myThreadId :: IO ThreadId
myThreadId = do
@@ -74,11 +81,17 @@ threadMap :: MVar ThreadMap
{-# NOINLINE threadMap #-}
threadMap = unsafePerformIO . newMVar $ ThreadMap M.empty M.empty
+-- | Erase 'threadMap', making it possible to re-run 'withLinksDo'. For
+-- debugging only, as it loses track of threads.
+unsafeResetThreadMap :: IO ()
+unsafeResetThreadMap =
+ modifyMVar_ threadMap $ const . return $ ThreadMap M.empty M.empty
+
-- | Assert various invariants of the global link and monitor state, printing a
-- message to stdout if any assertions fail.
assertThreadMap :: ThreadMap -> IO ()
assertThreadMap tm =
- E.handleJust E.assertions (putStr . ("assertThreadMap: " ++)) $
+ E.handle (\(E.AssertionFailed msg) -> putStrLn $ "assertThreadMap: " ++ msg) $
E.assert (M.size (ids tm) > 0) $
E.assert (M.size (ids tm) == M.size (state tm)) $
E.assert (M.elems (ids tm) == nub (M.elems (ids tm))) $
@@ -100,36 +113,35 @@ assertThreadMap tm =
-- | An internal type used to transmit the originating 'ThreadId' in an
-- asynchronous exception to a linked thread.
data ExitSignal = ExitSignal !ThreadId !ExitReason
- deriving Typeable
-
--- | Extract the 'ExitReason' from an 'ExitSignal' contained within a
--- dynamically-typed exception. If the exception doesn't contain an
--- 'ExitSignal', tag it with 'Just'.
-extractReason :: E.Exception -> ExitReason
-extractReason (E.DynException dyn)
- | Just (ExitSignal _ e) <- fromDynamic dyn = e
-extractReason e = Just e
-
--- | Extract an exit signal from an 'E.Exception' if it has the right type.
-fromExitSignal :: Typeable a => E.Exception -> Maybe (ThreadId, a)
-fromExitSignal (E.DynException d)
- | Just (ExitSignal tid (Just (E.DynException d'))) <- fromDynamic d
- = (,) tid `fmap` fromDynamic d'
+ deriving (Show, Typeable)
+
+instance E.Exception ExitSignal
+
+-- | Extract the 'ExitReason' from an 'ExitSignal' contained within an
+-- exception. If the exception doesn't contain an 'ExitReason', it becomes the
+-- exit reason itself.
+extractReason :: E.SomeException -> ExitReason
+extractReason (E.fromException -> Just (ExitSignal _ e)) = e
+extractReason e = AbnormalExit e
+
+-- | Extract a particular exception type from an 'ExitSignal', if present.
+fromExitSignal :: E.Exception e => ExitSignal -> Maybe (ThreadId, e)
+fromExitSignal (ExitSignal tid (AbnormalExit e))
+ | Just e' <- E.fromException e = Just (tid, e')
fromExitSignal _ = Nothing
-- | The default action used to signal a thread. Abnormal 'ExitReason's are
-- sent to the thread and normal exits are ignored.
defaultSignal :: C.ThreadId -> ThreadId -> ExitReason -> IO ()
-defaultSignal dst src e@(Just _) = E.throwDynTo dst $ ExitSignal src e
-defaultSignal _ _ Nothing = return ()
-
--- | Initialize the state supporting links and monitors. Use the given function
--- to display an uncaught exception. It is an error to call this function
--- outside the main thread, or to call any other functions in this module
--- outside this function.
-withLinksDo :: (E.Exception -> String) -> IO a -> IO ()
-withLinksDo showE io = E.block $ do
- E.setUncaughtExceptionHandler . const . return $ ()
+defaultSignal dst src NormalExit = return ()
+defaultSignal dst src e = E.throwTo dst $ ExitSignal src e
+
+-- | Initialize the state supporting links and monitors. It is an error to call
+-- this function outside the main thread, or to call any other functions in this
+-- module outside this function.
+withLinksDo :: IO a -> IO ()
+withLinksDo io = E.mask $ \restore -> do
+ setUncaughtExceptionHandler . const . return $ ()
main <- C.myThreadId
mainId <- Tid `fmap` newUnique
let initialState = ThreadState
@@ -146,12 +158,13 @@ withLinksDo showE io = E.block $ do
, state = M.insert main initialState (state tm) }
-- Don't bother propagating signals from the main thread
-- since it's about to exit.
- (E.unblock io >> return ()) `E.catch` \e ->
+ (restore io >> return ()) `E.catch` \(e :: E.SomeException) ->
case extractReason e of
- Nothing -> return ()
- Just e'@(E.ExitException _) -> E.throwIO e'
- Just e' -> do
- hPutStrLn stderr ("*** Exception: " ++ showE e')
+ NormalExit -> return ()
+ AbnormalExit (E.fromException -> Just e') ->
+ E.throwIO (e' :: ExitCode)
+ AbnormalExit e' -> do
+ hPutStrLn stderr ("*** Exception: " ++ show e')
hFlush stderr
E.throwIO e'
@@ -169,15 +182,19 @@ forkLinkIO' shouldLink io = E.block $ do
parent <- C.myThreadId
childId <- Tid `fmap` newUnique
modifyMVar_ threadMap $ \tm -> do
- -- assertThreadMap tm
+#ifdef DEBUG
+ assertThreadMap tm
+#endif
child <- forkHandler $ do
child <- C.myThreadId
- e <- either extractReason (const Nothing) `fmap` E.try (E.unblock io)
+ e <- either extractReason (const NormalExit) `fmap` E.try (E.unblock io)
-- modifyMVar is interruptible (a misfeature in this case), so an async
-- exception could be delivered here. Forking an anonymous thread should
-- avoid this race since nobody can throwTo it.
forkHandler $ do
- -- withMVar threadMap assertThreadMap
+#ifdef DEBUG
+ withMVar threadMap assertThreadMap
+#endif
(signalAll,notifyAll) <- modifyMVar threadMap $ \tm1 ->
let s = state tm1 M.! child
unlinkAll = flip (F.foldl' (flip (child `unlinkFrom`))) (links s)
@@ -193,7 +210,9 @@ forkLinkIO' shouldLink io = E.block $ do
notifyAll ex = F.mapM_ (forkHandler . ($ ex) . fst) (monitors s)
tm1' = tm1 { ids = newIds, state = newState }
in tm1' `seq` return (tm1', (signalAll, notifyAll))
- -- withMVar threadMap assertThreadMap
+#ifdef DEBUG
+ withMVar threadMap assertThreadMap
+#endif
notifyAll e
signalAll e
@@ -211,11 +230,13 @@ forkLinkIO' shouldLink io = E.block $ do
tm { ids = M.insert childId child $ ids tm
, state = M.insert child initialState . linkToParent $ state tm }
- -- withMVar threadMap assertThreadMap
+#ifdef DEBUG
+ withMVar threadMap assertThreadMap
+#endif
return childId
where
forkHandler = C.forkIO . ignore . (>> return ()) . E.block
- ignore = E.handle . const . return $ ()
+ ignore = E.handle $ \(e :: E.SomeException) -> return ()
-- | Establish a bidirectional link between the calling thread and a given
-- thread. If either thread terminates, an exit signal will be sent to the other
@@ -225,16 +246,14 @@ forkLinkIO' shouldLink io = E.block $ do
linkThread :: ThreadId -> IO ()
linkThread tid = do
me <- C.myThreadId
- mbSignalSelf <- modifyMVar threadMap $ \tm ->
+ mbSignalSelf <- modifyMVar threadMap $ \tm -> return $!
case M.lookup tid (ids tm) of
Just tid'
- | tid' == me -> return (tm, Nothing)
- | otherwise -> let tm' = tm { state = linkTogether me tid' $ state tm }
- in tm' `seq` return (tm', Nothing)
- Nothing ->
- let s = state tm M.! me
- in return (tm, Just . signal s tid . Just . E.DynException .
- toDyn $ NonexistentThread)
+ | tid' == me -> (tm, Nothing)
+ | otherwise -> (tm', Nothing)
+ where !tm' = tm { state = linkTogether me tid' $ state tm }
+ Nothing -> (tm, Just . signal s tid $ exitReason NonexistentThread)
+ where s = state tm M.! me
whenJust mbSignalSelf id
where linkTogether x y = (x `linkTo` y) . (y `linkTo` x)
@@ -258,9 +277,22 @@ linkTo, unlinkFrom :: C.ThreadId -> C.ThreadId -> StateModifier
data Monitor = Monitor !ThreadId !Unique
deriving (Eq, Ord)
--- | The reason a thread was terminated. @Nothing@ means the thread exited
--- normally. @Just exception@ contains the reason for an abnormal exit.
-type ExitReason = Maybe E.Exception
+-- | The reason a thread was terminated.
+data ExitReason = NormalExit | AbnormalExit E.SomeException
+ deriving (Show)
+
+-- | Construct an 'ExitReason' from any 'E.Exception'.
+exitReason :: E.Exception e => e -> ExitReason
+exitReason = AbnormalExit . E.toException
+
+isAbnormal :: ExitReason -> Bool
+isAbnormal (AbnormalExit _) = True
+isAbnormal _ = False
+
+-- | Check the exit reason and re-throw it if it's an 'AbnormalExit'.
+throwAbnormal :: ExitReason -> IO ()
+throwAbnormal NormalExit = return ()
+throwAbnormal (AbnormalExit e) = E.throwIO e
-- | Start monitoring the given thread, invoking an 'IO' action with the
-- 'ExitReason' when the thread dies. Return a handle to the monitor, which can
@@ -271,20 +303,19 @@ monitorThread :: ThreadId -> (ExitReason -> IO ()) -> IO Monitor
monitorThread tid notify = do
me <- C.myThreadId
mon@(Monitor _ monId) <- Monitor tid `fmap` newUnique
- let cleanup tid' = adjust' (\ts ->
- ts { ownedMons = S.delete (tid', monId) (ownedMons ts) }) me
+ let cleanup tid' = flip adjust' me $ \ts ->
+ ts { ownedMons = S.delete (tid', monId) (ownedMons ts) }
addMon tid' ts = ts { monitors = M.insert monId (notify, cleanup tid')
(monitors ts) }
addOwned tid' ts = ts {ownedMons = S.insert (tid', monId) (ownedMons ts)}
- exists <- modifyMVar threadMap $ \tm ->
+ exists <- modifyMVar threadMap $ \tm -> return $!
case M.lookup tid (ids tm) of
- Nothing -> return (tm, False)
- Just tid' ->
- let tm' = tm { state = adjust' (addMon tid') tid' .
- adjust' (addOwned tid') me $ state tm }
- in tm' `seq` return (tm', True)
+ Nothing -> (tm, False)
+ Just tid' -> (tm', True)
+ where !tm' = tm { state = adjust' (addMon tid') tid' .
+ adjust' (addOwned tid') me $ state tm }
unless exists $
- notify . Just . E.DynException . toDyn $ NonexistentThread
+ notify $ exitReason NonexistentThread
return mon
-- | Cancel a monitor, if it is currently active.
@@ -310,10 +341,10 @@ withMonitor tid notify =
-- | Terminate the calling thread with the given 'ExitReason'.
exit :: ExitReason -> IO a
-exit e = E.throwDyn . flip ExitSignal e =<< myThreadId
+exit e = E.throwIO . (`ExitSignal` e) =<< myThreadId
-- | Send an exit signal with an 'ExitReason' to a thread. If the 'ExitReason'
--- is 'Nothing', the signal will be ignored unless the target thread is trapping
+-- is 'NormalExit', the signal will be ignored unless the target thread is trapping
-- signals. Otherwise, the target thread will either exit with the same
-- 'ExitReason' or be notified of the signal depending on whether it is trapping
-- signals. If the target thread doesn't exist, do nothing.
@@ -324,25 +355,20 @@ throwTo tid e = do
let me' = ident (state tm M.! me)
in if tid == me'
-- special case: an exception thrown to oneself is untrappable
- then E.throwDyn $ ExitSignal me' e
+ then E.throwIO $ ExitSignal me' e
else return $ do tid' <- M.lookup tid (ids tm)
return $ signal (state tm M.! tid') me'
-- since signal can block, we don't want to hold a lock on threadMap
whenJust mbSignal ($ e)
--- | A variant of 'throwTo' for dynamically typed 'ExitReason's.
-throwDynTo :: Typeable a => ThreadId -> a -> IO ()
-throwDynTo tid = throwTo tid . Just . E.DynException . toDyn
-
-- | Send an untrappable exit signal to a thread, if it exists.
killThread :: ThreadId -> IO ()
killThread tid = do
me <- C.myThreadId
mbSignal <- withMVar threadMap $ \tm -> return $ do
tid' <- M.lookup tid (ids tm)
- return .
- E.throwDynTo tid' $ ExitSignal (ident (state tm M.! me))
- (Just (E.AsyncException E.ThreadKilled))
+ return . E.throwTo tid' $ ExitSignal (ident (state tm M.! me))
+ (exitReason E.ThreadKilled)
whenJust mbSignal id
-- | Redirect exit signals destined for the calling thread to the given 'IO'
@@ -367,6 +393,4 @@ data LinkException = NonexistentThread -- ^
instance Show LinkException where
show NonexistentThread = "Attempt to link to nonexistent thread"
--- | Boilerplate conversion of a dynamically typed 'LinkException' to a string.
-showLinkException :: Dynamic -> Maybe String
-showLinkException = fmap (show :: LinkException -> String) . fromDynamic
+instance E.Exception LinkException
diff --git a/src/TorDNSEL/Control/Concurrent/Util.hs b/src/TorDNSEL/Control/Concurrent/Util.hs
index 1defc31..a7c2a65 100644
--- a/src/TorDNSEL/Control/Concurrent/Util.hs
+++ b/src/TorDNSEL/Control/Concurrent/Util.hs
@@ -13,7 +13,8 @@
module TorDNSEL.Control.Concurrent.Util where
import qualified Control.Exception as E
-import Control.Concurrent.MVar (newEmptyMVar, takeMVar, putMVar, tryPutMVar)
+import Data.Functor ( (<$) )
+import Control.Concurrent.MVar (MVar, newEmptyMVar, takeMVar, putMVar, tryPutMVar)
import Data.Dynamic (Dynamic)
import Data.Maybe (isJust)
import System.Timeout
@@ -51,10 +52,9 @@ terminateThread mbWait tid terminate = do
sendSyncMessage :: (IO () -> IO ()) -> ThreadId -> IO ()
sendSyncMessage sendMsg tid = do
err <- newEmptyMVar
- let putResponse = (>> return ()) . tryPutMVar err
- withMonitor tid putResponse $ do
- sendMsg $ putResponse Nothing
- takeMVar err >>= flip whenJust E.throwIO
+ withMonitor tid (tryPutMVar_ err) $ do
+ sendMsg $ tryPutMVar_ err NormalExit
+ takeMVar err >>= throwAbnormal
-- | Send a message parameterized by a reply action to @tid@, returning the
-- response value. If the thread exits before responding to the message, throw
@@ -62,19 +62,13 @@ sendSyncMessage sendMsg tid = do
call :: ((a -> IO ()) -> IO ()) -> ThreadId -> IO a
call sendMsg tid = do
mv <- newEmptyMVar
- let putResponse = (>> return ()) . tryPutMVar mv
- withMonitor tid (putResponse . Left) $ do
- sendMsg $ putResponse . Right
+ withMonitor tid (tryPutMVar_ mv . Left) $ do
+ sendMsg $ tryPutMVar_ mv . Right
response <- takeMVar mv
case response of
- Left Nothing -> E.throwDyn NonexistentThread
- Left (Just e) -> E.throwIO e
- Right r -> return r
-
--- | A wrapper for using 'showException' with 'ExitReason's.
-showExitReason :: [Dynamic -> Maybe String] -> ExitReason -> String
-showExitReason _ Nothing = "Normal exit"
-showExitReason fs (Just e) = showException (showLinkException:fs) e
+ Left NormalExit -> E.throwIO NonexistentThread
+ Left (AbnormalExit e) -> E.throwIO e
+ Right r -> return r
-- | Invoke the given 'IO' action in a new thread, passing it an action to
-- invoke when it has successfully started. Link the new thread to the calling
@@ -83,14 +77,13 @@ showExitReason fs (Just e) = showException (showLinkException:fs) e
startLink :: (IO () -> IO a) -> IO ThreadId
startLink io = do
sync <- newEmptyMVar
- err <- newEmptyMVar
- let putResponse = (>> return ()) . tryPutMVar err
+ err <- newEmptyMVar
tid <- forkLinkIO $ do
takeMVar sync
- io (putResponse Nothing)
- withMonitor tid putResponse $ do
+ io $ tryPutMVar_ err NormalExit
+ withMonitor tid (tryPutMVar_ err) $ do
putMVar sync ()
- takeMVar err >>= flip whenJust E.throwIO
+ takeMVar err >>= throwAbnormal
return tid
-- | Invoke the given 'IO' action in a temporary thread (linked to the calling
@@ -100,15 +93,17 @@ startLink io = do
tryForkLinkIO :: Thread a => IO a -> IO (Either ExitReason a, ThreadId)
tryForkLinkIO io = do
sync <- newEmptyMVar
- response <- newEmptyMVar
- let putResponse = (>> return ()) . tryPutMVar response
+ resp <- newEmptyMVar
intermediate <- forkLinkIO $ do
takeMVar sync
- io >>= putResponse . Right
- r <- withMonitor intermediate (putResponse . Left) $ do
+ io >>= tryPutMVar_ resp . Right
+ r <- withMonitor intermediate (tryPutMVar_ resp . Left) $ do
putMVar sync ()
E.block $ do
- r <- takeMVar response
+ r <- takeMVar resp
either (const $ return ()) (linkThread . threadId) r
return r
return (r, intermediate)
+
+tryPutMVar_ :: MVar a -> a -> IO ()
+tryPutMVar_ = ((() <$) .) . tryPutMVar
More information about the tor-commits
mailing list