Introduce a Shutdown method to DeviceControllerFactory #14723
Merged
Conversation
This file contains 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
|
PR #14723: Size comparison from 22f5d22 to caa01ed Increases (3 builds for linux)
Full report (43 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
|
bzbarsky-apple
approved these changes
Feb 3, 2022
|
@kianooshkarami please fix "restyled" bits so this will request reviews... |
Damian-Nordic
approved these changes
Feb 3, 2022
The new method would the symmetrical API for DeviceControllerFactory's
already existing `Init()` API.
What is the root of the problem?
The problem we intend to solve is that we'd like to use `Shutdown()` method
to shut down and clean up any memory/functionality enabled by `Init()` as
opposed to waiting for the DeviceControllerFactory desctructor, which only
happens once the program shuts down due to the nature of how
DeviceControllerFactory is instantiated.
DeviceControllerFactory can only be instantiated through ::GetInstance(),
this method constructs DeviceControllerFactory only once for the life of
the program (the constructor is a private method). Once the Init() is
invoked, the `mSystemState` is allocated as shown in the following:
```
mSystemState = chip::Platform::New<DeviceControllerSystemState>(stateParams);
```
However, in DeviceControllerFactory's destructor, which currently is only
invoked once the program shuts down, the deallocation of `mSystemState`
is requested via `chip::Platform::Delete`. This can be problematic since
the application will need to ensure `chip::Platform` memory stays initailized
throughout the memory shutdown.
Adding a shutdown methods allows the applications to have a symmetry between
the initialization and shutdown sequence related to the
DeviceControllerFactory
kianooshkarami
force-pushed
the
add-shutdown-to-dev-ctrl-factory
branch
from
caa01ed to
85a56f9
Compare
pullapprove
bot
requested review from
andy31415,
Byungjoo-Lee,
cecille,
chrisdecenzo,
chshu,
chulspro,
dhrishi,
electrocucaracha,
gjc13,
hawk248,
harsha-rajendran,
jelderton,
jepenven-silabs,
jmartinez-silabs,
kghost,
mlepage-google,
mrjerryjohns,
msandstedt and
mspang
pullapprove
bot
requested review from
gjc13,
hawk248,
yunhanw-google,
harsha-rajendran,
jelderton,
jepenven-silabs,
jmartinez-silabs,
kghost,
mlepage-google,
mrjerryjohns,
msandstedt,
mspang,
robszewczyk,
saurabhst,
selissia,
tcarmelveilleux,
tecimovic,
turon,
vijs,
wbschiller and
xylophone21
@bzbarsky-apple Thanks. Done, also changed the method to a void return. |
saurabhst
approved these changes
Feb 3, 2022
|
PR #14723: Size comparison from 1ee9366 to 85a56f9 Increases (3 builds for linux)
Full report (31 builds for cyw30739, efr32, k32w, linux, mbed, p6, qpg, telink)
|
msandstedt
approved these changes
Feb 3, 2022
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.
Change overview
The new method would be the symmetrical API for DeviceControllerFactory's
already existing
Init()API.What is the root of the problem?
The problem we intend to solve is that we'd like to use
Shutdown()methodto shut down and clean up any memory/functionality enabled by
Init()asopposed to waiting for the DeviceControllerFactory destructor, which only
happens once the program shuts down due to the nature of how
DeviceControllerFactory is instantiated.
DeviceControllerFactory can only be instantiated through ::GetInstance(),
this method constructs DeviceControllerFactory only once for the life of
the program (the constructor is a private method). Once the Init() is
invoked, the
mSystemStateis allocated as shown in the following:However, in DeviceControllerFactory's destructor, which currently is only
invoked once the program shuts down, the deallocation of
mSystemStateis requested via
chip::Platform::Delete. This can be problematic sincethe application will need to ensure
chip::Platformmemory stays initializedthroughout the memory shutdown.
Adding a shutdown method allows the applications to have symmetry between
the initialization and shutdown sequence related to the
DeviceControllerFactory
Problem
What is being fixed?
Potentially Memory crashes when freeing
mSystemStatewhen the process shuts downTesting
How was this tested? (at least one bullet point required)
I could use some assistance if there are already existing unit tests that would help with the validation