Refactor WorkerThread 99/274699/8
authorChanggyu Choi <changyu.choi@samsung.com>
Fri, 6 May 2022 08:21:51 +0000 (17:21 +0900)
committerChanggyu Choi <changyu.choi@samsung.com>
Thu, 12 May 2022 07:23:58 +0000 (16:23 +0900)
Changes:
 - Moves Scheduler to CreateCacheRequestHandler.
 - Adds PreExec() & PostExec() to RequestHandler steps.
 - Adds RequestHandlerFactory for loose coupling.

Change-Id: I0520f73164d335e6b5db97000db25c4132caad6d
Signed-off-by: Changgyu Choi <changyu.choi@samsung.com>
src/server/request_handler/abstract_request_handler.cc
src/server/request_handler/abstract_request_handler.hh
src/server/request_handler/create_cache_request_handler.cc
src/server/request_handler/create_cache_request_handler.hh
src/server/request_handler_factory.cc [new file with mode: 0644]
src/server/request_handler_factory.hh [new file with mode: 0644]
src/server/worker_thread.cc
src/server/worker_thread.hh

index 4f00260..bc33e01 100644 (file)
@@ -19,5 +19,11 @@ pid_t AbstractRequestHandler::GetPID() {
   return pid_;
 }
 
+void AbstractRequestHandler::PreExec() {
+}
+
+void AbstractRequestHandler::PostExec() {
+}
+
 }  // namespace request_handler
 }  // namespace pkgmgr_server
index 8285e80..8b463d4 100644 (file)
@@ -26,6 +26,8 @@ class EXPORT_API AbstractRequestHandler {
 
   virtual std::vector<uint8_t> ExtractResult() = 0;
 
+  virtual void PreExec();
+  virtual void PostExec();
   void SetPID(pid_t pid);
 
  protected:
index 4e60054..6cebec6 100644 (file)
@@ -7,12 +7,69 @@
 #include <string>
 
 #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_, &param_);
+}
+
+void CreateCacheRequestHandler::Scheduler::ChangePolicy() {
+  struct sched_param param {
+    .sched_priority = 1,
+  };
+  if (Set(SCHED_RR, &param) != 0)
+    return;
+
+  int policy = -1;
+  Get(&policy, &param);
+  LOG(WARNING) << "Current policy: " << policy
+               << ", priority: " << param.sched_priority;
+}
+
+void CreateCacheRequestHandler::Scheduler::ResetPolicy() {
+  if (Set(policy_, &param_) != 0)
+    return;
+
+  struct sched_param param = {
+    .sched_priority = 0,
+  };
+  int policy = -1;
+  Get(&policy, &param);
+  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<uint8_t> CreateCacheRequestHandler::ExtractResult() {
   return {};
 }
 
+void CreateCacheRequestHandler::PreExec() {
+  scheduler_.ChangePolicy();
+}
+
+void CreateCacheRequestHandler::PostExec() {
+  scheduler_.ResetPolicy();
+}
+
 }  // namespace request_handler
 }  // namespace pkgmgr_server
index 2c7a548..cee35c0 100644 (file)
@@ -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<uint8_t> 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<pkgmgr_common::parcel::ResultParcelable> 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 (file)
index 0000000..cdb3f1f
--- /dev/null
@@ -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<request_handler::AbstractRequestHandler>
+    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 (file)
index 0000000..5d693d5
--- /dev/null
@@ -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 <memory>
+
+#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<request_handler::AbstractRequestHandler> GetRequestHandler(
+      pkgmgr_common::ReqType type);
+
+ private:
+  std::shared_ptr<request_handler::AbstractRequestHandler>
+      handler[pkgmgr_common::ReqType::MAX];
+};
+
+}  // namespace pkgmgr_server
+
+#endif  // SERVER_REQUEST_HANDLER_FACTORY_
index c377a8f..0fae011 100644 (file)
 #include <malloc.h>
 
 #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"
 
 namespace pkgmgr_server {
 
-WorkerThread::Scheduler::Scheduler(pid_t tid) : tid_(tid) {
-  Get(&policy_, &param_);
-}
-
-void WorkerThread::Scheduler::ChangePolicy() {
-  struct sched_param param = { 0, };
-  param.sched_priority = 1;
-  if (Set(SCHED_RR, &param) != 0)
-    return;
-
-  int policy = -1;
-  Get(&policy, &param);
-  LOG(WARNING) << "Current policy: " << policy << ", priority: "
-               << param.sched_priority;
-}
-
-void WorkerThread::Scheduler::ResetPolicy() {
-  if (Set(policy_, &param_) != 0)
-    return;
-
-  struct sched_param param = { 0, };
-  int policy = -1;
-  Get(&policy, &param);
-  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<PkgRequest> req) {
   return true;
 }
 
-void WorkerThread::Run() {
-  std::unique_ptr<request_handler::AbstractRequestHandler>
-      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<PkgRequest> WorkerThread::PopQueue() {
+  SetMemoryTrimTimer();
+  std::unique_lock<std::mutex> u(lock_);
+  cv_.wait(u, [this] { return !this->queue_.empty() || stop_all_; });
+  if (stop_all_ && queue_.empty())
+    return nullptr;
 
-  std::unique_ptr<WorkerThread::Scheduler> 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<PkgRequest> req;
-    {
-      std::unique_lock<std::mutex> u(lock_);
-      cv_.wait(u, [this] { return !this->queue_.empty() || stop_all_; });
-      if (stop_all_ && queue_.empty())
-        return;
-      req = PopQueue();
-    }
+    std::shared_ptr<PkgRequest> 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<int>(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<uint8_t> 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<uint8_t> 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<PkgRequest> 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<PkgRequest> req) {
+void WorkerThread::SendError(const std::shared_ptr<PkgRequest>& req) {
   pkgmgr_common::parcel::AbstractParcelable parcelable(
       0, pkgmgr_common::parcel::ParcelableType::Unknown, PMINFO_R_ERROR);
   tizen_base::Parcel p;
index 4775cb8..c5ebcb2 100644 (file)
@@ -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<PkgRequest> req);
   void SetLocale(std::string locale);
 
  private:
   void Run();
-  void SendError(std::shared_ptr<PkgRequest> req);
+  void SendError(const std::shared_ptr<PkgRequest>& req);
   void SetMemoryTrimTimer();
   static gboolean TrimMemory(void* data);
   std::shared_ptr<PkgRequest> 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<std::string> locale_;
   std::queue<std::shared_ptr<PkgRequest>> queue_;