Skip to content

Commit

Permalink
connection-manager: renamed methods (#5000)
Browse files Browse the repository at this point in the history
  • Loading branch information
coot authored Oct 29, 2024
1 parent 0d72db1 commit 1a7ff3e
Show file tree
Hide file tree
Showing 10 changed files with 95 additions and 92 deletions.
3 changes: 3 additions & 0 deletions ouroboros-network-framework/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
* Addapted to `network-mux` changes in https://github.com/IntersectMBO/ouroboros-network/pull/4999
* Addapted to `network-mux` changes in https://github.com/IntersectMBO/ouroboros-network/pull/4997
* Removed deprecated `Ouroboros.Network.Channel.{to,from}Channel` functions.
* Renamed `requestOutboundConnection` to `acquireOutboundConnection` and
`unregister{Inbound,Outbound}Connection` to `release{Inbound,Outbound}Connection`.
`AssertionLocation` constructors were renamed as well.

### Non-breaking changes

Expand Down
6 changes: 3 additions & 3 deletions ouroboros-network-framework/demo/connection-manager.hs
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@ bidirectionalExperiment
})
muxBundle
res <-
unregisterOutboundConnection
releaseOutboundConnection
connectionManager remoteAddr
case res of
UnsupportedState inState -> do
Expand Down Expand Up @@ -541,9 +541,9 @@ bidirectionalExperiment
Mux.InitiatorResponderMode
UnversionedProtocol))
connect n cm | n <= 1 =
requestOutboundConnection cm remoteAddr
acquireOutboundConnection cm remoteAddr
connect n cm =
requestOutboundConnection cm remoteAddr
acquireOutboundConnection cm remoteAddr
`catch` \(_ :: IOException) -> threadDelay 1
>> connect (pred n) cm
`catch` \(_ :: Mux.Error) -> threadDelay 1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -810,7 +810,7 @@ prop_valid_transitions (Fixed rnd) (SkewedBool bindToLocalAddress) scheduleMap =
-- another 5s is the longest delay for
-- handshake negotiation.
timeout (1 + 5 + testTimeWaitTimeout)
(requestOutboundConnection
(acquireOutboundConnection
connectionManager addr))
`catches`
[ Handler $ \(e :: IOException) -> return (Left (toException e))
Expand Down Expand Up @@ -849,11 +849,11 @@ prop_valid_transitions (Fixed rnd) (SkewedBool bindToLocalAddress) scheduleMap =
-- 'unregisterOutboundConnection' to be
-- successful.
void $
unregisterInboundConnection
releaseInboundConnection
connectionManager addr

res <-
unregisterOutboundConnection
releaseOutboundConnection
connectionManager addr
case res of
UnsupportedState st ->
Expand Down Expand Up @@ -929,7 +929,7 @@ prop_valid_transitions (Fixed rnd) (SkewedBool bindToLocalAddress) scheduleMap =
Left _ ->
-- TODO: should we run 'unregisterInboundConnection' depending on 'seActiveDelay'
void $
unregisterInboundConnection
releaseInboundConnection
connectionManager addr
go (thread : threads) acceptNext conns'
(AcceptFailure err, _acceptNext) ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -873,7 +873,7 @@ multinodeExperiment inboundTrTracer trTracer inboundTracer debugTracer cmTracer
case fromException e of
Just SomeAsyncException {} -> Nothing
_ -> Just e)
$ requestOutboundConnection cm remoteAddr
$ acquireOutboundConnection cm remoteAddr
case connHandle of
Left _ ->
go connMap
Expand All @@ -888,7 +888,7 @@ multinodeExperiment inboundTrTracer trTracer inboundTracer debugTracer cmTracer
m <- readTVar connVar
check (Map.member (connId remoteAddr) m)
writeTVar connVar (Map.delete (connId remoteAddr) m)
void (unregisterOutboundConnection cm remoteAddr)
void (releaseOutboundConnection cm remoteAddr)
go (Map.delete remoteAddr connMap)
RunMiniProtocols remoteAddr reqs -> do
atomically $ do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,7 @@ defaultResetTimeout = 5

newtype PruneAction m = PruneAction { runPruneAction :: m () }

-- | Instruction used internally in @unregisterOutboundConnectionImpl@, e.g. in
-- | Instruction used internally in @releaseOutboundConnectionImpl@, e.g. in
-- the implementation of one of the two @DemotedToCold^{dataFlow}_{Local}@
-- transitions.
--
Expand Down Expand Up @@ -690,11 +690,11 @@ with args@Arguments {
getConnectionManager =
WithInitiatorMode
OutboundConnectionManager {
ocmRequestConnection =
requestOutboundConnectionImpl freshIdSupply stateVar
ocmAcquireConnection =
acquireOutboundConnectionImpl freshIdSupply stateVar
outboundHandler,
ocmUnregisterConnection =
unregisterOutboundConnectionImpl stateVar stdGenVar
ocmReleaseConnection =
releaseOutboundConnectionImpl stateVar stdGenVar
},
readState,
waitForOutboundDemotion
Expand All @@ -708,8 +708,8 @@ with args@Arguments {
icmIncludeConnection =
includeInboundConnectionImpl freshIdSupply stateVar
inboundHandler,
icmUnregisterConnection =
unregisterInboundConnectionImpl stateVar,
icmReleaseConnection =
releaseInboundConnectionImpl stateVar,
icmPromotedToWarmRemote =
promotedToWarmRemoteImpl stateVar stdGenVar,
icmDemotedToColdRemote =
Expand All @@ -726,18 +726,18 @@ with args@Arguments {
getConnectionManager =
WithInitiatorResponderMode
OutboundConnectionManager {
ocmRequestConnection =
requestOutboundConnectionImpl freshIdSupply stateVar
ocmAcquireConnection =
acquireOutboundConnectionImpl freshIdSupply stateVar
outboundHandler,
ocmUnregisterConnection =
unregisterOutboundConnectionImpl stateVar stdGenVar
ocmReleaseConnection =
releaseOutboundConnectionImpl stateVar stdGenVar
}
InboundConnectionManager {
icmIncludeConnection =
includeInboundConnectionImpl freshIdSupply stateVar
inboundHandler,
icmUnregisterConnection =
unregisterInboundConnectionImpl stateVar,
icmReleaseConnection =
releaseInboundConnectionImpl stateVar,
icmPromotedToWarmRemote =
promotedToWarmRemoteImpl stateVar stdGenVar,
icmDemotedToColdRemote =
Expand Down Expand Up @@ -863,7 +863,7 @@ with args@Arguments {
cleanup :: m ()
cleanup =
-- We must ensure that we update 'connVar',
-- `requestOutboundConnection` might be blocked on it awaiting for:
-- `acquireOutboundConnection` might be blocked on it awaiting for:
-- - handshake negotiation; or
-- - `Terminate: TerminatingState → TerminatedState` transition.
-- That's why we use 'uninterruptibleMask'. Note that this cleanup
Expand Down Expand Up @@ -959,7 +959,7 @@ with args@Arguments {
in forceThreadDelay timeWaitTimeout
`finally` do
-- We must ensure that we update 'connVar',
-- `requestOutboundConnection` might be blocked on it awaiting for:
-- `acquireOutboundConnection` might be blocked on it awaiting for:
-- - handshake negotiation; or
-- - `Terminate: TerminatingState → TerminatedState` transition.
traceWith tracer (TrConnectionTimeWaitDone connId)
Expand Down Expand Up @@ -1125,7 +1125,7 @@ with args@Arguments {
--
-- This is subtle part, which needs to handle a near simultaneous
-- open. We cannot rely on 'ReservedOutboundState' state as
-- a lock. It may happen that the `requestOutboundConnection`
-- a lock. It may happen that the `acquireOutboundConnection`
-- will put 'ReservedOutboundState', but before it will call `connect`
-- the `accept` call will return. We overwrite the state and
-- replace the connection state 'TVar' with a fresh one. Nothing
Expand Down Expand Up @@ -1223,7 +1223,7 @@ with args@Arguments {
--
-- Note: we don't set an explicit timeout here. The
-- server will set a timeout and call
-- 'unregisterInboundConnection' when it expires.
-- 'releaseInboundConnection' when it expires.
--
UnnegotiatedState {} -> do
let connState' = InboundIdleState
Expand Down Expand Up @@ -1278,7 +1278,7 @@ with args@Arguments {
-- @
-- This is not needed! When we return from this call, the inbound
-- protocol governor will monitor the connection. Once it becomes
-- idle, it will call 'unregisterInboundConnection' which will
-- idle, it will call 'releaseInboundConnection' which will
-- perform the aforementioned @Commit@ transition.

if connected
Expand Down Expand Up @@ -1378,12 +1378,12 @@ with args@Arguments {
-- We need 'mask' in order to guarantee that the traces are logged if an
-- async exception lands between the successful STM action and the logging
-- action.
unregisterInboundConnectionImpl
releaseInboundConnectionImpl
:: StrictTMVar m (ConnectionManagerState peerAddr handle handleError version m)
-> peerAddr
-> m (OperationResult DemotedToColdRemoteTr)
unregisterInboundConnectionImpl stateVar peerAddr = mask_ $ do
traceWith tracer (TrUnregisterConnection Inbound peerAddr)
releaseInboundConnectionImpl stateVar peerAddr = mask_ $ do
traceWith tracer (TrReleaseConnection Inbound peerAddr)
(mbThread, mbTransition, result, mbAssertion) <- atomically $ do
state <- readTMVar stateVar
case Map.lookup peerAddr state of
Expand All @@ -1400,7 +1400,7 @@ with args@Arguments {
connState <- readTVar connVar
let st = abstractState (Known connState)
case connState of
-- In any of the following two states unregistering is not
-- In any of the following two states releasing is not
-- supported. 'includeInboundConnection' is a synchronous
-- operation which returns only once the connection is
-- negotiated.
Expand Down Expand Up @@ -1435,8 +1435,8 @@ with args@Arguments {
, Nothing
, OperationSuccess KeepTr
, Just (TrUnexpectedlyFalseAssertion
(UnregisterInboundConnection (Just connId)
st)
(ReleaseInboundConnection (Just connId)
st)
)
)

Expand All @@ -1454,8 +1454,8 @@ with args@Arguments {
, Nothing
, OperationSuccess CommitTr
, Just (TrUnexpectedlyFalseAssertion
(UnregisterInboundConnection (Just connId)
st)
(ReleaseInboundConnection (Just connId)
st)
)
)

Expand Down Expand Up @@ -1483,8 +1483,8 @@ with args@Arguments {
, Just (mkTransition connState connState')
, UnsupportedState st
, Just (TrUnexpectedlyFalseAssertion
(UnregisterInboundConnection (Just connId)
st)
(ReleaseInboundConnection (Just connId)
st)
)
)

Expand All @@ -1497,13 +1497,13 @@ with args@Arguments {
, Just (mkTransition connState connState')
, UnsupportedState st
, Just (TrUnexpectedlyFalseAssertion
(UnregisterInboundConnection (Just connId)
st)
(ReleaseInboundConnection (Just connId)
st)
)
)

-- If 'unregisterOutboundConnection' is called just before
-- 'unregisterInboundConnection', the latter one might observe
-- If 'releaseOutboundConnection' is called just before
-- 'releaseInboundConnection', the latter one might observe
-- 'TerminatingState'.
TerminatingState _connId _connThread _handleError ->
return ( Nothing
Expand All @@ -1512,7 +1512,7 @@ with args@Arguments {
, Nothing
)
-- However, 'TerminatedState' should not be observable by
-- 'unregisterInboundConnection', unless 'timeWaitTimeout' is
-- 'releaseInboundConnection', unless 'timeWaitTimeout' is
-- close to 'serverProtocolIdleTimeout'.
TerminatedState _handleError ->
return ( Nothing
Expand All @@ -1535,14 +1535,14 @@ with args@Arguments {

return result

requestOutboundConnectionImpl
acquireOutboundConnectionImpl
:: HasCallStack
=> FreshIdSupply m
-> StrictTMVar m (ConnectionManagerState peerAddr handle handleError version m)
-> ConnectionHandlerFn handlerTrace socket peerAddr handle handleError (version, versionData) m
-> peerAddr
-> m (Connected peerAddr handle handleError)
requestOutboundConnectionImpl freshIdSupply stateVar handler peerAddr = do
acquireOutboundConnectionImpl freshIdSupply stateVar handler peerAddr = do
let provenance = Outbound
traceWith tracer (TrIncludeConnection provenance peerAddr)
(trace, mutableConnState@MutableConnState { connVar }
Expand Down Expand Up @@ -1845,7 +1845,7 @@ with args@Arguments {
_ ->
return ( Nothing
, Just (TrUnexpectedlyFalseAssertion
(RequestOutboundConnection
(AcquireOutboundConnection
(Just connId)
(abstractState (Known connState))
)
Expand Down Expand Up @@ -1975,7 +1975,7 @@ with args@Arguments {
-- → OutboundState^\tau Duplex
-- @
-- This transition can happen if there are concurrent
-- `includeInboundConnection` and `requestOutboundConnection`
-- `includeInboundConnection` and `acquireOutboundConnection`
-- calls.
let connState' = OutboundDupState connId connThread handle Ticking
writeTVar connVar connState'
Expand Down Expand Up @@ -2117,29 +2117,29 @@ with args@Arguments {
return (Disconnected connId handleErrorM)


unregisterOutboundConnectionImpl
releaseOutboundConnectionImpl
:: StrictTMVar m
(ConnectionManagerState peerAddr handle handleError version m)
-> StrictTVar m StdGen
-> peerAddr
-> m (OperationResult AbstractState)
unregisterOutboundConnectionImpl stateVar stdGenVar peerAddr = do
traceWith tracer (TrUnregisterConnection Outbound peerAddr)
releaseOutboundConnectionImpl stateVar stdGenVar peerAddr = do
traceWith tracer (TrReleaseConnection Outbound peerAddr)
(transition, mbAssertion)
<- atomically $ do
state <- readTMVar stateVar
case Map.lookup peerAddr state of
-- if the connection errored, it will remove itself from the state.
-- Calling 'unregisterOutboundConnection' is a no-op in this case.
-- Calling 'releaseOutboundConnection' is a no-op in this case.
Nothing -> pure ( DemoteToColdLocalNoop Nothing UnknownConnectionSt
, Nothing)

Just MutableConnState { connVar } -> do
connState <- readTVar connVar
let st = abstractState (Known connState)
case connState of
-- In any of the following three states unregistering is not
-- supported. 'requestOutboundConnection' is a synchronous
-- In any of the following three states releaseing is not
-- supported. 'acquireOutboundConnection' is a synchronous
-- operation which returns only once the connection is
-- negotiated.
ReservedOutboundState ->
Expand Down Expand Up @@ -2240,7 +2240,7 @@ with args@Arguments {
if dataFlow == Duplex
then Nothing
else Just (TrUnexpectedlyFalseAssertion
(UnregisterOutboundConnection
(ReleaseOutboundConnection
(Just connId)
st)
)
Expand Down Expand Up @@ -2697,7 +2697,7 @@ withCallStack k = k callStack
--
data Trace peerAddr handlerTrace
= TrIncludeConnection Provenance peerAddr
| TrUnregisterConnection Provenance peerAddr
| TrReleaseConnection Provenance peerAddr
| TrConnect (Maybe peerAddr) -- ^ local address
peerAddr -- ^ remote address
| TrConnectError (Maybe peerAddr) -- ^ local address
Expand Down
Loading

0 comments on commit 1a7ff3e

Please sign in to comment.