Fix the matter server shutdown flow and add DataModelShutdown #38515
+192
−34
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jadhavrohit924
force-pushed
the
fix/server/shutdown
branch
from
a4d73d5 to
4eddd6e
Compare
bzbarsky-apple
approved these changes
Apr 24, 2025
shubhamdp
approved these changes
Apr 24, 2025
jadhavrohit924
force-pushed
the
fix/server/shutdown
branch
from
59e4055 to
7028444
Compare
|
PR #38515: Size comparison from 37948e2 to 6df1418 Increases above 0.2%:
Full report (24 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, qpg, stm32, telink)
|
|
PR #38515: Size comparison from 37948e2 to 720ea1c Increases above 0.2%:
Full report (55 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
jadhavrohit924
force-pushed
the
fix/server/shutdown
branch
from
720ea1c to
1bcb1ea
Compare
|
PR #38515: Size comparison from 36c5265 to 1bcb1ea Increases above 0.2%:
Full report (75 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
jadhavrohit924
force-pushed
the
fix/server/shutdown
branch
from
1bcb1ea to
291897f
Compare
|
PR #38515: Size comparison from 4e8a75c to 291897f Increases above 0.2%:
Full report (75 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
jadhavrohit924
force-pushed
the
fix/server/shutdown
branch
from
291897f to
7e0794c
Compare
jadhavrohit924
force-pushed
the
fix/server/shutdown
branch
2 times, most recently
from
b5a34f1 to
22b18df
Compare
|
PR #38515: Size comparison from 7460361 to 22b18df Increases above 0.2%:
Full report (75 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
PR #38515: Size comparison from 79faaa3 to 245ad43 Increases above 0.2%:
Full report (75 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
jadhavrohit924
force-pushed
the
fix/server/shutdown
branch
from
245ad43 to
d05639f
Compare
|
PR #38515: Size comparison from 53ed908 to d05639f Increases above 0.2%:
Full report (73 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
jadhavrohit924
force-pushed
the
fix/server/shutdown
branch
2 times, most recently
from
c92a17e to
03f5e5d
Compare
|
PR #38515: Size comparison from feba8fd to 03f5e5d Increases above 0.2%:
Full report (75 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
jadhavrohit924
force-pushed
the
fix/server/shutdown
branch
from
03f5e5d to
547fbb8
Compare
|
PR #38515: Size comparison from 576b957 to 547fbb8 Increases above 0.2%:
Full report (75 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
jadhavrohit924
force-pushed
the
fix/server/shutdown
branch
from
547fbb8 to
00c52de
Compare
|
PR #38515: Size comparison from 7c1536e to 00c52de Increases above 0.2%:
Full report (75 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
jadhavrohit924
commented
May 21, 2025
bzbarsky-apple
requested changes
May 22, 2025
src/app/server/Server.cpp
Outdated
| @@ -651,6 +651,9 @@ void Server::Shutdown() | |||
| #if CHIP_CONFIG_ENABLE_ICD_SERVER | |||
| app::DnssdServer::Instance().SetICDManager(nullptr); | |||
| #endif // CHIP_CONFIG_ENABLE_ICD_SERVER | |||
|
|
|||
| app::InteractionModelEngine::GetInstance()->Shutdown(); | |||
There was a problem hiding this comment.
I would expect shutdown to just happen in reverse order from init, though this code seems to be doing a terrible job of that.
But just in terms of safety, why are we moving where IM engine shutdown happens? Why didn't we just add the new call sometime after that point?
There was a problem hiding this comment.
Revered the shutdown sequence and use an existing API to ShutdownActiveReads which was guarded by CONFIG_BUILD_FOR_HOST_UNIT_TEST
jadhavrohit924
force-pushed
the
fix/server/shutdown
branch
from
00c52de to
82ef723
Compare
|
PR #38515: Size comparison from a7cf007 to 82ef723 Increases above 0.2%:
Full report (3 builds for cc32xx, stm32)
|
Co-authored-by: Boris Zbarsky <[email protected]>
server shutdown sequence.
jadhavrohit924
force-pushed
the
fix/server/shutdown
branch
from
82ef723 to
a1c6f42
Compare
|
PR #38515: Size comparison from d761e1b to a1c6f42 Increases above 0.2%:
Full report (41 builds for cc32xx, cyw30739, efr32, linux, nrfconnect, psoc6, qpg, stm32, telink, tizen)
|
|
PR #38515: Size comparison from d761e1b to 22eaff4 Increases above 0.2%:
Full report (41 builds for cc32xx, cyw30739, efr32, linux, nrfconnect, psoc6, qpg, stm32, telink, tizen)
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
Changes
ShutdownDataModelForTestingwhich consumesMatter*PluginServerShutdownCallbackandemberAf*ClusterShutdownCallbackto shutdown the clusters initializations.Testing
Server::ShutdownAPI by calling it from application.