connection-manager: make it possible to connect to oneself

When connecting to itself, the connection manager would create two
`TVar`'s with the state of the connection eventually leading to invalid
transitions.  We also mark the `IncludeConnection Outbound
→ IncludeConnection Inbound` a valid transition, which will happen once
when connecting to oneself.

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

@@ -1048,11 +1048,16 @@ withConnectionManager ConnectionManagerArguments {
                 let connState' = UnnegotiatedState provenance connId connThread
                 (mutableConnVar', connState0') <-
                   atomically $ do
-                    v <- newMutableConnState freshIdSupply connState'
-                    labelTVar (connVar v) ("conn-state-" ++ show connId)
-                    connState0' <- traverse (readTVar . connVar)
-                                            (Map.lookup peerAddr state)
-                    return (v, connState0')
+                    let v0 = Map.lookup peerAddr state
+                    connState0' <- traverse (readTVar . connVar) v0
+                    case v0 of
+                      Nothing -> do
+                        v <- newMutableConnState freshIdSupply connState'
+                        labelTVar (connVar v) ("conn-state-" ++ show connId)
+                        return (v, connState0')
+                      Just v -> do
+                        writeTVar (connVar v) connState'
+                        return (v, connState0')
                 connThread' <-
                   forkConnectionHandler
                      stateVar mutableConnVar' socket connId writer handler
@@ -1599,7 +1604,7 @@ withConnectionManager ConnectionManagerArguments {
                     (openToConnect cmSnocket peerAddr)
                     (\socket -> uninterruptibleMask_ $ do
                       close cmSnocket socket
-                      res <- atomically $ modifyTMVarSTM stateVar $ \state -> do
+                      trs <- atomically $ modifyTMVarSTM stateVar $ \state -> do
                         case Map.lookup peerAddr state of
                           -- Lookup failed, which means connection was already
                           -- removed.  So we just update the connVar and trace
@@ -1610,41 +1615,42 @@ withConnectionManager ConnectionManagerArguments {
                             writeTVar connVar connState'
                             return
                               ( state
-                              , Right [ mkTransition connState connState'
-                                      , Transition (Known connState') Unknown
-                                      ]
+                              , [ mkTransition connState connState'
+                                , Transition (Known connState') Unknown
+                                ]
                               )
 
                           -- Current connVar.
-                          Just mutableConnState' ->
-                            -- If accept call returned first than connect then
-                            -- the connVar will be replaced. If it was
-                            -- replaced then we do not need to do anything.
-                            -- Otherwise, we need to remove the connVar from
-                            -- the state and trace accordingly.
-                            if mutableConnState' == mutableConnState
-                               then do
-                                connState <- readTVar connVar
+                          Just mutableConnState' -> do
+                            connState <- readTVar connVar
+                            case connState of
+                              -- Update the state only if the connection was in
+                              -- 'ReservedOutboundState'.  This covers the case
+                              -- when we connect to our selves, in which
+                              -- case: we first set the connection state to
+                              -- `ReservedOutboundState`, then race connect
+                              -- & accept calls.  If the connection was
+                              -- accepted it, it will use the same
+                              -- 'MutableConnState', and if the inbound side is
+                              -- using the connection the state will be
+                              -- different than `ReservedOutboundState`.
+                              ReservedOutboundState | mutableConnState' == mutableConnState -> do
                                 let state' = Map.delete peerAddr state
                                     connState' = TerminatedState Nothing
                                 writeTVar connVar connState'
                                 return
                                   ( state'
-                                  , Right [ mkTransition connState connState'
-                                          , Transition (Known connState')
-                                                       Unknown
-                                          ]
-                                  )
-                               else
-                                return
-                                  ( state
-                                  , Left ()
+                                  , [ mkTransition connState connState'
+                                    , Transition (Known connState')
+                                                 Unknown
+                                    ]
                                   )
 
-                      case res of
-                        Left _ -> pure ()
-                        Right trs ->
-                          traverse_ (traceWith trTracer . TransitionTrace peerAddr) trs
+                                -- the connection might have been accepted, in
+                                -- such case do not modify its state.
+                              _ -> return (state, [])
+
+                      traverse_ (traceWith trTracer . TransitionTrace peerAddr) trs
                       traceCounters stateVar
                     )
                     $ \socket -> do

ouroboros-network-framework/testlib/TestLib/ConnectionManager.hs

@@ -97,6 +97,8 @@ verifyAbstractTransition Transition { fromState, toState } =
       (OutboundIdleSt Duplex, InboundSt Duplex) -> True
       (OutboundIdleSt _dataFlow, TerminatingSt) -> True
 
+      (UnnegotiatedSt Outbound, UnnegotiatedSt Inbound) -> True
+
       --
       -- Terminate
       --
@@ -212,6 +214,7 @@ allValidTransitionsNames =
   [ Transition UnknownConnectionSt             ReservedOutboundSt
   -- , Transition TerminatedSt                    ReservedOutboundSt
   , Transition ReservedOutboundSt              (UnnegotiatedSt Outbound)
+  , Transition (UnnegotiatedSt Outbound)       (UnnegotiatedSt Inbound)
   , Transition (UnnegotiatedSt Outbound)       OutboundUniSt
   , Transition (UnnegotiatedSt Outbound)       (OutboundDupSt Ticking)
   , Transition OutboundUniSt                   (OutboundIdleSt Unidirectional)