[Tizen] Need to check PlatformManager Init / Shutdown function in detail

[dh79pyun]

This issue is for handling the potential issue for PlatformManager's Init() and Shutdown() API.
We found the deadlock issue when the application calls Shutdown twice. Because the deadlock issue is very critical. So to release the hotfix patchset, we added the prevent logic for deadlock.
#28094

But still remain the potential issues, so need to handle it using this issue.

---

#28094 (comment)

I would ask why the Shutdown() is called twice? If that's a normal behavior, then you will have to move this check at the beginning of this function (above GenericPlatformManagerImpl_POSIX::_Shutdown()), because calling GenericPlatformManagerImpl_POSIX::_Shutdown() twice will cause double-free.

However, if we need to protect against calling Shutdown() twice in a row, what about Init()? It also can not be called twice (without calling Shutdown in between). And this issue is not Tizen specific. IMHO, the rule applies to almost all Init/Shutdown calls.

Slack's converstions.

Arkadiusz Bokowy
1일 전
Hi, I've got a question regarding one PR for Tizen: #28094
We've got an issue where the Shutdown() is called twice, and I'd like to ask someone who is more familiar with the overall design, whether calling Init()/Shutdown() twice (or more) in a row is in general allowed and should be handled by the platform code? Or maybe if one of these functions is called more than once in a row is a sign of an issue somewhere else? I'm asking, because there are other platforms where Init() and Shutdown() are not idempotent.
#28094 [Tizen] Fix the deadlock issue during shutdown the system layer
When the app calls PlatformManager's Shutdown API twice, the function tries to join the abnormal main loop's thread. Adds the prevent function to avoid this deadlock issue.
Labels
platform, review - pending, tizen
https://github.com/[project-chip/connectedhomeip](https://github.com/project-chip/connectedhomeip)|project-chip/connectedhomeipproject-chip/connectedhomeip | 어제 오후 1:42 | 봇이 추가한 GitHub
5개의 댓글

Boris Zbarsky
16시간 전
I would think double-shutdown is generally an indication of an issue....

Boris Zbarsky
16시간 전
But depends on how the API is documented.

Arkadiusz Bokowy
15시간 전
Yes, we can blame the documentation :웃음: but I'll like to know what is the grand design for using Init()'s instead of using constructors/destructors, and whether Init()/Shutdown() should behave like ones. As for now we are OK with a quick fix of making Shutdown() idempotent, but maybe later it should be fixed in a more general/platform agnostic way.

Boris Zbarsky
5시간 전
Oh, PlatformManager shutdown is definitely not OK to call twice....

Arkadiusz Bokowy
1시간 전
OK, thanks

[dh79pyun]

When I added the simulation codes called PlatformManager's Init() twice, the platform's impl function is not called twice.
In PlatformManager.h file, already added the prevent code.
https://github.com/project-chip/connectedhomeip/blob/master/src/include/platform/PlatformManager.h#L366

Same with this, we can add the preventing logic in PlatformManager.h's inline function. And it will be the generic solution for all platforms. But the change will effect on all platform's operation. So I am not sure, it will be good to raise PR in my side.

diff --git a/src/include/platform/PlatformManager.h b/src/include/platform/PlatformManager.h
index 41334a465b..6d5b682956 100644
--- a/src/include/platform/PlatformManager.h
+++ b/src/include/platform/PlatformManager.h
@@ -442,6 +442,11 @@ inline CHIP_ERROR PlatformManager::StopEventLoopTask()
  */
 inline void PlatformManager::Shutdown()
 {
+    if (!mInitialized)
+    {
+        return;
+    }
+
     static_cast<ImplClass *>(this)->_Shutdown();
     mInitialized = false;
 }