Fix ConnectionManager finally handle

This includes changes to the way a node behaves when its InboundGovernor
dies: changed link to linkOnly (const True) to correctly propagate
AsyncCancelled exceptions to the ConnectionManager and thus terminate it
as well.

The ConnectionManager needs to take care of closing the connection
handlers of each connection in a way that does not make us go beyond the
enforced timeouts. Since closing a connection is blocking we need to
mask exceptions during the exception handler and since doing it
sequentially can lead to waits on atomically blocks, we close each
connection in parallel.

ouroboros-network-framework/src/Ouroboros/Network/ConnectionManager/Core.hs

@@ -35,13 +35,13 @@ import           Control.Monad.Class.MonadTimer
 import           Control.Tracer (Tracer, contramap, traceWith)
 import           Data.Foldable (foldMap', traverse_)
 import           Data.Function (on)
-import           Data.Functor (void, ($>))
+import           Data.Functor (($>))
 import           Data.Maybe (maybeToList)
 import           Data.Proxy (Proxy (..))
 import           Data.Typeable (Typeable)
 import           GHC.Stack (CallStack, HasCallStack, callStack)
 
-import           Data.Map (Map, traverseWithKey)
+import           Data.Map (Map)
 import qualified Data.Map as Map
 import qualified Data.Set as Set
 
@@ -652,12 +652,18 @@ withConnectionManager ConnectionManagerArguments {
               }
 
     k connectionManager
-      `finally` do
+      -- Since this exception handler is blocking it might receive exceptions
+      -- during its execution, which we want to avoid, so we wrap it around
+      -- uninterruptibleMask_.
+      `finally` uninterruptibleMask_ (do
         traceWith tracer TrShutdown
 
         state <- atomically $ readTMVar stateVar
-        void $ traverseWithKey
-          (\peerAddr MutableConnState { connVar } -> do
+        -- Spawning one thread for each connection cleanup avoids spending time
+        -- waiting for locks and cleanup logic that could delay closing the
+        -- connections and making us not respecting certain timeouts.
+        forConcurrently_ (Map.assocs state)
+          (\(peerAddr, MutableConnState { connVar }) -> do
             -- cleanup handler for that thread will close socket associated
             -- with the thread.  We put each connection in 'TerminatedState' to
             -- try that none of the connection threads will enter
@@ -682,8 +688,7 @@ withConnectionManager ConnectionManagerArguments {
             -- using 'cancel' here, since we want to block until connection
             -- handler thread terminates.
             traverse_ cancel (getConnThread connState)
-          )
-          state
+          ))
   where
     traceCounters :: StrictTMVar m (ConnectionManagerState peerAddr handle handleError version m) -> m ()
     traceCounters stateVar = do

ouroboros-network-framework/test/Test/Ouroboros/Network/Server2.hs

@@ -1680,7 +1680,7 @@ multinodeExperiment inboundTrTracer trTracer cmTracer inboundTracer
                           timeLimitsHandshake
                           acceptedConnLimit
                           ( \ connectionManager _ serverAsync -> do
-                            link serverAsync
+                            linkOnly (const True) serverAsync
                             connectionLoop SingInitiatorResponderMode localAddr cc connectionManager Map.empty connVar
                             return Nothing
                           )
@@ -2478,7 +2478,8 @@ prop_timeouts_enforced serverAcc (ArbDataFlow dataFlow)
                        . getTraceEvents
                        $ trace
 
-  in verifyAllTimeouts transitionSignal
+  in counterexample (ppTrace trace)
+   $ verifyAllTimeouts transitionSignal
   where
     verifyAllTimeouts :: [[(Time , AbstractTransitionTrace SimAddr)]]
                       -> Property