Skip to content

Commit 1ddaee7

Browse files
committed
test: cover session var leak on interrupted connect
UtilTests: tryAllErrors rethrows ThreadKilled/StackOverflow (the mechanism that skips putTMVar). SMPProxyTests: agent client reconnection after a cancelled connect, plus a control proving the stalling relay alone does not cause the failure; refine the relay reconnection tests.
1 parent 3ec840e commit 1ddaee7

2 files changed

Lines changed: 38 additions & 0 deletions

File tree

tests/CoreTests/UtilTests.hs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,13 @@ utilTests = do
5050
runExceptT (tryAllErrors throwTestException) `shouldReturn` Right (Left (TestException "user error (error)"))
5151
it "should return no errors as Right" $
5252
runExceptT (tryAllErrors noErrors) `shouldReturn` Right (Right "no errors")
53+
-- tryAllErrors rethrows asynchronous exceptions (it uses UnliftIO.catch). Any recovery placed
54+
-- after `tryAllErrors action` - e.g. putTMVar to fill a SessionVar - is therefore SKIPPED when
55+
-- the thread is killed mid-action. Unlike tryAllOwnErrors, it also rethrows the overflow exceptions.
56+
it "should rethrow ThreadKilled" $
57+
runExceptT (tryAllErrors $ throwAsync ThreadKilled) `shouldThrow` (\e -> e == ThreadKilled)
58+
it "should rethrow StackOverflow" $
59+
runExceptT (tryAllErrors $ throwAsync StackOverflow) `shouldThrow` (\e -> e == StackOverflow)
5360
describe "catchAllErrors" $ do
5461
it "should catch ExceptT error" $
5562
runExceptT (throwTestError `catchAllErrors` handleCatch) `shouldReturn` Right "caught TestError \"error\""

tests/SMPProxyTests.hs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,9 @@ smpProxyTests = do
6363
testProxyRecoversWithoutDisconnect
6464
it "reconnects to relay after sender disconnects mid-connection" $ \_ ->
6565
testProxyReconnectAfterRelayRestart
66+
describe "agent client reconnection" $ do
67+
it "reconnects after a connect is cancelled mid-flight" $ \_ ->
68+
testAgentClientReconnectAfterCancel
6669
describe "proxy requests" $ do
6770
describe "bad relay URIs" $ do
6871
xit "host not resolved" todo
@@ -507,6 +510,34 @@ testProxyReconnectAfterRelayRestart =
507510
race_ (threadDelay 1000000) requestRelaySession
508511
requireProxyReconnect
509512

513+
-- Bug B (same root cause as the proxy, in the messaging agent): getSMPServerClient inserts an
514+
-- empty SessionVar into smpClients, then connects inside newProtocolClient's tryAllErrors, which
515+
-- rethrows async exceptions. If the connecting thread is cancelled mid-connect, putTMVar is
516+
-- skipped and the empty var is left in smpClients, so every later connection to that server times
517+
-- out on it. Phase 1 cancels a connect to a stalling relay; phase 2 requires a fresh connect to a
518+
-- healthy relay to succeed.
519+
testAgentClientReconnectAfterCancel :: IO ()
520+
testAgentClientReconnectAfterCancel =
521+
withAgent 1 agentCfg agentServersLeak testDB $ \a -> do
522+
withStallingServerOn testPort2 $ do
523+
t <- async $ runExceptT $ A.createConnection a NRMInteractive 1 True True SCMInvitation Nothing Nothing CR.IKPQOn SMSubscribe
524+
threadDelay 1000000 -- let the connect to the stalling relay start, then kill it mid-flight
525+
cancel t
526+
withSmpServerConfigOn (transport @TLS) cfgJ2 testPort2 $ \_ -> do
527+
testSMPClient_ "127.0.0.1" testPort2 proxyVRangeV8 Nothing $ \(th :: THandleSMP TLS 'TClient) -> do
528+
(_, _, reply) <- sendRecv th (Nothing, "0", NoEntity, SMP.PING)
529+
reply `shouldBe` Right SMP.PONG -- the relay is up and reachable, so a timeout can only be the poisoned var
530+
r <- timeout 8000000 $ runExceptT $ A.createConnection a NRMInteractive 1 True True SCMInvitation Nothing Nothing CR.IKPQOn SMSubscribe
531+
case r of
532+
Just (Right _) -> pure ()
533+
_ -> expectationFailure $ "agent failed to connect after a cancelled connect; got: " <> show r
534+
where
535+
agentServersLeak =
536+
initAgentServers
537+
{ smp = userServers [testSMPServer2],
538+
netCfg = (netCfg initAgentServers) {tcpConnectTimeout = NetworkTimeout 4000000 4000000}
539+
}
540+
510541
todo :: AStoreType -> IO ()
511542
todo _ = fail "TODO"
512543

0 commit comments

Comments
 (0)