From dfdb0c87f4e363abbd10632de08ce10f26f7a53c Mon Sep 17 00:00:00 2001 From: Boram Bae Date: Mon, 21 Dec 2020 21:22:16 +0900 Subject: [PATCH] Fix a crash when TizenVsyncWaiter is destroyed. * Don't have vblank_ecore_pipe as a member anymore * Does not consider abnormal situations. This means, this situation should never happen. * Refactor TizenVsyncWaiter Signed-off-by: Boram Bae --- shell/platform/tizen/tizen_embedder_engine.cc | 10 +- shell/platform/tizen/tizen_surface_gl.cc | 2 +- shell/platform/tizen/tizen_vsync_waiter.cc | 166 ++++++++---------- shell/platform/tizen/tizen_vsync_waiter.h | 32 ++-- 4 files changed, 96 insertions(+), 114 deletions(-) diff --git a/shell/platform/tizen/tizen_embedder_engine.cc b/shell/platform/tizen/tizen_embedder_engine.cc index 62e26f34fda01..86f4e44f564a5 100644 --- a/shell/platform/tizen/tizen_embedder_engine.cc +++ b/shell/platform/tizen/tizen_embedder_engine.cc @@ -56,7 +56,7 @@ TizenEmbedderEngine::TizenEmbedderEngine( message_dispatcher = std::make_unique(messenger.get()); - tizen_vsync_waiter_ = std::make_unique(); + tizen_vsync_waiter_ = std::make_unique(this); } TizenEmbedderEngine::~TizenEmbedderEngine() { @@ -164,8 +164,6 @@ bool TizenEmbedderEngine::RunEngine( return false; } - tizen_vsync_waiter_->AsyncWaitForRunEngineSuccess(flutter_engine); - std::unique_ptr textures = std::make_unique(); textures->flutter_engine = flutter_engine; @@ -330,7 +328,11 @@ void TizenEmbedderEngine::OnFlutterPlatformMessage( void TizenEmbedderEngine::OnVsyncCallback(void* user_data, intptr_t baton) { TizenEmbedderEngine* tizen_embedder_engine = reinterpret_cast(user_data); - tizen_embedder_engine->tizen_vsync_waiter_->AsyncWaitForVsync(baton); + if (tizen_embedder_engine->tizen_vsync_waiter_->IsValid()) { + tizen_embedder_engine->tizen_vsync_waiter_->AsyncWaitForVsync(baton); + } else { + LoggerW("TizenVsyncWaiter is not valid"); + } } // Converts a FlutterPlatformMessage to an equivalent FlutterDesktopMessage. diff --git a/shell/platform/tizen/tizen_surface_gl.cc b/shell/platform/tizen/tizen_surface_gl.cc index 2a15af8f19002..53493221f1cd1 100644 --- a/shell/platform/tizen/tizen_surface_gl.cc +++ b/shell/platform/tizen/tizen_surface_gl.cc @@ -342,7 +342,7 @@ void* TizenSurfaceGL::OnProcResolver(const char* name) { GL_FUNC(glVertexAttrib4fv) GL_FUNC(glVertexAttribPointer) GL_FUNC(glViewport) - LoggerW("Could not resolve: %s", name); + LoggerD("Could not resolve: %s", name); return nullptr; } #undef GL_FUNC diff --git a/shell/platform/tizen/tizen_vsync_waiter.cc b/shell/platform/tizen/tizen_vsync_waiter.cc index 9ee480a1aff1c..edb94983b3241 100644 --- a/shell/platform/tizen/tizen_vsync_waiter.cc +++ b/shell/platform/tizen/tizen_vsync_waiter.cc @@ -4,65 +4,87 @@ #include "tizen_vsync_waiter.h" +#include + #include "flutter/shell/platform/tizen/logger.h" +#include "flutter/shell/platform/tizen/tizen_embedder_engine.h" + +static std::atomic g_vblank_ecore_pipe = nullptr; -TizenVsyncWaiter::TizenVsyncWaiter() - : client_(NULL), - output_(NULL), - vblank_(NULL), - flutter_engine_(nullptr), - baton_(0), - vblank_ecore_pipe_(NULL) { - if (CreateTDMVblank()) { - std::thread t(CreateVblankEventLoop, this); - t.join(); - } else { - LoggerE("CreateVsyncVaiter fail"); +static const int VBLANK_LOOP_REQUEST = 1; +static const int VBLANK_LOOP_DEL_PIPE = 2; + +static void SendVblankLoopRequest(int event_type) { + if (ecore_pipe_write(g_vblank_ecore_pipe.load(), &event_type, + sizeof(event_type)) == EINA_FALSE) { + LoggerE("Failed to Send Reqeust [%s]", event_type == VBLANK_LOOP_REQUEST + ? "VBLANK_LOOP_REQUEST" + : "VBLANK_LOOP_DEL_PIPE"); } } -void TizenVsyncWaiter::CreateVblankEventLoop(void* data) { - TizenVsyncWaiter* tizen_vsync_waiter = - reinterpret_cast(data); - if (!ecore_init()) { - LoggerE("ERROR: Cannot init Ecore!"); +TizenVsyncWaiter::TizenVsyncWaiter(TizenEmbedderEngine* engine) + : engine_(engine) { + if (!CreateTDMVblank()) { + LoggerE("Failed to create TDM vblank"); return; } - tizen_vsync_waiter->vblank_ecore_pipe_ = - ecore_pipe_add(VblankEventLoopCallback, tizen_vsync_waiter); - LoggerD("ecore_init successful"); - ecore_main_loop_begin(); - ecore_shutdown(); -} -void TizenVsyncWaiter::VblankEventLoopCallback(void* data, void* buffer, - unsigned int nbyte) { - TizenVsyncWaiter* tizen_vsync_waiter = - reinterpret_cast(data); - int* event_type = reinterpret_cast(buffer); - if ((*event_type) == VBLANK_LOOP_REQUEST) { - tizen_vsync_waiter->AsyncWaitForVsyncCallback(); - } else if ((*event_type) == VBLANK_LOOP_DEL_PIPE) { - tizen_vsync_waiter->DeleteVblankEventPipe(); + std::thread t( + [this](void* data) { + if (!ecore_init()) { + LoggerE("Failed to init Ecore"); + return; + } + Ecore_Pipe* vblank_ecore_pipe = ecore_pipe_add( + [](void* data, void* buffer, unsigned int nbyte) { + TizenVsyncWaiter* tizen_vsync_waiter = + reinterpret_cast(data); + int event_type = *(reinterpret_cast(buffer)); + if (event_type == VBLANK_LOOP_REQUEST) { + tizen_vsync_waiter->HandleVblankLoopRequest(); + } else if (event_type == VBLANK_LOOP_DEL_PIPE) { + if (g_vblank_ecore_pipe.load()) { + ecore_pipe_del(g_vblank_ecore_pipe); + g_vblank_ecore_pipe = NULL; + } + ecore_main_loop_quit(); + } + }, + this); + + g_vblank_ecore_pipe.store(vblank_ecore_pipe); + ecore_main_loop_begin(); + ecore_shutdown(); + }, + nullptr); + t.join(); + + if (g_vblank_ecore_pipe.load() == nullptr) { + LoggerE("Failed to create Ecore Pipe"); } } -void TizenVsyncWaiter::AsyncWaitForVsyncCallback() { - tdm_error ret; - ret = tdm_client_vblank_wait(vblank_, 1, TdmClientVblankCallback, this); - if (ret != TDM_ERROR_NONE) { - LoggerE("ERROR, ret = %d", ret); - return; +TizenVsyncWaiter::~TizenVsyncWaiter() { + if (g_vblank_ecore_pipe.load()) { + SendVblankLoopRequest(VBLANK_LOOP_DEL_PIPE); + } + if (vblank_) { + tdm_client_vblank_destroy(vblank_); + } + if (client_) { + tdm_client_destroy(client_); } - tdm_client_handle_events(client_); } -void TizenVsyncWaiter::DeleteVblankEventPipe() { - if (vblank_ecore_pipe_) { - ecore_pipe_del(vblank_ecore_pipe_); - vblank_ecore_pipe_ = NULL; - } - ecore_main_loop_quit(); +void TizenVsyncWaiter::AsyncWaitForVsync(intptr_t baton) { + baton_ = baton; + SendVblankLoopRequest(VBLANK_LOOP_REQUEST); +} + +bool TizenVsyncWaiter::IsValid() { + return g_vblank_ecore_pipe.load() && client_ && output_ && vblank_ && + engine_->flutter_engine; } bool TizenVsyncWaiter::CreateTDMVblank() { @@ -86,6 +108,7 @@ bool TizenVsyncWaiter::CreateTDMVblank() { } tdm_client_vblank_set_enable_fake(vblank_, 1); + return true; } @@ -94,57 +117,20 @@ void TizenVsyncWaiter::TdmClientVblankCallback( unsigned int tv_sec, unsigned int tv_usec, void* user_data) { TizenVsyncWaiter* tizen_vsync_waiter = reinterpret_cast(user_data); - if (tizen_vsync_waiter == nullptr) { - LoggerE("tizen_vsync_waiter is null"); - return; - } - if (tizen_vsync_waiter->flutter_engine_ == nullptr) { - LoggerI("flutter engine creation is not completed"); - return; - } + uint64_t frame_start_time_nanos = tv_sec * 1e9 + tv_usec * 1e3; uint64_t frame_target_time_nanos = 16.6 * 1e6 + frame_start_time_nanos; - FlutterEngineOnVsync(tizen_vsync_waiter->flutter_engine_, + FlutterEngineOnVsync(tizen_vsync_waiter->engine_->flutter_engine, tizen_vsync_waiter->baton_, frame_start_time_nanos, frame_target_time_nanos); } -bool TizenVsyncWaiter::AsyncWaitForVsync() { - if (nullptr == flutter_engine_) { - LoggerD("flutter_engine_ is null"); - return false; - } - if (vblank_ecore_pipe_) { - int event_type = VBLANK_LOOP_REQUEST; - ecore_pipe_write(vblank_ecore_pipe_, &event_type, sizeof(event_type)); - } - return true; -} - -TizenVsyncWaiter::~TizenVsyncWaiter() { - if (vblank_ecore_pipe_) { - int event_type = VBLANK_LOOP_DEL_PIPE; - ecore_pipe_write(vblank_ecore_pipe_, &event_type, sizeof(event_type)); - } - if (vblank_) { - tdm_client_vblank_destroy(vblank_); - } - if (client_) { - tdm_client_destroy(client_); - } -} - -void TizenVsyncWaiter::AsyncWaitForVsync(intptr_t baton) { - baton_ = baton; - AsyncWaitForVsync(); -} - -void TizenVsyncWaiter::AsyncWaitForRunEngineSuccess( - FLUTTER_API_SYMBOL(FlutterEngine) flutter_engine) { - flutter_engine_ = flutter_engine; - if (baton_ == 0) { - LoggerD("baton_ == 0"); +void TizenVsyncWaiter::HandleVblankLoopRequest() { + tdm_error ret; + ret = tdm_client_vblank_wait(vblank_, 1, TdmClientVblankCallback, this); + if (ret != TDM_ERROR_NONE) { + LoggerE("ERROR, ret = %d", ret); return; } - AsyncWaitForVsync(); + tdm_client_handle_events(client_); } diff --git a/shell/platform/tizen/tizen_vsync_waiter.h b/shell/platform/tizen/tizen_vsync_waiter.h index b7bb977fdd477..494e65a4aee9f 100644 --- a/shell/platform/tizen/tizen_vsync_waiter.h +++ b/shell/platform/tizen/tizen_vsync_waiter.h @@ -5,41 +5,35 @@ #ifndef EMBEDDER_TIZEN_VSYNC_WAITER_H_ #define EMBEDDER_TIZEN_VSYNC_WAITER_H_ -#include #include #include #include "flutter/shell/platform/embedder/embedder.h" +class TizenEmbedderEngine; + class TizenVsyncWaiter { public: - TizenVsyncWaiter(); + TizenVsyncWaiter(TizenEmbedderEngine* engine); virtual ~TizenVsyncWaiter(); - bool CreateTDMVblank(); - bool AsyncWaitForVsync(); void AsyncWaitForVsync(intptr_t baton); - void AsyncWaitForRunEngineSuccess(FLUTTER_API_SYMBOL(FlutterEngine) - flutter_engine); + + bool IsValid(); private: - static const int VBLANK_LOOP_REQUEST = 1; - static const int VBLANK_LOOP_DEL_PIPE = 2; - void AsyncWaitForVsyncCallback(); - void DeleteVblankEventPipe(); - static void CreateVblankEventLoop(void* data); + bool CreateTDMVblank(); + void HandleVblankLoopRequest(); static void TdmClientVblankCallback(tdm_client_vblank* vblank, tdm_error error, unsigned int sequence, unsigned int tv_sec, unsigned int tv_usec, void* user_data); - static void VblankEventLoopCallback(void* data, void* buffer, - unsigned int nbyte); - tdm_client* client_; - tdm_client_output* output_; - tdm_client_vblank* vblank_; - FLUTTER_API_SYMBOL(FlutterEngine) flutter_engine_; - intptr_t baton_; - Ecore_Pipe* vblank_ecore_pipe_; + + tdm_client* client_{nullptr}; + tdm_client_output* output_{nullptr}; + tdm_client_vblank* vblank_{nullptr}; + TizenEmbedderEngine* engine_{nullptr}; + intptr_t baton_{0}; }; #endif // EMBEDDER_TIZEN_VSYNC_WAITER_H_