Skip to content

Commit 0c3358c

Browse files
authored
Make TestBdxTransferServer::Shutdown() restore the default UnsolicitedMessageHandler (project-chip#41536)
* Save the SendInit handler, re-register it during shutdown, make it so Shutdown is actually called. * Moved implementation to Shutdown, simplifying * Restyled fixes * Init saves the handler so factory is not needed during shutdown * Style and comment fixes * Comment change
1 parent 1eb1102 commit 0c3358c

File tree

10 files changed

+106
-27
lines changed

10 files changed

+106
-27
lines changed

src/controller/python/matter/ChipStack.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,9 @@ def Shutdown(self):
193193
for subscription in tuple(self._subscriptions.values()):
194194
subscription.Shutdown()
195195

196+
# Shut down the BDX server.
197+
Bdx.Shutdown()
198+
196199
# Terminate Matter thread and shutdown the stack.
197200
self._ChipStackLib.pychip_DeviceController_StackShutdown()
198201

src/controller/python/matter/bdx/Bdx.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,16 @@ def Init():
248248
setter.Set('pychip_Bdx_InitCallbacks', None, [
249249
_OnTransferObtainedCallbackFunct, _OnFailedToObtainTransferCallbackFunct, _OnDataReceivedCallbackFunct,
250250
_OnTransferCompletedCallbackFunct])
251+
setter.Set('pychip_Bdx_Shutdown',
252+
None, [])
251253

252254
handle.pychip_Bdx_InitCallbacks(
253255
_OnTransferObtainedCallback, _OnFailedToObtainTransferCallback, _OnDataReceivedCallback, _OnTransferCompletedCallback)
256+
257+
258+
def Shutdown():
259+
'''
260+
Shut down the BDX server.
261+
'''
262+
handle = GetLibraryHandle()
263+
handle.pychip_Bdx_Shutdown()

src/controller/python/matter/bdx/bdx.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,12 @@ void pychip_Bdx_InitCallbacks(OnTransferObtainedCallback onTransferObtainedCallb
215215
gBdxTransferServer.Init(systemLayer, factory.GetSystemState()->ExchangeMgr());
216216
}
217217

218+
void pychip_Bdx_Shutdown()
219+
{
220+
// Shut down and clean up resources associated with the TestBdxTransferServer instance.
221+
gBdxTransferServer.Shutdown();
222+
}
223+
218224
// Prepares the BDX system to expect a new transfer.
219225
// If max_block_size != 0, it overrides the default cap for the next transfer only.
220226
PyChipError pychip_Bdx_ExpectBdxTransfer(PyObject transferObtainedContext, uint16_t max_block_size)

src/controller/python/matter/bdx/test-bdx-transfer-server.cpp

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,14 +36,23 @@ CHIP_ERROR TestBdxTransferServer::Init(System::Layer * systemLayer, Messaging::E
3636
mSystemLayer = systemLayer;
3737
mExchangeManager = exchangeManager;
3838
// This removes the BdxTransferServer registered as part of CHIPDeviceControllerFactory.
39-
mExchangeManager->UnregisterUnsolicitedMessageHandlerForType(MessageType::SendInit);
39+
// Store the handler that was previously registered for SendInit so we can restore it on shutdown.
40+
mExchangeManager->UnregisterUnsolicitedMessageHandlerForType(MessageType::SendInit, &mPrevSendInitHandler);
4041
return mExchangeManager->RegisterUnsolicitedMessageHandlerForProtocol(Protocols::BDX::Id, this);
4142
}
4243

4344
void TestBdxTransferServer::Shutdown()
4445
{
4546
VerifyOrReturn(mExchangeManager != nullptr);
4647
LogErrorOnFailure(mExchangeManager->UnregisterUnsolicitedMessageHandlerForProtocol(Protocols::BDX::Id));
48+
49+
// Re-register the BdxTransferServer that was registered as part of CHIPDeviceControllerFactory.
50+
// Otherwise when BdxTransferServer::Shutdown() attempts to unregister this, it causes an error.
51+
if (mPrevSendInitHandler != nullptr)
52+
{
53+
LogErrorOnFailure(mExchangeManager->RegisterUnsolicitedMessageHandlerForType(MessageType::SendInit, mPrevSendInitHandler));
54+
}
55+
4756
mExchangeManager = nullptr;
4857
}
4958

src/controller/python/matter/bdx/test-bdx-transfer-server.h

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,11 @@ class TestBdxTransferServer : public Messaging::UnsolicitedMessageHandler
5959
static constexpr size_t kTransferPoolSize = 2;
6060

6161
ObjectPool<BdxTransfer, kTransferPoolSize> mTransferPool;
62-
System::Layer * mSystemLayer = nullptr;
63-
Messaging::ExchangeManager * mExchangeManager = nullptr;
64-
BdxTransfer::Delegate * mBdxTransferDelegate = nullptr;
65-
size_t mExpectedTransfers = 0;
62+
System::Layer * mSystemLayer = nullptr;
63+
Messaging::ExchangeManager * mExchangeManager = nullptr;
64+
BdxTransfer::Delegate * mBdxTransferDelegate = nullptr;
65+
size_t mExpectedTransfers = 0;
66+
Messaging::UnsolicitedMessageHandler * mPrevSendInitHandler = nullptr;
6667
};
6768

6869
} // namespace bdx

src/messaging/ExchangeMgr.cpp

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -135,9 +135,10 @@ CHIP_ERROR ExchangeManager::UnregisterUnsolicitedMessageHandlerForProtocol(Proto
135135
return UnregisterUMH(protocolId, kAnyMessageType);
136136
}
137137

138-
CHIP_ERROR ExchangeManager::UnregisterUnsolicitedMessageHandlerForType(Protocols::Id protocolId, uint8_t msgType)
138+
CHIP_ERROR ExchangeManager::UnregisterUnsolicitedMessageHandlerForType(Protocols::Id protocolId, uint8_t msgType,
139+
Messaging::UnsolicitedMessageHandler ** outHandler)
139140
{
140-
return UnregisterUMH(protocolId, static_cast<int16_t>(msgType));
141+
return UnregisterUMH(protocolId, static_cast<int16_t>(msgType), outHandler);
141142
}
142143

143144
CHIP_ERROR ExchangeManager::RegisterUMH(Protocols::Id protocolId, int16_t msgType, UnsolicitedMessageHandler * handler)
@@ -170,18 +171,29 @@ CHIP_ERROR ExchangeManager::RegisterUMH(Protocols::Id protocolId, int16_t msgTyp
170171
return CHIP_NO_ERROR;
171172
}
172173

173-
CHIP_ERROR ExchangeManager::UnregisterUMH(Protocols::Id protocolId, int16_t msgType)
174+
CHIP_ERROR ExchangeManager::UnregisterUMH(Protocols::Id protocolId, int16_t msgType,
175+
Messaging::UnsolicitedMessageHandler ** outHandler)
174176
{
175177
for (auto & umh : UMHandlerPool)
176178
{
177179
if (umh.IsInUse() && umh.Matches(protocolId, msgType))
178180
{
181+
// Store the handler before unregistering.
182+
if (outHandler != nullptr)
183+
{
184+
*outHandler = umh.Handler;
185+
}
179186
umh.Reset();
180187
SYSTEM_STATS_DECREMENT(chip::System::Stats::kExchangeMgr_NumUMHandlers);
181188
return CHIP_NO_ERROR;
182189
}
183190
}
184191

192+
if (outHandler != nullptr)
193+
{
194+
*outHandler = nullptr;
195+
}
196+
185197
return CHIP_ERROR_NO_UNSOLICITED_MESSAGE_HANDLER;
186198
}
187199

src/messaging/ExchangeMgr.h

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -168,20 +168,26 @@ class DLL_EXPORT ExchangeManager : public SessionMessageDelegate
168168
*
169169
* @param[in] msgType The message type of the corresponding protocol.
170170
*
171+
* @param[out] outHandler If non-null, receives the handler that was unregistered. If no
172+
* handler matched, *outHandler is set to nullptr. Callers may pass
173+
* nullptr if they do not need this information.
174+
*
171175
* @retval #CHIP_ERROR_NO_UNSOLICITED_MESSAGE_HANDLER If the matching unsolicited message handler
172176
* is not found.
173177
* @retval #CHIP_NO_ERROR On success.
174178
*/
175-
CHIP_ERROR UnregisterUnsolicitedMessageHandlerForType(Protocols::Id protocolId, uint8_t msgType);
179+
CHIP_ERROR UnregisterUnsolicitedMessageHandlerForType(Protocols::Id protocolId, uint8_t msgType,
180+
Messaging::UnsolicitedMessageHandler ** outHandler = nullptr);
176181

177182
/**
178183
* A strongly-message-typed version of UnregisterUnsolicitedMessageHandlerForType.
179184
*/
180185
template <typename MessageType, typename = std::enable_if_t<std::is_enum<MessageType>::value>>
181-
CHIP_ERROR UnregisterUnsolicitedMessageHandlerForType(MessageType msgType)
186+
CHIP_ERROR UnregisterUnsolicitedMessageHandlerForType(MessageType msgType,
187+
Messaging::UnsolicitedMessageHandler ** outHandler = nullptr)
182188
{
183189
return UnregisterUnsolicitedMessageHandlerForType(Protocols::MessageTypeTraits<MessageType>::ProtocolId(),
184-
to_underlying(msgType));
190+
to_underlying(msgType), outHandler);
185191
}
186192

187193
/**
@@ -244,7 +250,8 @@ class DLL_EXPORT ExchangeManager : public SessionMessageDelegate
244250
UnsolicitedMessageHandlerSlot UMHandlerPool[CHIP_CONFIG_MAX_UNSOLICITED_MESSAGE_HANDLERS];
245251

246252
CHIP_ERROR RegisterUMH(Protocols::Id protocolId, int16_t msgType, UnsolicitedMessageHandler * handler);
247-
CHIP_ERROR UnregisterUMH(Protocols::Id protocolId, int16_t msgType);
253+
CHIP_ERROR UnregisterUMH(Protocols::Id protocolId, int16_t msgType,
254+
Messaging::UnsolicitedMessageHandler ** outHandler = nullptr);
248255

249256
void OnMessageReceived(const PacketHeader & packetHeader, const PayloadHeader & payloadHeader, const SessionHandle & session,
250257
DuplicateMessage isDuplicate, System::PacketBufferHandle && msgBuf) override;

src/messaging/tests/TestExchange.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,8 +151,11 @@ void TestExchange::DoRoundTripTest(MockExchangeDelegate & delegate1, MockExchang
151151
ec1->Close();
152152
ec2->Close();
153153

154-
err = GetExchangeManager().UnregisterUnsolicitedMessageHandlerForType(Protocols::SecureChannel::Id, kMsgType_TEST1);
154+
Messaging::UnsolicitedMessageHandler * removedHandler = nullptr;
155+
err = GetExchangeManager().UnregisterUnsolicitedMessageHandlerForType(Protocols::SecureChannel::Id, kMsgType_TEST1,
156+
&removedHandler);
155157
EXPECT_EQ(err, CHIP_NO_ERROR);
158+
EXPECT_EQ(removedHandler, &delegate2);
156159
}
157160

158161
TEST_F(TestExchange, CheckBasicMessageRoundTrip)

src/messaging/tests/TestExchangeMgr.cpp

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -149,8 +149,10 @@ TEST_F(TestExchangeMgr, CheckSessionExpirationBasics)
149149
EXPECT_FALSE(receiveDelegate.IsOnMessageReceivedCalled);
150150
ec1->Close();
151151

152-
err = GetExchangeManager().UnregisterUnsolicitedMessageHandlerForType(Protocols::BDX::Id, kMsgType_TEST1);
152+
Messaging::UnsolicitedMessageHandler * removedHandler = nullptr;
153+
err = GetExchangeManager().UnregisterUnsolicitedMessageHandlerForType(Protocols::BDX::Id, kMsgType_TEST1, &removedHandler);
153154
EXPECT_EQ(err, CHIP_NO_ERROR);
155+
EXPECT_EQ(removedHandler, &receiveDelegate);
154156

155157
// recreate closed session.
156158
EXPECT_EQ(CreateSessionAliceToBob(), CHIP_NO_ERROR);
@@ -219,11 +221,15 @@ TEST_F(TestExchangeMgr, CheckUmhRegistrationTest)
219221
err = GetExchangeManager().UnregisterUnsolicitedMessageHandlerForProtocol(Protocols::Echo::Id);
220222
EXPECT_NE(err, CHIP_NO_ERROR);
221223

222-
err = GetExchangeManager().UnregisterUnsolicitedMessageHandlerForType(Protocols::Echo::Id, kMsgType_TEST1);
224+
Messaging::UnsolicitedMessageHandler * removedHandler = nullptr;
225+
err = GetExchangeManager().UnregisterUnsolicitedMessageHandlerForType(Protocols::Echo::Id, kMsgType_TEST1, &removedHandler);
223226
EXPECT_EQ(err, CHIP_NO_ERROR);
227+
EXPECT_EQ(removedHandler, &mockAppDelegate);
224228

225-
err = GetExchangeManager().UnregisterUnsolicitedMessageHandlerForType(Protocols::Echo::Id, kMsgType_TEST2);
229+
removedHandler = nullptr;
230+
err = GetExchangeManager().UnregisterUnsolicitedMessageHandlerForType(Protocols::Echo::Id, kMsgType_TEST2, &removedHandler);
226231
EXPECT_NE(err, CHIP_NO_ERROR);
232+
EXPECT_EQ(removedHandler, nullptr);
227233
}
228234

229235
TEST_F(TestExchangeMgr, CheckExchangeMessages)
@@ -256,8 +262,10 @@ TEST_F(TestExchangeMgr, CheckExchangeMessages)
256262
DrainAndServiceIO();
257263
EXPECT_TRUE(mockUnsolicitedAppDelegate.IsOnMessageReceivedCalled);
258264

259-
err = GetExchangeManager().UnregisterUnsolicitedMessageHandlerForType(Protocols::BDX::Id, kMsgType_TEST1);
265+
Messaging::UnsolicitedMessageHandler * removedHandler = nullptr;
266+
err = GetExchangeManager().UnregisterUnsolicitedMessageHandlerForType(Protocols::BDX::Id, kMsgType_TEST1, &removedHandler);
260267
EXPECT_EQ(err, CHIP_NO_ERROR);
268+
EXPECT_EQ(removedHandler, &mockUnsolicitedAppDelegate);
261269
}
262270

263271
} // namespace

src/messaging/tests/TestReliableMessageProtocol.cpp

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -704,8 +704,10 @@ TEST_F(TestReliableMessageProtocol, CheckResendApplicationMessageWithPeerExchang
704704
EXPECT_EQ(rm->TestGetCountRetransTable(), 0);
705705
EXPECT_TRUE(mockReceiver.IsOnMessageReceivedCalled);
706706

707-
err = GetExchangeManager().UnregisterUnsolicitedMessageHandlerForType(Echo::MsgType::EchoRequest);
707+
Messaging::UnsolicitedMessageHandler * removedHandler = nullptr;
708+
err = GetExchangeManager().UnregisterUnsolicitedMessageHandlerForType(Echo::MsgType::EchoRequest, &removedHandler);
708709
EXPECT_EQ(err, CHIP_NO_ERROR);
710+
EXPECT_EQ(removedHandler, &mockReceiver);
709711
}
710712

711713
TEST_F(TestReliableMessageProtocol, CheckDuplicateMessageClosedExchange)
@@ -757,8 +759,10 @@ TEST_F(TestReliableMessageProtocol, CheckDuplicateMessageClosedExchange)
757759
// Let's not drop the duplicate message
758760
mockReceiver.SetDropAckResponse(false);
759761

760-
err = GetExchangeManager().UnregisterUnsolicitedMessageHandlerForType(Echo::MsgType::EchoRequest);
762+
Messaging::UnsolicitedMessageHandler * removedHandler = nullptr;
763+
err = GetExchangeManager().UnregisterUnsolicitedMessageHandlerForType(Echo::MsgType::EchoRequest, &removedHandler);
761764
EXPECT_EQ(err, CHIP_NO_ERROR);
765+
EXPECT_EQ(removedHandler, &mockReceiver);
762766

763767
// Wait for the first re-transmit and ack (should take 64ms)
764768
GetIOContext().DriveIOUntil(1000_ms32, [&] { return loopback.mSentMessageCount >= 3; });
@@ -849,8 +853,10 @@ TEST_F(TestReliableMessageProtocol, CheckDuplicateOldMessageClosedExchange)
849853
// Let's not drop the duplicate message's ack.
850854
mockReceiver.SetDropAckResponse(false);
851855

852-
err = GetExchangeManager().UnregisterUnsolicitedMessageHandlerForType(Echo::MsgType::EchoRequest);
856+
Messaging::UnsolicitedMessageHandler * removedHandler = nullptr;
857+
err = GetExchangeManager().UnregisterUnsolicitedMessageHandlerForType(Echo::MsgType::EchoRequest, &removedHandler);
853858
EXPECT_EQ(err, CHIP_NO_ERROR);
859+
EXPECT_EQ(removedHandler, &mockReceiver);
854860

855861
// Wait for the first re-transmit and ack (should take 64ms)
856862
rm->StartTimer();
@@ -914,8 +920,10 @@ TEST_F(TestReliableMessageProtocol, CheckResendSessionEstablishmentMessageWithPe
914920
EXPECT_EQ(rm->TestGetCountRetransTable(), 0);
915921
EXPECT_TRUE(mockReceiver.IsOnMessageReceivedCalled);
916922

917-
err = GetExchangeManager().UnregisterUnsolicitedMessageHandlerForType(Echo::MsgType::EchoRequest);
923+
Messaging::UnsolicitedMessageHandler * removedHandler = nullptr;
924+
err = GetExchangeManager().UnregisterUnsolicitedMessageHandlerForType(Echo::MsgType::EchoRequest, &removedHandler);
918925
EXPECT_EQ(err, CHIP_NO_ERROR);
926+
EXPECT_EQ(removedHandler, &mockReceiver);
919927
}
920928

921929
TEST_F(TestReliableMessageProtocol, CheckDuplicateMessage)
@@ -964,8 +972,10 @@ TEST_F(TestReliableMessageProtocol, CheckDuplicateMessage)
964972
EXPECT_EQ(loopback.mDroppedMessageCount, 0u);
965973
EXPECT_EQ(rm->TestGetCountRetransTable(), 1);
966974

967-
err = GetExchangeManager().UnregisterUnsolicitedMessageHandlerForType(Echo::MsgType::EchoRequest);
975+
Messaging::UnsolicitedMessageHandler * removedHandler = nullptr;
976+
err = GetExchangeManager().UnregisterUnsolicitedMessageHandlerForType(Echo::MsgType::EchoRequest, &removedHandler);
968977
EXPECT_EQ(err, CHIP_NO_ERROR);
978+
EXPECT_EQ(removedHandler, &mockReceiver);
969979

970980
// Let's not drop the duplicate message
971981
mockReceiver.SetDropAckResponse(false);
@@ -1062,8 +1072,10 @@ TEST_F(TestReliableMessageProtocol, CheckReceiveAfterStandaloneAck)
10621072
// Ensure that we have received that response.
10631073
EXPECT_TRUE(mockSender.IsOnMessageReceivedCalled);
10641074

1065-
err = GetExchangeManager().UnregisterUnsolicitedMessageHandlerForType(Echo::MsgType::EchoRequest);
1075+
Messaging::UnsolicitedMessageHandler * removedHandler = nullptr;
1076+
err = GetExchangeManager().UnregisterUnsolicitedMessageHandlerForType(Echo::MsgType::EchoRequest, &removedHandler);
10661077
EXPECT_EQ(err, CHIP_NO_ERROR);
1078+
EXPECT_EQ(removedHandler, &mockReceiver);
10671079

10681080
EXPECT_EQ(rm->TestGetCountRetransTable(), 0);
10691081
}
@@ -1187,8 +1199,10 @@ TEST_F(TestReliableMessageProtocol, CheckPiggybackAfterPiggyback)
11871199
EXPECT_TRUE(mockSender.IsOnMessageReceivedCalled);
11881200
EXPECT_TRUE(mockSender.mReceivedPiggybackAck);
11891201

1190-
err = GetExchangeManager().UnregisterUnsolicitedMessageHandlerForType(Echo::MsgType::EchoRequest);
1202+
Messaging::UnsolicitedMessageHandler * removedHandler = nullptr;
1203+
err = GetExchangeManager().UnregisterUnsolicitedMessageHandlerForType(Echo::MsgType::EchoRequest, &removedHandler);
11911204
EXPECT_EQ(err, CHIP_NO_ERROR);
1205+
EXPECT_EQ(removedHandler, &mockReceiver);
11921206

11931207
EXPECT_EQ(rm->TestGetCountRetransTable(), 0);
11941208
}
@@ -1371,8 +1385,10 @@ TEST_F(TestReliableMessageProtocol, CheckMessageAfterClosed)
13711385
// immediately on the receiver, due to the exchange being closed.
13721386
EXPECT_EQ(rm->TestGetCountRetransTable(), 0);
13731387

1374-
err = GetExchangeManager().UnregisterUnsolicitedMessageHandlerForType(Echo::MsgType::EchoRequest);
1388+
Messaging::UnsolicitedMessageHandler * removedHandler = nullptr;
1389+
err = GetExchangeManager().UnregisterUnsolicitedMessageHandlerForType(Echo::MsgType::EchoRequest, &removedHandler);
13751390
EXPECT_EQ(err, CHIP_NO_ERROR);
1391+
EXPECT_EQ(removedHandler, &mockReceiver);
13761392

13771393
EXPECT_EQ(rm->TestGetCountRetransTable(), 0);
13781394
}
@@ -1926,8 +1942,10 @@ TEST_F(TestReliableMessageProtocol, CheckApplicationResponseDelayed)
19261942
err = CreateSessionAliceToBob();
19271943
EXPECT_EQ(err, CHIP_NO_ERROR);
19281944

1929-
err = GetExchangeManager().UnregisterUnsolicitedMessageHandlerForType(Echo::MsgType::EchoRequest);
1945+
Messaging::UnsolicitedMessageHandler * removedHandler = nullptr;
1946+
err = GetExchangeManager().UnregisterUnsolicitedMessageHandlerForType(Echo::MsgType::EchoRequest, &removedHandler);
19301947
EXPECT_EQ(err, CHIP_NO_ERROR);
1948+
EXPECT_EQ(removedHandler, &mockReceiver);
19311949
}
19321950

19331951
TEST_F(TestReliableMessageProtocol, CheckApplicationResponseNeverComes)
@@ -2048,8 +2066,10 @@ TEST_F(TestReliableMessageProtocol, CheckApplicationResponseNeverComes)
20482066
err = CreateSessionAliceToBob();
20492067
EXPECT_EQ(err, CHIP_NO_ERROR);
20502068

2051-
err = GetExchangeManager().UnregisterUnsolicitedMessageHandlerForType(Echo::MsgType::EchoRequest);
2069+
Messaging::UnsolicitedMessageHandler * removedHandler = nullptr;
2070+
err = GetExchangeManager().UnregisterUnsolicitedMessageHandlerForType(Echo::MsgType::EchoRequest, &removedHandler);
20522071
EXPECT_EQ(err, CHIP_NO_ERROR);
2072+
EXPECT_EQ(removedHandler, &mockReceiver);
20532073
}
20542074

20552075
#if CHIP_CONFIG_MRP_ANALYTICS_ENABLED

0 commit comments

Comments
 (0)