From 6c8a3fb88c272c306733976c35b64e856089613f Mon Sep 17 00:00:00 2001 From: Changgyu Choi Date: Fri, 6 May 2022 17:21:51 +0900 Subject: [PATCH] Refactor WorkerThread Changes: - Moves Scheduler to CreateCacheRequestHandler. - Adds PreExec() & PostExec() to RequestHandler steps. - Adds RequestHandlerFactory for loose coupling. Change-Id: I0520f73164d335e6b5db97000db25c4132caad6d Signed-off-by: Changgyu Choi --- .../request_handler/abstract_request_handler.cc | 6 + .../request_handler/abstract_request_handler.hh | 2 + .../create_cache_request_handler.cc | 65 ++++++++ .../create_cache_request_handler.hh | 27 +++- src/server/request_handler_factory.cc | 65 ++++++++ src/server/request_handler_factory.hh | 42 ++++++ src/server/worker_thread.cc | 167 +++++---------------- src/server/worker_thread.hh | 22 +-- 8 files changed, 242 insertions(+), 154 deletions(-) create mode 100644 src/server/request_handler_factory.cc create mode 100644 src/server/request_handler_factory.hh diff --git a/src/server/request_handler/abstract_request_handler.cc b/src/server/request_handler/abstract_request_handler.cc index 4f00260..bc33e01 100644 --- a/src/server/request_handler/abstract_request_handler.cc +++ b/src/server/request_handler/abstract_request_handler.cc @@ -19,5 +19,11 @@ pid_t AbstractRequestHandler::GetPID() { return pid_; } +void AbstractRequestHandler::PreExec() { +} + +void AbstractRequestHandler::PostExec() { +} + } // namespace request_handler } // namespace pkgmgr_server diff --git a/src/server/request_handler/abstract_request_handler.hh b/src/server/request_handler/abstract_request_handler.hh index 8285e80..8b463d4 100644 --- a/src/server/request_handler/abstract_request_handler.hh +++ b/src/server/request_handler/abstract_request_handler.hh @@ -26,6 +26,8 @@ class EXPORT_API AbstractRequestHandler { virtual std::vector ExtractResult() = 0; + virtual void PreExec(); + virtual void PostExec(); void SetPID(pid_t pid); protected: diff --git a/src/server/request_handler/create_cache_request_handler.cc b/src/server/request_handler/create_cache_request_handler.cc index 4e60054..6cebec6 100644 --- a/src/server/request_handler/create_cache_request_handler.cc +++ b/src/server/request_handler/create_cache_request_handler.cc @@ -7,12 +7,69 @@ #include #include "cache_db_handler.hh" +#include "utils/logging.hh" namespace psd = pkgmgr_server::database; namespace pkgmgr_server { namespace request_handler { +CreateCacheRequestHandler::Scheduler::Scheduler(pid_t tid) : tid_(tid) { + Get(&policy_, ¶m_); +} + +void CreateCacheRequestHandler::Scheduler::ChangePolicy() { + struct sched_param param { + .sched_priority = 1, + }; + if (Set(SCHED_RR, ¶m) != 0) + return; + + int policy = -1; + Get(&policy, ¶m); + LOG(WARNING) << "Current policy: " << policy + << ", priority: " << param.sched_priority; +} + +void CreateCacheRequestHandler::Scheduler::ResetPolicy() { + if (Set(policy_, ¶m_) != 0) + return; + + struct sched_param param = { + .sched_priority = 0, + }; + int policy = -1; + Get(&policy, ¶m); + LOG(WARNING) << "Current policy: " << policy + << ", priority: " << param.sched_priority; +} + +void CreateCacheRequestHandler::Scheduler::Get(int* policy, + struct sched_param* param) { + if (sched_getparam(tid_, param) != 0) + LOG(ERROR) << "sched_getparam() is failed. errno: " << errno; + + *policy = sched_getscheduler(tid_); + if (*policy < 0) + LOG(ERROR) << "sched_getscheduler() is failed. errno: " << errno; +} + +int CreateCacheRequestHandler::Scheduler::Set(int policy, + struct sched_param* param) { + if (sched_setscheduler(tid_, policy, param) != 0) { + LOG(ERROR) << "sched_setscheduler() is failed. policy: " << policy + << ", errno: " << errno; + return -1; + } + + LOG(WARNING) << "policy: " << policy + << ", sched_priority: " << param->sched_priority; + return 0; +} + +CreateCacheRequestHandler::CreateCacheRequestHandler(): scheduler_(gettid()) { +} + bool CreateCacheRequestHandler::HandleRequest(unsigned char* data, int size, const std::string& locale) { // TODO(ilho159.kim) need to get logined user id @@ -28,5 +85,13 @@ std::vector CreateCacheRequestHandler::ExtractResult() { return {}; } +void CreateCacheRequestHandler::PreExec() { + scheduler_.ChangePolicy(); +} + +void CreateCacheRequestHandler::PostExec() { + scheduler_.ResetPolicy(); +} + } // namespace request_handler } // namespace pkgmgr_server diff --git a/src/server/request_handler/create_cache_request_handler.hh b/src/server/request_handler/create_cache_request_handler.hh index 2c7a548..cee35c0 100644 --- a/src/server/request_handler/create_cache_request_handler.hh +++ b/src/server/request_handler/create_cache_request_handler.hh @@ -19,15 +19,40 @@ namespace request_handler { class EXPORT_API CreateCacheRequestHandler : public AbstractRequestHandler { public: + CreateCacheRequestHandler(); bool HandleRequest(unsigned char* data, int size, const std::string& locale) override; std::vector ExtractResult() override; + void PreExec() override; + void PostExec() override; + + private: + class Scheduler { + public: + explicit Scheduler(pid_t tid); + + void ChangePolicy(); + void ResetPolicy(); + + private: + void Get(int* policy, struct sched_param* param); + int Set(int policy, struct sched_param* param); + + private: + pid_t tid_; + int policy_ = 0; + struct sched_param param_ = { + 0, + }; + }; + private: std::shared_ptr result_; + Scheduler scheduler_; }; } // namespace request_handler } // namespace pkgmgr_server -#endif // CREATE_CACHE_REQUEST_HANDLER_HH_ \ No newline at end of file +#endif // CREATE_CACHE_REQUEST_HANDLER_HH_ diff --git a/src/server/request_handler_factory.cc b/src/server/request_handler_factory.cc new file mode 100644 index 0000000..cdb3f1f --- /dev/null +++ b/src/server/request_handler_factory.cc @@ -0,0 +1,65 @@ +/* + * Copyright (c) 2022 Samsung Electronics Co., Ltd 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 "server/request_handler_factory.hh" + +#include "server/request_handler/command_request_handler.hh" +#include "server/request_handler/create_cache_request_handler.hh" +#include "server/request_handler/create_db_request_handler.hh" +#include "server/request_handler/get_appinfo_request_handler.hh" +#include "server/request_handler/get_cert_request_handler.hh" +#include "server/request_handler/get_depinfo_request_handler.hh" +#include "server/request_handler/get_pkginfo_request_handler.hh" +#include "server/request_handler/query_request_handler.hh" +#include "server/request_handler/set_cert_request_handler.hh" +#include "server/request_handler/set_pkginfo_request_handler.hh" +#include "utils/logging.hh" + +namespace pkgmgr_server { + +RequestHandlerFactory::RequestHandlerFactory() { + handler[pkgmgr_common::ReqType::GET_PKG_INFO].reset( + new request_handler::GetPkginfoRequestHandler()); + handler[pkgmgr_common::ReqType::GET_APP_INFO].reset( + new request_handler::GetAppinfoRequestHandler()); + handler[pkgmgr_common::ReqType::SET_PKG_INFO].reset( + new request_handler::SetPkginfoRequestHandler()); + handler[pkgmgr_common::ReqType::SET_CERT_INFO].reset( + new request_handler::SetCertRequestHandler()); + handler[pkgmgr_common::ReqType::GET_CERT_INFO].reset( + new request_handler::GetCertRequestHandler()); + handler[pkgmgr_common::ReqType::GET_PKG_DEP_INFO].reset( + new request_handler::GetDepinfoRequestHandler()); + handler[pkgmgr_common::ReqType::QUERY].reset( + new request_handler::QueryRequestHandler()); + handler[pkgmgr_common::ReqType::COMMAND].reset( + new request_handler::CommandRequestHandler()); + handler[pkgmgr_common::ReqType::CREATE_DB].reset( + new request_handler::CreateDBRequestHandler()); + handler[pkgmgr_common::ReqType::CREATE_CACHE].reset( + new request_handler::CreateCacheRequestHandler()); +} + +std::shared_ptr + RequestHandlerFactory::GetRequestHandler(pkgmgr_common::ReqType type) { + if (type <= pkgmgr_common::ReqType::REQ_TYPE_NONE || + type >= pkgmgr_common::ReqType::MAX) + return nullptr; + + return handler[type]; +} + +} // namespace pkgmgr_server diff --git a/src/server/request_handler_factory.hh b/src/server/request_handler_factory.hh new file mode 100644 index 0000000..5d693d5 --- /dev/null +++ b/src/server/request_handler_factory.hh @@ -0,0 +1,42 @@ +/* + * Copyright (c) 2022 Samsung Electronics Co., Ltd 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. + */ + +#ifndef SERVER_REQUEST_HANDLER_FACTORY_ +#define SERVER_REQUEST_HANDLER_FACTORY_ + +#include + +#include "common/request_type.hh" +#include "server/request_handler/abstract_request_handler.hh" + +namespace pkgmgr_server { + +class RequestHandlerFactory { + public: + RequestHandlerFactory(); + ~RequestHandlerFactory() = default; + + std::shared_ptr GetRequestHandler( + pkgmgr_common::ReqType type); + + private: + std::shared_ptr + handler[pkgmgr_common::ReqType::MAX]; +}; + +} // namespace pkgmgr_server + +#endif // SERVER_REQUEST_HANDLER_FACTORY_ diff --git a/src/server/worker_thread.cc b/src/server/worker_thread.cc index c377a8f..0fae011 100644 --- a/src/server/worker_thread.cc +++ b/src/server/worker_thread.cc @@ -20,17 +20,8 @@ #include #include "abstract_parcelable.hh" -#include "command_request_handler.hh" -#include "create_cache_request_handler.hh" -#include "create_db_request_handler.hh" -#include "db_handle_provider.hh" -#include "get_appinfo_request_handler.hh" -#include "get_cert_request_handler.hh" -#include "get_depinfo_request_handler.hh" -#include "get_pkginfo_request_handler.hh" -#include "query_request_handler.hh" -#include "set_cert_request_handler.hh" -#include "set_pkginfo_request_handler.hh" +#include "request_handler_factory.hh" +#include "server/database/db_handle_provider.hh" #include "utils/logging.hh" #include "pkgmgrinfo_debug.h" @@ -46,54 +37,6 @@ namespace pkgmgr_server { -WorkerThread::Scheduler::Scheduler(pid_t tid) : tid_(tid) { - Get(&policy_, ¶m_); -} - -void WorkerThread::Scheduler::ChangePolicy() { - struct sched_param param = { 0, }; - param.sched_priority = 1; - if (Set(SCHED_RR, ¶m) != 0) - return; - - int policy = -1; - Get(&policy, ¶m); - LOG(WARNING) << "Current policy: " << policy << ", priority: " - << param.sched_priority; -} - -void WorkerThread::Scheduler::ResetPolicy() { - if (Set(policy_, ¶m_) != 0) - return; - - struct sched_param param = { 0, }; - int policy = -1; - Get(&policy, ¶m); - LOG(WARNING) << "Current policy: " << policy << ", priority: " - << param.sched_priority; -} - -void WorkerThread::Scheduler::Get(int* policy, struct sched_param* param) { - if (sched_getparam(tid_, param) != 0) - LOG(ERROR) << "sched_getparam() is failed. errno: " << errno; - - *policy = sched_getscheduler(tid_); - if (*policy < 0) - LOG(ERROR) << "sched_getscheduler() is failed. errno: " << errno; -} - -int WorkerThread::Scheduler::Set(int policy, struct sched_param* param) { - if (sched_setscheduler(tid_, policy, param) != 0) { - LOG(ERROR) << "sched_setscheduler() is failed. policy: " << policy - << ", errno: " << errno; - return -1; - } - - LOG(WARNING) << "policy: " << policy << ", sched_priority: " - << param->sched_priority; - return 0; -} - WorkerThread::WorkerThread(unsigned int num) : stop_all_(false) { threads_.reserve(num); for (unsigned int i = 0; i < num; ++i) @@ -119,91 +62,56 @@ bool WorkerThread::PushQueue(std::shared_ptr req) { return true; } -void WorkerThread::Run() { - std::unique_ptr - handler[pkgmgr_common::ReqType::MAX]; - handler[pkgmgr_common::ReqType::GET_PKG_INFO].reset( - new request_handler::GetPkginfoRequestHandler()); - handler[pkgmgr_common::ReqType::GET_APP_INFO].reset( - new request_handler::GetAppinfoRequestHandler()); - handler[pkgmgr_common::ReqType::SET_PKG_INFO].reset( - new request_handler::SetPkginfoRequestHandler()); - handler[pkgmgr_common::ReqType::SET_CERT_INFO].reset( - new request_handler::SetCertRequestHandler()); - handler[pkgmgr_common::ReqType::GET_CERT_INFO].reset( - new request_handler::GetCertRequestHandler()); - handler[pkgmgr_common::ReqType::GET_PKG_DEP_INFO].reset( - new request_handler::GetDepinfoRequestHandler()); - handler[pkgmgr_common::ReqType::QUERY].reset( - new request_handler::QueryRequestHandler()); - handler[pkgmgr_common::ReqType::COMMAND].reset( - new request_handler::CommandRequestHandler()); - handler[pkgmgr_common::ReqType::CREATE_DB].reset( - new request_handler::CreateDBRequestHandler()); - handler[pkgmgr_common::ReqType::CREATE_CACHE].reset( - new request_handler::CreateCacheRequestHandler()); +std::shared_ptr WorkerThread::PopQueue() { + SetMemoryTrimTimer(); + std::unique_lock u(lock_); + cv_.wait(u, [this] { return !this->queue_.empty() || stop_all_; }); + if (stop_all_ && queue_.empty()) + return nullptr; - std::unique_ptr scheduler( - new WorkerThread::Scheduler(gettid())); + auto req = queue_.front(); + queue_.pop(); + return req; +} +void WorkerThread::Run() { + RequestHandlerFactory factory; LOG(DEBUG) << "Initialize request handlers"; while (true) { - std::shared_ptr req; - { - std::unique_lock u(lock_); - cv_.wait(u, [this] { return !this->queue_.empty() || stop_all_; }); - if (stop_all_ && queue_.empty()) - return; - req = PopQueue(); - } + std::shared_ptr req = PopQueue(); + if (req == nullptr) + return; - pkgmgr_common::ReqType type = req->GetRequestType(); + auto type = req->GetRequestType(); LOG(WARNING) << "Request type: " << pkgmgr_common::ReqTypeToString(type) - << " pid: " << req->GetSenderPID(); - if (type <= pkgmgr_common::ReqType::REQ_TYPE_NONE - || type >= pkgmgr_common::ReqType::MAX) { - LOG(ERROR) << "Request type is invalid: " << static_cast(type) - << ", pid:" << req->GetSenderPID(); - SendError(req); - + << " pid: " << req->GetSenderPID(); + auto handler = factory.GetRequestHandler(type); + if (handler == nullptr) continue; - } try { - if (type == pkgmgr_common::ReqType::CREATE_CACHE) - scheduler->ChangePolicy(); + handler->PreExec(); - handler[type]->SetPID(req->GetSenderPID()); - if (!handler[type]->HandleRequest(req->GetData(), req->GetSize(), - locale_.GetObject())) + handler->SetPID(req->GetSenderPID()); + if (!handler->HandleRequest(req->GetData(), req->GetSize(), + locale_.GetObject())) LOG(ERROR) << "Failed to handle request"; + + std::vector result_data = handler->ExtractResult(); + if (req->SendData(result_data.data(), result_data.size()) == false) + LOG(ERROR) << "Failed to send response pid: " << req->GetSenderPID(); + else + LOG(WARNING) << "Success response pid: " << req->GetSenderPID(); } catch (const std::exception& err) { LOG(ERROR) << "Exception occurred: " << err.what() - << ", pid: " << req->GetSenderPID(); + << ", pid: " << req->GetSenderPID(); SendError(req); - if (type == pkgmgr_common::ReqType::CREATE_CACHE) - scheduler->ResetPolicy(); - - continue; } catch (...) { LOG(ERROR) << "Exception occurred pid: " << req->GetSenderPID(); SendError(req); - if (type == pkgmgr_common::ReqType::CREATE_CACHE) - scheduler->ResetPolicy(); - - continue; } - std::vector result_data = handler[type]->ExtractResult(); - if (req->SendData(result_data.data(), result_data.size()) == false) { - LOG(ERROR) << "Failed to send response pid: " << req->GetSenderPID(); - continue; - } - - if (type == pkgmgr_common::ReqType::CREATE_CACHE) - scheduler->ResetPolicy(); - - LOG(WARNING) << "Success response pid: " << req->GetSenderPID(); + handler->PostExec(); } } @@ -232,20 +140,13 @@ gboolean WorkerThread::TrimMemory(void* data) { return G_SOURCE_REMOVE; } -std::shared_ptr WorkerThread::PopQueue() { - SetMemoryTrimTimer(); - auto req = queue_.front(); - queue_.pop(); - return req; -} - void WorkerThread::SetLocale(std::string locale) { LOG(DEBUG) << "Change locale : " << locale_.GetObject() << " -> " << locale; locale_.SetObject(std::move(locale)); } -void WorkerThread::SendError(std::shared_ptr req) { +void WorkerThread::SendError(const std::shared_ptr& req) { pkgmgr_common::parcel::AbstractParcelable parcelable( 0, pkgmgr_common::parcel::ParcelableType::Unknown, PMINFO_R_ERROR); tizen_base::Parcel p; diff --git a/src/server/worker_thread.hh b/src/server/worker_thread.hh index 4775cb8..c5ebcb2 100644 --- a/src/server/worker_thread.hh +++ b/src/server/worker_thread.hh @@ -40,37 +40,19 @@ namespace pkgmgr_server { class EXPORT_API WorkerThread { public: - WorkerThread(unsigned int num); + explicit WorkerThread(unsigned int num); ~WorkerThread(); bool PushQueue(std::shared_ptr req); void SetLocale(std::string locale); private: void Run(); - void SendError(std::shared_ptr req); + void SendError(const std::shared_ptr& req); void SetMemoryTrimTimer(); static gboolean TrimMemory(void* data); std::shared_ptr PopQueue(); private: - class Scheduler { - public: - Scheduler(pid_t tid); - - void ChangePolicy(); - void ResetPolicy(); - - private: - void Get(int* policy, struct sched_param* param); - int Set(int policy, struct sched_param* param); - - private: - pid_t tid_; - int policy_ = 0; - struct sched_param param_ = { 0, }; - }; - - private: bool stop_all_; utils::SharedObject locale_; std::queue> queue_; -- 2.7.4