Skip to content

Commit 3338248

Browse files
pidarpedcecille
andauthored
[ Cherry pick from master ] 2 TCP Bug fixes (project-chip#37327)
* TCP tests: TC-SC-8.x - Use ArmFailsafe as cmd (project-chip#37313) * TCP tests: TC-SC-8.x - Use ArmFailsafe as cmd Also add top-level pics * Fix payload capability * Fix for Bug project-chip#36732 (project-chip#36879) Set the app_state callback object in the Connection state to null when the CASE session object is being cleared, on top of setting the inner callback methods to null. This prevents the callback object from being accessed later, when the connection is getting closed(after the CASE session has been set up and the session object no longer exists). * Fix for Bug project-chip#36731. (project-chip#36880) Add CloseActiveConnections() call in TCPBase::Close(), which is called as part of Server::Shutdown(). Active connections should be closed as part of Server shutdown. This allows the TCPConnectionState to also close the associated TCPEndpoint object as part of this shutdown flow. Previously, the CloseActiveConnections() call was present in the TCPBase destructor alone. Add test for Connection Close() and checking for TCPEndPoint. --------- Co-authored-by: C Freeman <cecille@google.com>
1 parent fc00c97 commit 3338248

File tree

4 files changed

+45
-14
lines changed

4 files changed

+45
-14
lines changed

src/protocols/secure_channel/CASESession.cpp

+13-5
Original file line numberDiff line numberDiff line change
@@ -428,12 +428,20 @@ void CASESession::Clear()
428428
mTCPConnCbCtxt.connClosedCb = nullptr;
429429
mTCPConnCbCtxt.connReceivedCb = nullptr;
430430

431-
if (mPeerConnState && mPeerConnState->mConnectionState != Transport::TCPState::kConnected)
431+
if (mPeerConnState)
432432
{
433-
// Abort the connection if the CASESession is being destroyed and the
434-
// connection is in the middle of being set up.
435-
mSessionManager->TCPDisconnect(mPeerConnState, /* shouldAbort = */ true);
436-
mPeerConnState = nullptr;
433+
// Set the app state callback object in the Connection state to null
434+
// to prevent any dangling pointer to memory(mTCPConnCbCtxt) owned
435+
// by the CASESession object, that is now getting cleared.
436+
mPeerConnState->mAppState = nullptr;
437+
438+
if (mPeerConnState->mConnectionState != Transport::TCPState::kConnected)
439+
{
440+
// Abort the connection if the CASESession is being destroyed and the
441+
// connection is in the middle of being set up.
442+
mSessionManager->TCPDisconnect(mPeerConnState, /* shouldAbort = */ true);
443+
mPeerConnState = nullptr;
444+
}
437445
}
438446
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
439447
}

src/transport/raw/ActiveTCPConnectionState.h

+4-1
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,10 @@ struct ActiveTCPConnectionState
6262

6363
void Free()
6464
{
65-
mEndPoint->Free();
65+
if (mEndPoint)
66+
{
67+
mEndPoint->Free();
68+
}
6669
mPeerAddr = PeerAddress::Uninitialized();
6770
mEndPoint = nullptr;
6871
mReceived = nullptr;

src/transport/raw/TCP.cpp

+5-8
Original file line numberDiff line numberDiff line change
@@ -55,14 +55,8 @@ constexpr int kListenBacklogSize = 2;
5555

5656
TCPBase::~TCPBase()
5757
{
58-
if (mListenSocket != nullptr)
59-
{
60-
// endpoint is only non null if it is initialized and listening
61-
mListenSocket->Free();
62-
mListenSocket = nullptr;
63-
}
64-
65-
CloseActiveConnections();
58+
// Call Close to free the listening socket and close all active connections.
59+
Close();
6660
}
6761

6862
void TCPBase::CloseActiveConnections()
@@ -125,6 +119,9 @@ void TCPBase::Close()
125119
mListenSocket->Free();
126120
mListenSocket = nullptr;
127121
}
122+
123+
CloseActiveConnections();
124+
128125
mState = TCPState::kNotReady;
129126
}
130127

src/transport/raw/tests/TestTCP.cpp

+23
Original file line numberDiff line numberDiff line change
@@ -609,6 +609,29 @@ TEST_F(TestTCP, HandleConnCloseCalledTest6)
609609
HandleConnCloseTest(addr);
610610
}
611611

612+
TEST_F(TestTCP, CheckTCPEndpointAfterCloseTest)
613+
{
614+
TCPImpl tcp;
615+
616+
IPAddress addr;
617+
IPAddress::FromString("::1", addr);
618+
619+
MockTransportMgrDelegate gMockTransportMgrDelegate(mIOContext);
620+
gMockTransportMgrDelegate.InitializeMessageTest(tcp, addr);
621+
gMockTransportMgrDelegate.ConnectTest(tcp, addr);
622+
623+
Transport::PeerAddress lPeerAddress = Transport::PeerAddress::TCP(addr, gChipTCPPort);
624+
void * state = TestAccess::FindActiveConnection(tcp, lPeerAddress);
625+
ASSERT_NE(state, nullptr);
626+
TCPEndPoint * lEndPoint = TestAccess::GetEndpoint(state);
627+
ASSERT_NE(lEndPoint, nullptr);
628+
629+
// Call Close and check the TCPEndpoint
630+
tcp.Close();
631+
lEndPoint = TestAccess::GetEndpoint(state);
632+
ASSERT_EQ(lEndPoint, nullptr);
633+
}
634+
612635
TEST_F(TestTCP, CheckProcessReceivedBuffer)
613636
{
614637
TCPImpl tcp;

0 commit comments

Comments
 (0)