Skip to content

Commit

Permalink
Un-revert #22932 + Fix Python Storage lifetime
Browse files Browse the repository at this point in the history
22932 shuts down the fabric table as part of shutting down the stack.
This entails interactions with the persistent storage layer
to update some keys. In Python however, the storage layer has already
been shutdown by the time we get to stack shutdown, causing a
use-after-free.

Flip the order in which we shutdown the storage adapter relative to
shutting down the rest of the stack.
  • Loading branch information
mrjerryjohns committed Sep 29, 2022
1 parent 852c879 commit 5575bf2
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 10 deletions.
1 change: 1 addition & 0 deletions src/controller/CHIPDeviceControllerFactory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,7 @@ void DeviceControllerSystemState::Shutdown()

if (mTempFabricTable != nullptr)
{
mTempFabricTable->Shutdown();
chip::Platform::Delete(mTempFabricTable);
mTempFabricTable = nullptr;
// if we created a temp fabric table, then mFabrics points to it.
Expand Down
25 changes: 15 additions & 10 deletions src/controller/python/chip/storage/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ class PersistentStorage:
This interfaces with a C++ adapter that implements the PersistentStorageDelegate interface
and can be passed into C++ logic that needs an instance of that interface.
Object must be resident before the Matter stack starts up and last past its shutdown.
'''
@classmethod
def logger(cls):
Expand Down Expand Up @@ -241,15 +243,21 @@ def DeleteSdkKey(self, key: str):

def Shutdown(self):
''' Shuts down the object by free'ing up the associated adapter instance.
You cannot interact with this object there-after.
This should only be called after the CHIP stack has shutdown (i.e
after calling pychip_DeviceController_StackShutdown()).
'''
self._handle.pychip_Storage_ShutdownAdapter.argtypes = [c_void_p]
builtins.chipStack.Call(
lambda: self._handle.pychip_Storage_ShutdownAdapter(self._closure)
)
if (self._isActive):
self._handle.pychip_Storage_ShutdownAdapter.argtypes = [c_void_p]

self._isActive = False
#
# Since the stack is not running at this point, we can safely call
# C++ method directly on the current execution context without worrying
# about race conditions.
#
self._handle.pychip_Storage_ShutdownAdapter(self._closure)
self._isActive = False

@property
def jsonData(self) -> Dict:
Expand All @@ -258,7 +266,4 @@ def jsonData(self) -> Dict:
return copy.deepcopy(self._jsonData)

def __del__(self):
if (self._isActive):
builtins.chipStack.Call(
lambda: self._handle.pychip_Storage_ShutdownAdapter()
)
self.Shutdown()

0 comments on commit 5575bf2

Please sign in to comment.