project-chip / connectedhomeip

Matter (formerly Project CHIP) creates more connections between more objects, simplifying development for manufacturers and increasing compatibility for consumers, guided by the Connectivity Standards Alliance.
https://buildwithmatter.com
Apache License 2.0
7.31k stars 1.95k forks source link

Darwin crashes due to releasing CommandHandler::Handle on the wrong thread #25129

Open bzbarsky-apple opened 1 year ago

bzbarsky-apple commented 1 year ago

I am seeing crashes with stacks like so:

0   libsystem_kernel.dylib                 0x180fa882c __pthread_kill + 8
1   libsystem_pthread.dylib                0x181066150 pthread_kill + 268 (
2   libsystem_c.dylib                      0x180f1bc5c abort + 176 
3   Matter                                 0x113a5df30 chipAbort
4   Matter                                 0x113b2e1e0 chipDie
5   Matter                                 0x113b2e0c4 chip::Platform::Internal::AssertChipStackLockedByCurrentThread(char const*, int)
6   Matter                                 0x113b4e35c chip::Transport::Session::RemoveHolder(chip::SessionHolder&)
7   Matter                                 0x113b4d824 chip::SessionHolder::Release()
8   Matter                                 0x113b4d7b8 chip::SessionHolder::~SessionHolder()
9   Matter                                 0x113a66cc4 chip::SessionHolderWithDelegate::~SessionHolderWithDelegate()
10  Matter                                 0x113b24794 chip::Messaging::ExchangeContext::ExchangeSessionHolder::~ExchangeSessionHolder()
11  Matter                                 0x113b230e0 chip::Messaging::ExchangeContext::ExchangeSessionHolder::~ExchangeSessionHolder()
12  Matter                                 0x113b23084 chip::Messaging::ExchangeContext::~ExchangeContext()
13  Matter                                 0x113b23124 chip::Messaging::ExchangeContext::~ExchangeContext()
14  Matter                                 0x113b246d0 void chip::Platform::Delete<chip::Messaging::ExchangeContext>(chip::Messaging::ExchangeContext*)
15  Matter                                 0x113b24600 chip::HeapObjectPool<chip::Messaging::ExchangeContext>::ReleaseObject(chip::Messaging::ExchangeContext*)
16  Matter                                 0x113b22848 chip::Messaging::ExchangeManager::ReleaseContext(chip::Messaging::ExchangeContext*)
17  Matter                                 0x113b22814 chip::Messaging::ExchangeContextDeletor::Release(chip::Messaging::ExchangeContext*)
18  Matter                                 0x113b22608 chip::ReferenceCounted<chip::Messaging::ExchangeContext, chip::Messaging::ExchangeContextDeletor, 1, unsigned int>::Release()
19  Matter                                 0x113b227c0 chip::Messaging::ExchangeContext::Abort()
20  Matter                                 0x1128e603c chip::Messaging::ExchangeHolder::Release()
21  Matter                                 0x1128e5fa8 chip::Messaging::ExchangeHolder::~ExchangeHolder()
22  Matter                                 0x1128e5794 chip::Messaging::ExchangeHolder::~ExchangeHolder()
23  Matter                                 0x1139edc08 chip::app::CommandHandler::~CommandHandler()
24  Matter                                 0x1139ed7d4 chip::app::CommandHandler::~CommandHandler()
25  Matter                                 0x113a008a8 void chip::Platform::Delete<chip::app::CommandHandler>(chip::app::CommandHandler*)
26  Matter                                 0x1139f7034 chip::HeapObjectPool<chip::app::CommandHandler>::ReleaseObject(chip::app::CommandHandler*)
27  Matter                                 0x1139f6ea0 chip::app::InteractionModelEngine::OnDone(chip::app::CommandHandler&)
28  Matter                                 0x1139ebfe0 chip::app::CommandHandler::Close()
29  Matter                                 0x1139ec490 chip::app::CommandHandler::DecrementHoldOff()
30  Matter                                 0x1139ed63c chip::app::CommandHandler::Handle::Release()
31  Matter                                 0x11288dfe4 chip::app::CommandHandler::Handle::~Handle()
32  Matter                                 0x112885430 chip::app::CommandHandler::Handle::~Handle()
33  Matter                                 0x112885404 __Block_byref_object_dispose_
34  libsystem_blocks.dylib                 0x180ea60b0 _Block_object_dispose + 256 
35  libsystem_blocks.dylib                 0x180ea6eb8 bool HelperBase<GenericInline>::disposeCapture<(HelperBase<GenericInline>::BlockCaptureKind)5>(unsigned int, unsigned char*) + 72 
bzbarsky-apple commented 1 year ago

So if we get into this situation then:

  1. We have a CommandHandler::Handle that has been captured by a block.
  2. We never called Release() on the handle.
  3. The block is being destroyed.

Looking at our uses of CommandHandler::Handle, we have three:

@ksperling-apple @nivi-apple anything I am missing here?

bzbarsky-apple commented 1 year ago

I guess for the "delegate never calls completionHandler" case we could have an intermediate object that actually owns the handle and holds a ref to the controller and whose destructor tries to queue the handle release over to the controller....

nivi-apple commented 1 year ago

Took a quick look. Looks like you covered all the use cases for MTROTAProviderDelegateBridge

woody-apple commented 1 year ago

@ksperling-apple ?