From 31676f6201f774419e2022561715aed0da18cd9b Mon Sep 17 00:00:00 2001
From: Andrew Gaul <andrew@gaul.org>
Date: Wed, 13 Mar 2024 21:27:12 +0900
Subject: [PATCH] Convert thpoolman_param to value (#2430)

This simplifies memory management.
---
 src/fdcache_fdinfo.cpp | 10 +++++-----
 src/threadpoolman.cpp  | 45 ++++++++++++++++--------------------------
 src/threadpoolman.h    |  7 +++----
 3 files changed, 25 insertions(+), 37 deletions(-)

diff --git a/src/fdcache_fdinfo.cpp b/src/fdcache_fdinfo.cpp
index c71dbb7114..3cbf37677d 100644
--- a/src/fdcache_fdinfo.cpp
+++ b/src/fdcache_fdinfo.cpp
@@ -449,13 +449,13 @@ bool PseudoFdInfo::ParallelMultipartUpload(const char* path, const mp_part_list_
         thargs->petag                = petag;
 
         // make parameter for thread pool
-        std::unique_ptr<thpoolman_param> ppoolparam(new thpoolman_param);
-        ppoolparam->args             = thargs;
-        ppoolparam->psem             = &uploaded_sem;
-        ppoolparam->pfunc            = PseudoFdInfo::MultipartUploadThreadWorker;
+        thpoolman_param ppoolparam;
+        ppoolparam.args             = thargs;
+        ppoolparam.psem             = &uploaded_sem;
+        ppoolparam.pfunc            = PseudoFdInfo::MultipartUploadThreadWorker;
 
         // setup instruction
-        if(!ThreadPoolMan::Instruct(std::move(ppoolparam))){
+        if(!ThreadPoolMan::Instruct(ppoolparam)){
             S3FS_PRN_ERR("failed setup instruction for uploading.");
             delete thargs;
             return false;
diff --git a/src/threadpoolman.cpp b/src/threadpoolman.cpp
index 682529d761..f2eab02bfe 100644
--- a/src/threadpoolman.cpp
+++ b/src/threadpoolman.cpp
@@ -53,13 +53,14 @@ void ThreadPoolMan::Destroy()
     }
 }
 
-bool ThreadPoolMan::Instruct(std::unique_ptr<thpoolman_param> pparam)
+bool ThreadPoolMan::Instruct(const thpoolman_param& param)
 {
     if(!ThreadPoolMan::singleton){
         S3FS_PRN_WARN("The singleton object is not initialized yet.");
         return false;
     }
-    return ThreadPoolMan::singleton->SetInstruction(std::move(pparam));
+    ThreadPoolMan::singleton->SetInstruction(param);
+    return true;
 }
 
 //
@@ -84,30 +85,25 @@ void* ThreadPoolMan::Worker(void* arg)
         }
 
         // get instruction
-        std::unique_ptr<thpoolman_param> pparam;
+        thpoolman_param param;
         {
             AutoLock auto_lock(&(psingleton->thread_list_lock));
 
-            if(!psingleton->instruction_list.empty()){
-                pparam = std::move(psingleton->instruction_list.front());
-                psingleton->instruction_list.pop_front();
-                if(!pparam){
-                    S3FS_PRN_WARN("Got a semaphore, but the instruction is empty.");
-                }
+            if(psingleton->instruction_list.empty()){
+                S3FS_PRN_DBG("Got a semaphore, but the instruction is empty.");
+                continue;
             }else{
-                S3FS_PRN_WARN("Got a semaphore, but there is no instruction.");
-                pparam = nullptr;
+                param = psingleton->instruction_list.front();
+                psingleton->instruction_list.pop_front();
             }
         }
 
-        if(pparam){
-            void* retval     = pparam->pfunc(pparam->args);
-            if(nullptr != retval){
-                S3FS_PRN_WARN("The instruction function returned with somthign error code(%ld).", reinterpret_cast<long>(retval));
-            }
-            if(pparam->psem){
-                pparam->psem->post();
-            }
+        void* retval = param.pfunc(param.args);
+        if(nullptr != retval){
+            S3FS_PRN_WARN("The instruction function returned with somthign error code(%ld).", reinterpret_cast<long>(retval));
+        }
+        if(param.psem){
+            param.psem->post();
         }
     }
 
@@ -235,23 +231,16 @@ bool ThreadPoolMan::StartThreads(int count)
     return true;
 }
 
-bool ThreadPoolMan::SetInstruction(std::unique_ptr<thpoolman_param> pparam)
+void ThreadPoolMan::SetInstruction(const thpoolman_param& param)
 {
-    if(!pparam){
-        S3FS_PRN_ERR("The parameter value is nullptr.");
-        return false;
-    }
-
     // set parameter to list
     {
         AutoLock auto_lock(&thread_list_lock);
-        instruction_list.push_back(std::move(pparam));
+        instruction_list.push_back(param);
     }
 
     // run thread
     thpoolman_sem.post();
-
-    return true;
 }
 
 /*
diff --git a/src/threadpoolman.h b/src/threadpoolman.h
index 675f374b04..6fb65d95a5 100644
--- a/src/threadpoolman.h
+++ b/src/threadpoolman.h
@@ -23,7 +23,6 @@
 
 #include <atomic>
 #include <list>
-#include <memory>
 #include <vector>
 
 #include "psemaphore.h"
@@ -53,7 +52,7 @@ struct thpoolman_param
     thpoolman_param() : args(nullptr), psem(nullptr), pfunc(nullptr) {}
 };
 
-typedef std::list<std::unique_ptr<thpoolman_param>> thpoolman_params_t;
+typedef std::list<thpoolman_param> thpoolman_params_t;
 
 typedef std::vector<pthread_t> thread_list_t;
 
@@ -89,12 +88,12 @@ class ThreadPoolMan
 
         bool StopThreads();
         bool StartThreads(int count);
-        bool SetInstruction(std::unique_ptr<thpoolman_param> pparam);
+        void SetInstruction(const thpoolman_param& pparam);
 
     public:
         static bool Initialize(int count);
         static void Destroy();
-        static bool Instruct(std::unique_ptr<thpoolman_param> pparam);
+        static bool Instruct(const thpoolman_param& pparam);
 };
 
 #endif // S3FS_THREADPOOLMAN_H_