From 261271683f49dc9855ca2170e4f14eb56d6134bc Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Thu, 3 Aug 2023 15:03:11 -0400 Subject: [PATCH] Update python logic for executing work in chip main loop (#28449) * Add python main loop work method * Fix typo and restyle * Fix typo * Code review updates * Comment update * Restyle --------- Co-authored-by: Andrei Litvin --- src/controller/python/BUILD.gn | 9 ++ .../python/chip/native/ChipMainLoopWork.h | 40 ++++++++ .../native/ChipMainLoopWork_StackLock.cpp | 44 +++++++++ .../native/ChipMainLoopWork_WorkSchedule.cpp | 75 +++++++++++++++ .../python/chip/tracing/TracingSetup.cpp | 92 +++++++++---------- 5 files changed, 210 insertions(+), 50 deletions(-) create mode 100644 src/controller/python/chip/native/ChipMainLoopWork.h create mode 100644 src/controller/python/chip/native/ChipMainLoopWork_StackLock.cpp create mode 100644 src/controller/python/chip/native/ChipMainLoopWork_WorkSchedule.cpp diff --git a/src/controller/python/BUILD.gn b/src/controller/python/BUILD.gn index 375a65cf7becbc..4b84c63d5ff30d 100644 --- a/src/controller/python/BUILD.gn +++ b/src/controller/python/BUILD.gn @@ -20,6 +20,7 @@ import("$dir_pw_build/python.gni") import("${chip_root}/build/chip/tools.gni") import("${chip_root}/src/platform/python.gni") +import("${chip_root}/src/system/system.gni") import("${dir_pw_unit_test}/test.gni") if (current_os == "mac") { @@ -33,6 +34,7 @@ config("controller_wno_deprecate") { declare_args() { chip_python_version = "0.0" chip_python_package_prefix = "chip" + chip_python_supports_stack_locking = chip_system_config_locking != "none" } shared_library("ChipDeviceCtrl") { @@ -77,12 +79,19 @@ shared_library("ChipDeviceCtrl") { "chip/internal/ChipThreadWork.h", "chip/internal/CommissionerImpl.cpp", "chip/logging/LoggingRedirect.cpp", + "chip/native/ChipMainLoopWork.h", "chip/native/PyChipError.cpp", "chip/tracing/TracingSetup.cpp", "chip/utils/DeviceProxyUtils.cpp", ] defines += [ "CHIP_CONFIG_MAX_GROUPS_PER_FABRIC=50" ] defines += [ "CHIP_CONFIG_MAX_GROUP_KEYS_PER_FABRIC=50" ] + + if (chip_python_supports_stack_locking) { + sources += [ "chip/native/ChipMainLoopWork_StackLock.cpp" ] + } else { + sources += [ "chip/native/ChipMainLoopWork_WorkSchedule.cpp" ] + } } else { sources += [ "chip/server/Options.cpp", diff --git a/src/controller/python/chip/native/ChipMainLoopWork.h b/src/controller/python/chip/native/ChipMainLoopWork.h new file mode 100644 index 00000000000000..831badefd8855b --- /dev/null +++ b/src/controller/python/chip/native/ChipMainLoopWork.h @@ -0,0 +1,40 @@ +/* + * + * Copyright (c) 2023 Project CHIP Authors + * All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +#pragma once + +#include + +namespace chip { +namespace MainLoopWork { + +/** + * Executes the given function in the CHIP main loop if one exists. + * + * Several implementations exist, however generally: + * + * - if already in the chip main loop (or main loop is not running), + * `f` gets executed right away + * - otherwise: + * - if chip stack locking is available, `f` is executed within the lock + * - if chip stack locking not available, this will schedule and WAIT + * for `f` to execute + */ +void ExecuteInMainLoop(std::function f); + +} // namespace MainLoopWork +} // namespace chip diff --git a/src/controller/python/chip/native/ChipMainLoopWork_StackLock.cpp b/src/controller/python/chip/native/ChipMainLoopWork_StackLock.cpp new file mode 100644 index 00000000000000..f848a3bc2c32dc --- /dev/null +++ b/src/controller/python/chip/native/ChipMainLoopWork_StackLock.cpp @@ -0,0 +1,44 @@ +/* + * + * Copyright (c) 2023 Project CHIP Authors + * All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +#include "ChipMainLoopWork.h" + +#include + +namespace chip { +namespace MainLoopWork { + +void ExecuteInMainLoop(std::function f) +{ + // NOTE: requires CHIP_STACK_LOCK_TRACKING_ENABLED to be available (which python builds + // generally have) to ensure chip stack locks are not deadlocking, since these + // functions do not know the actual state of the chip main loop. + // + // TODO: it may be a good assumption that python code asking for this will NOT run in + // chip main loop, however we try to be generic + if (chip::DeviceLayer::PlatformMgr().IsChipStackLockedByCurrentThread()) + { + f(); + return; + } + + chip::DeviceLayer::StackLock lock; + f(); +} + +} // namespace MainLoopWork +} // namespace chip diff --git a/src/controller/python/chip/native/ChipMainLoopWork_WorkSchedule.cpp b/src/controller/python/chip/native/ChipMainLoopWork_WorkSchedule.cpp new file mode 100644 index 00000000000000..fec65a150c9396 --- /dev/null +++ b/src/controller/python/chip/native/ChipMainLoopWork_WorkSchedule.cpp @@ -0,0 +1,75 @@ +/* + * + * Copyright (c) 2023 Project CHIP Authors + * All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +#include "ChipMainLoopWork.h" + +#include +#include + +namespace chip { +namespace MainLoopWork { +namespace { + +struct WorkData +{ + std::function callback; + sem_t done; + + WorkData() { sem_init(&done, 0 /* shared */, 0); } + ~WorkData() { sem_destroy(&done); } + void Post() { sem_post(&done); } + void Wait() { sem_wait(&done); } +}; + +void PerformWork(intptr_t arg) +{ + WorkData * work = reinterpret_cast(arg); + + work->callback(); + work->Post(); +} + +} // namespace + +void ExecuteInMainLoop(std::function f) +{ + + // NOTE: requires CHIP_STACK_LOCK_TRACKING_ENABLED to be available (which python builds + // generally have) to ensure chip stack locks are not deadlocking, since these + // functions do not know the actual state of the chip main loop. + // + // TODO: it may be a good assumption that python code asking for this will NOT run in + // chip main loop, however we try to be generic + if (chip::DeviceLayer::PlatformMgr().IsChipStackLockedByCurrentThread()) + { + f(); + return; + } + + // NOTE: the code below assumes that chip main loop is running. + // if it does not, this will deadlock. + // + // IsChipStackLockedByCurrentThread is expected to be aware of main loop + // not running. + WorkData workdata; + workdata.callback = f; + chip::DeviceLayer::PlatformMgr().ScheduleWork(PerformWork, reinterpret_cast(&workdata)); + workdata.Wait(); +} + +} // namespace MainLoopWork +} // namespace chip diff --git a/src/controller/python/chip/tracing/TracingSetup.cpp b/src/controller/python/chip/tracing/TracingSetup.cpp index d09a655b74c633..73e0633b3bf373 100644 --- a/src/controller/python/chip/tracing/TracingSetup.cpp +++ b/src/controller/python/chip/tracing/TracingSetup.cpp @@ -16,8 +16,8 @@ * limitations under the License. */ +#include #include -#include #include #include @@ -27,17 +27,6 @@ #include namespace { - -using chip::DeviceLayer::PlatformMgr; - -class ScopedStackLock -{ -public: - ScopedStackLock() { PlatformMgr().LockChipStack(); } - - ~ScopedStackLock() { PlatformMgr().UnlockChipStack(); } -}; - chip::Tracing::Json::JsonBackend gJsonBackend; chip::Tracing::Perfetto::FileTraceOutput gPerfettoFileOutput; @@ -47,58 +36,61 @@ chip::Tracing::Perfetto::PerfettoBackend gPerfettoBackend; extern "C" void pychip_tracing_start_json_log(const char * file_name) { - - ScopedStackLock lock; - - gJsonBackend.CloseFile(); // just in case, ensure no file output - chip::Tracing::Register(gJsonBackend); + chip::MainLoopWork::ExecuteInMainLoop([] { + gJsonBackend.CloseFile(); // just in case, ensure no file output + chip::Tracing::Register(gJsonBackend); + }); } extern "C" PyChipError pychip_tracing_start_json_file(const char * file_name) { - ScopedStackLock lock; - - CHIP_ERROR err = gJsonBackend.OpenFile(file_name); - if (err != CHIP_NO_ERROR) - { - return ToPyChipError(err); - } - chip::Tracing::Register(gJsonBackend); - return ToPyChipError(CHIP_NO_ERROR); + CHIP_ERROR err = CHIP_NO_ERROR; + + chip::MainLoopWork::ExecuteInMainLoop([&err, file_name] { + err = gJsonBackend.OpenFile(file_name); + if (err != CHIP_NO_ERROR) + { + return; + } + chip::Tracing::Register(gJsonBackend); + }); + + return ToPyChipError(err); } extern "C" void pychip_tracing_start_perfetto_system() { - ScopedStackLock lock; - - chip::Tracing::Perfetto::Initialize(perfetto::kSystemBackend); - chip::Tracing::Perfetto::RegisterEventTrackingStorage(); - chip::Tracing::Register(gPerfettoBackend); + chip::MainLoopWork::ExecuteInMainLoop([] { + chip::Tracing::Perfetto::Initialize(perfetto::kSystemBackend); + chip::Tracing::Perfetto::RegisterEventTrackingStorage(); + chip::Tracing::Register(gPerfettoBackend); + }); } extern "C" PyChipError pychip_tracing_start_perfetto_file(const char * file_name) { - ScopedStackLock lock; - - chip::Tracing::Perfetto::Initialize(perfetto::kInProcessBackend); - chip::Tracing::Perfetto::RegisterEventTrackingStorage(); - - CHIP_ERROR err = gPerfettoFileOutput.Open(file_name); - if (err != CHIP_NO_ERROR) - { - return ToPyChipError(err); - } - chip::Tracing::Register(gPerfettoBackend); - - return ToPyChipError(CHIP_NO_ERROR); + CHIP_ERROR err = CHIP_NO_ERROR; + chip::MainLoopWork::ExecuteInMainLoop([&err, file_name] { + chip::Tracing::Perfetto::Initialize(perfetto::kInProcessBackend); + chip::Tracing::Perfetto::RegisterEventTrackingStorage(); + + err = gPerfettoFileOutput.Open(file_name); + if (err != CHIP_NO_ERROR) + { + return; + } + chip::Tracing::Register(gPerfettoBackend); + }); + + return ToPyChipError(err); } extern "C" void pychip_tracing_stop() { - ScopedStackLock lock; - - chip::Tracing::Perfetto::FlushEventTrackingStorage(); - gPerfettoFileOutput.Close(); - chip::Tracing::Unregister(gPerfettoBackend); - chip::Tracing::Unregister(gJsonBackend); + chip::MainLoopWork::ExecuteInMainLoop([] { + chip::Tracing::Perfetto::FlushEventTrackingStorage(); + gPerfettoFileOutput.Close(); + chip::Tracing::Unregister(gPerfettoBackend); + chip::Tracing::Unregister(gJsonBackend); + }); }