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

racing in chip_im_initiator, exchange context leak #6915

Closed
kghost opened this issue May 18, 2021 · 1 comment
Closed

racing in chip_im_initiator, exchange context leak #6915

kghost opened this issue May 18, 2021 · 1 comment
Assignees
Labels
leak Memory leak bug memory V1.0

Comments

@kghost
Copy link
Contributor

kghost commented May 18, 2021

Problem

Cirque fail: https://github.com/project-chip/connectedhomeip/actions/runs/844261919

There is a racing in chip_im_initiator tests. sometime ExchangeContext is leaked

chip-im-initiator: ../../../../../src/messaging/ExchangeMgr.cpp:101: chip::Messaging::ExchangeManager::Shutdown()::<lambda(auto:2*)> [with auto:2 = chip::Messaging::ExchangeContext]: Assertion `false' failed.

Apply following patch will make it fail everytime.

diff --git a/src/app/tests/integration/chip_im_initiator.cpp b/src/app/tests/integration/chip_im_initiator.cpp
index 09bceacc..9e971132 100644
--- a/src/app/tests/integration/chip_im_initiator.cpp
+++ b/src/app/tests/integration/chip_im_initiator.cpp
@@ -191,6 +191,7 @@ void HandleReadComplete()
            static_cast<double>(gReadRespCount) * 100 / gReadCount, static_cast<double>(transitTime) / 1000);
 
     gCond.notify_one();
+    usleep(20);
 }
 
 class MockInteractionModelApp : public chip::app::InteractionModelDelegate

There are 2 threads:

Event thread (Thread E):

  1. send read request
  2. receive read response
  3. notify main thread read successful
  4. delete ExchangeContext

Main thread (Thread M):

  1. Wait for read successful
  2. Shutdown ExchangeManager

if Shutting down the ExchangeManager happens before deleting the ExchangeContext, there will be an assert fail

Proposed Solution

Lock guard ExchangeManager::Shutdown

@woody-apple
Copy link
Contributor

Is this still an issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
leak Memory leak bug memory V1.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants