Skip to content

Commit

Permalink
Revert "[Python dev ctrl] ChipStack is initialized twice on Darwin (#… (
Browse files Browse the repository at this point in the history
#6550)

* Revert "[Python dev ctrl] ChipStack is initialized twice on Darwin (#6066)"

This reverts commit 23cba58.

Logic seemed correct before: python only works on device layer and
device layer requires initialization.

Unsure why darwin is initialized twice, howver the native bit is NOT
supposed to be called by the device-ctrl-code only by chip-native. Have
to figure out how not to break chip-native.

* Track platform manager initialization and do not allow double-init

* Added large comment about thread safety in the double-init check of the platform manager

* Removed std::atomic reference as it is not relevant here: we need a mutex to wait for init to complete, not just blindly say iniialized before it is done
  • Loading branch information
andy31415 authored and pull[bot] committed Jul 30, 2021
1 parent 4a85be2 commit 702b90d
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 3 deletions.
2 changes: 0 additions & 2 deletions src/controller/python/chip/native/StackInit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,11 @@ void pychip_native_init()
}
#endif // CHIP_DEVICE_LAYER_TARGET_LINUX && CHIP_DEVICE_CONFIG_ENABLE_CHIPOBLE

#ifndef CONFIG_DEVICE_LAYER
err = chip::DeviceLayer::PlatformMgr().InitChipStack();
if (err != CHIP_NO_ERROR)
{
ChipLogError(DeviceLayer, "Failed to initialize CHIP stack: platform init failed: %s", chip::ErrorStr(err));
}
#endif
int result = pthread_create(&sPlatformMainThread, nullptr, PlatformMainLoop, nullptr);
int tmpErrno = errno;

Expand Down
17 changes: 16 additions & 1 deletion src/include/platform/PlatformManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ class PlatformManager
CHIP_ERROR Shutdown();

private:
bool mInitialized = false;
// ===== Members for internal use by the following friends.

friend class PlatformManagerImpl;
Expand Down Expand Up @@ -181,7 +182,21 @@ namespace DeviceLayer {

inline CHIP_ERROR PlatformManager::InitChipStack()
{
return static_cast<ImplClass *>(this)->_InitChipStack();
// NOTE: this is NOT thread safe and cannot be as the chip stack lock is prepared by
// InitChipStack itself on many platforms.
//
// In the future, this could be moved into specific platform code (where it can
// be made thread safe). In general however, init twice
// is likely a logic error and we may want to avoid that path anyway. Likely to
// be done once code stabilizes a bit more.
if (mInitialized)
{
return CHIP_NO_ERROR;
}

CHIP_ERROR err = static_cast<ImplClass *>(this)->_InitChipStack();
mInitialized = (err == CHIP_NO_ERROR);
return err;
}

inline CHIP_ERROR PlatformManager::AddEventHandler(EventHandlerFunct handler, intptr_t arg)
Expand Down

0 comments on commit 702b90d

Please sign in to comment.