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

Python has a broken threading model #20233

Closed
mrjerryjohns opened this issue Jul 1, 2022 · 3 comments
Closed

Python has a broken threading model #20233

mrjerryjohns opened this issue Jul 1, 2022 · 3 comments
Assignees
Labels
python stale Stale issue or PR

Comments

@mrjerryjohns
Copy link
Contributor

mrjerryjohns commented Jul 1, 2022

Spurned by failures discovered in #20178, I did an investigation into the Python threading model. As I understood how it worked, it became readily apparent that it has a fairly broken threading model:

  1. The main Matter thread got spun up implicitly anytime any Python module attempted to get a handle to the underlying ChipDeviceCtrl.so to invoke a C++ function. This happened in chip.native.GetLibraryHandle() - that would call the C++ pychip_native_init, which then did the pthread creation. This made it highly in-deterministic and un-coordinated as to when the Matter thread would actually spool up relative to other core stack initialization that had to happen first.

Specifically, the failure seen in #20178 occurred because ChipReplStartup.py would first attempt to setup logging by calling chip.logging.RedirectToPythonLogging(), which attempted to get a handle to the shared library, which caused the Matter thread to spin-up.

It's only after that that we'd initialize ChipStack, which would actually do the core job of initializing the stack (which included initializing DeviceControllerFactory. It did not do this on the Matter thread, and called the accompanying C++ function directly, triggering the failure seen in #20178.

  1. The actual creation of the Matter thread happened in chip/native/StackInit.cpp::pychip_native_init. This however, did not use the PlatformMgr APIs. Instead, it directly instantiated a pthread and pumped the event loop from there. This evaded the checks in IsChipStackLockedByCurrentThread because mMainLoopStarted would never get set to true forever, effectively making it look like the Matter thread never started. Assert that controllers are inited and shut down with the Matter lock held. #20178 switching to using mHasValidChipTask now correctly triggers this, because mHasValidChipTask is set to true inside _RunEventLoop, and not EventLoopTaskMain (which the Python C++ code never used).

  2. There were 3 different 'stack init' APIs in Python - the one in StackInit.cpp, and two other ones in ChipDeviceController-ScriptBinding.cpp. The latter two fragmented the logic of stack initialization.

@turon
Copy link
Contributor

turon commented Jul 20, 2022

Can this be closed given both #20234 and #20465 have been merged?

@stale
Copy link

stale bot commented Jan 16, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale Stale issue or PR label Jan 16, 2023
@stale
Copy link

stale bot commented Feb 5, 2023

This stale issue has been automatically closed. Thank you for your contributions.

@stale stale bot closed this as completed Feb 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
python stale Stale issue or PR
Projects
None yet
Development

No branches or pull requests

2 participants