Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shut down InteractionModelEngine during DeviceController shutdown. #7629

Merged

Conversation

bzbarsky-apple
Copy link
Contributor

Otherwise if we have an IM action in flight at shutdown time we will
effectively leak the object managing it, and after a few
shutdown/restart cycles for the stack not be able to send any more IM
actions.

Problem

If I shut down and restart DeviceController a few times, and the shutdowns happen to come while there is a CommandSender outstanding (in that we sent a command and a response has not come back), then eventually we run out of CommandSenders.

Change overview

Make sure to shut down CommandSenders when we shut down DeviceController.

Testing

Manual testing with a bug added to chip-all-clusters-app that causes it to shut down the DeviceController while CommandSenders are life. Will try to add some automated tests too.

Otherwise if we have an IM action in flight at shutdown time we will
effectively leak the object managing it, and after a few
shutdown/restart cycles for the stack not be able to send any more IM
actions.
@woody-apple
Copy link
Contributor

@woody-apple woody-apple merged commit 9f9907b into project-chip:master Jun 15, 2021
@bzbarsky-apple bzbarsky-apple deleted the shutdown-im-in-controller branch June 15, 2021 21:49
nikita-s-wrk pushed a commit to nikita-s-wrk/connectedhomeip that referenced this pull request Sep 23, 2021
…roject-chip#7629)

Otherwise if we have an IM action in flight at shutdown time we will
effectively leak the object managing it, and after a few
shutdown/restart cycles for the stack not be able to send any more IM
actions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants