[Filesystem] Use non-static worker instead of singleton TaskQueue

In filesystem webapi, TaskQueue is a singleton instance used by
multiple FilesystemInstance. With thread model web service
applications, there might be smack violation because each service
app (thread) try to use same worker.

This commit resolves problem by changing usage of singleton TaskQueue to
a member worker owned by FilesystemInstance.

Commit keeps two separated workers for better operation
performance:
- global_worker queues 'global' operations on fielsystem instance e.g.
opening files, rename, move etc.
- file_worker queues only file operation as reading
Above design prevents to block 'quick' global operations in case of
long-lasting file reading/writing operation in background.

[Verification] Code compiles without errors.
Filesystem TCT passrate 100%.

Change-Id: Idf0bd1915f67b64a65fcc1afc2e17d011bb5b918
Signed-off-by: DongHyun Song <dh81.song@samsung.com>
Signed-off-by: Piotr Kosko/Tizen API (PLT) /SRPOL/Engineer/Samsung Electronics <p.kosko@samsung.com>

diff --git a/src/filesystem/filesystem_instance.cc b/src/filesystem/filesystem_instance.cc
index 010a202..a07c7ce 100644 (file)
--- a/src/filesystem/filesystem_instance.cc
+++ b/src/filesystem/filesystem_instance.cc
@@ -158,7 +158,8 @@ FilesystemInstance::FilesystemInstance() {
 
 FilesystemInstance::~FilesystemInstance() {
   ScopeLogger();
-  worker.stop();
+  global_worker.stop();
+  file_worker.stop();
   FilesystemManager::GetInstance().StopListening();
   FilesystemManager::GetInstance().RemoveListener();
 }
@@ -232,7 +233,7 @@ void FilesystemInstance::FileRename(const picojson::value& args, picojson::objec
   };
 
   FilesystemManager& fsm = FilesystemManager::GetInstance();
-  common::TaskQueue::GetInstance().Queue(
+  global_worker.add_job(
       std::bind(&FilesystemManager::Rename, &fsm, oldPath, newPath, onSuccess, onError));
 }
 
@@ -838,7 +839,7 @@ void FilesystemInstance::FileStat(const picojson::value& args, picojson::object&
   };
 
   FilesystemManager& fsm = FilesystemManager::GetInstance();
-  common::TaskQueue::GetInstance().Async(
+  global_worker.add_job(
       std::bind(&FilesystemManager::StatPath, &fsm, location, onSuccess, onError));
 }
 
@@ -968,7 +969,7 @@ void FilesystemInstance::FileSystemManagerMakeDirectory(const picojson::value& a
     FilesystemManager::GetInstance().MakeDirectory(location, onResult);
   };
 
-  common::TaskQueue::GetInstance().Async(onAction);
+  global_worker.add_job(onAction);
 }
 
 void FilesystemInstance::FileSystemManagerMakeDirectorySync(const picojson::value& args,
@@ -1041,8 +1042,7 @@ void FilesystemInstance::FileReadDir(const picojson::value& args, picojson::obje
   };
 
   FilesystemManager& fm = FilesystemManager::GetInstance();
-  common::TaskQueue::GetInstance().Async(
-      std::bind(&FilesystemManager::ReadDir, &fm, pathToDir, onSuccess, onError));
+  global_worker.add_job(std::bind(&FilesystemManager::ReadDir, &fm, pathToDir, onSuccess, onError));
 }
 
 void FilesystemInstance::FileUnlinkFile(const picojson::value& args, picojson::object& out) {
@@ -1078,7 +1078,7 @@ void FilesystemInstance::FileUnlinkFile(const picojson::value& args, picojson::o
   };
 
   FilesystemManager& fm = FilesystemManager::GetInstance();
-  common::TaskQueue::GetInstance().Async(
+  global_worker.add_job(
       std::bind(&FilesystemManager::UnlinkFile, &fm, pathToFile, onSuccess, onError));
 }
 
@@ -1115,7 +1115,7 @@ void FilesystemInstance::FileRemoveDirectory(const picojson::value& args, picojs
   };
 
   FilesystemManager& fm = FilesystemManager::GetInstance();
-  common::TaskQueue::GetInstance().Queue(
+  global_worker.add_job(
       std::bind(&FilesystemManager::RemoveDirectory, &fm, pathToDelete, onSuccess, onError));
 }
 
@@ -1161,8 +1161,8 @@ void FilesystemInstance::FileCopyTo(const picojson::value& args, picojson::objec
   };
 
   FilesystemManager& fm = FilesystemManager::GetInstance();
-  common::TaskQueue::GetInstance().Queue(std::bind(&FilesystemManager::CopyTo, &fm, originPath,
-                                                   destinationPath, overwrite, onSuccess, onError));
+  global_worker.add_job(std::bind(&FilesystemManager::CopyTo, &fm, originPath, destinationPath,
+                                  overwrite, onSuccess, onError));
 }
 
 void FilesystemInstance::PrepareError(const FilesystemError& error, picojson::object& out) {
@@ -1331,7 +1331,7 @@ void FilesystemInstance::FileSystemManagerCreateDirectory(const picojson::value&
   bool make_parents = args.get("makeParents").get<bool>();
   CHECK_STORAGE_ACCESS(path, &out);
 
-  common::TaskQueue::GetInstance().Async([this, callback_id, path, make_parents] {
+  global_worker.add_job([this, callback_id, path, make_parents] {
     picojson::value response = picojson::value(picojson::object());
     picojson::object& obj = response.get<picojson::object>();
     obj["callbackId"] = picojson::value(callback_id);
@@ -1357,7 +1357,7 @@ void FilesystemInstance::FileSystemManagerDeleteFile(const picojson::value& args
   const std::string& path = args.get("path").get<std::string>();
 
   CHECK_STORAGE_ACCESS(path, &out);
-  common::TaskQueue::GetInstance().Async([this, callback_id, path] {
+  global_worker.add_job([this, callback_id, path] {
     picojson::value response = picojson::value(picojson::object());
     picojson::object& obj = response.get<picojson::object>();
     obj["callbackId"] = picojson::value(callback_id);
@@ -1392,7 +1392,7 @@ void FilesystemInstance::FileSystemManagerDeleteDirectory(const picojson::value&
   CHECK_STORAGE_ACCESS(path, &out);
   bool recursive = args.get("recursive").get<bool>();
 
-  common::TaskQueue::GetInstance().Queue([this, callback_id, path, recursive] {
+  global_worker.add_job([this, callback_id, path, recursive] {
     ScopeLogger();
     picojson::value response = picojson::value(picojson::object());
     picojson::object& obj = response.get<picojson::object>();
@@ -1434,7 +1434,7 @@ void FilesystemInstance::FileSystemManagerCopyFile(const picojson::value& args,
   CHECK_STORAGE_ACCESS(destination_path, &out);
   bool overwrite = args.get("overwrite").get<bool>();
 
-  common::TaskQueue::GetInstance().Queue([this, callback_id, path, destination_path, overwrite] {
+  global_worker.add_job([this, callback_id, path, destination_path, overwrite] {
     picojson::value response = picojson::value(picojson::object());
     picojson::object& obj = response.get<picojson::object>();
     obj["callbackId"] = picojson::value(callback_id);
@@ -1477,7 +1477,7 @@ void FilesystemInstance::FileSystemManagerCopyDirectory(const picojson::value& a
   double callback_id = args.get("callbackId").get<double>();
   bool overwrite = args.get("overwrite").get<bool>();
 
-  common::TaskQueue::GetInstance().Queue([this, callback_id, path, destination_path, overwrite] {
+  global_worker.add_job([this, callback_id, path, destination_path, overwrite] {
     ScopeLogger();
     picojson::value response = picojson::value(picojson::object());
     picojson::object& obj = response.get<picojson::object>();
@@ -1523,7 +1523,7 @@ void FilesystemInstance::FileSystemManagerMoveFile(const picojson::value& args,
   double callback_id = args.get("callbackId").get<double>();
   bool overwrite = args.get("overwrite").get<bool>();
 
-  common::TaskQueue::GetInstance().Queue([this, callback_id, path, destination_path, overwrite] {
+  global_worker.add_job([this, callback_id, path, destination_path, overwrite] {
     ScopeLogger();
     picojson::value response = picojson::value(picojson::object());
     picojson::object& obj = response.get<picojson::object>();
@@ -1575,7 +1575,7 @@ void FilesystemInstance::FileSystemManagerMoveDirectory(const picojson::value& a
   CHECK_STORAGE_ACCESS(destination_path, &out);
   bool overwrite = args.get("overwrite").get<bool>();
 
-  common::TaskQueue::GetInstance().Queue([this, callback_id, path, destination_path, overwrite] {
+  global_worker.add_job([this, callback_id, path, destination_path, overwrite] {
     picojson::value response = picojson::value(picojson::object());
     picojson::object& obj = response.get<picojson::object>();
     obj["callbackId"] = picojson::value(callback_id);
@@ -1622,7 +1622,7 @@ void FilesystemInstance::FileSystemManagerRename(const picojson::value& args,
   double callback_id = args.get("callbackId").get<double>();
   const std::string& new_name = args.get("newName").get<std::string>();
 
-  common::TaskQueue::GetInstance().Async([this, callback_id, new_name, path] {
+  global_worker.add_job([this, callback_id, new_name, path] {
     ScopeLogger();
     picojson::value response = picojson::value(picojson::object());
     picojson::object& obj = response.get<picojson::object>();
@@ -1679,7 +1679,7 @@ void FilesystemInstance::FileSystemManagerListDirectory(const picojson::value& a
   const picojson::object& filter = args.get("filter").get<picojson::object>();
   CHECK_STORAGE_ACCESS(path, &out);
 
-  common::TaskQueue::GetInstance().Async([this, callback_id, path, filter] {
+  global_worker.add_job([this, callback_id, path, filter] {
     ScopeLogger();
     picojson::value response = picojson::value(picojson::object());
     picojson::object& obj = response.get<picojson::object>();
@@ -1895,7 +1895,7 @@ void FilesystemInstance::FileHandleSeek(const picojson::value& args, picojson::o
   } else {
     // Async logic
     double callback_id = args.get("callbackId").get<double>();
-    this->worker.add_job([this, callback_id, logic] {
+    this->file_worker.add_job([this, callback_id, logic] {
       picojson::value response = picojson::value(picojson::object());
       picojson::object& async_out = response.get<picojson::object>();
       async_out["callbackId"] = picojson::value(callback_id);
@@ -1993,7 +1993,7 @@ void FilesystemInstance::FileHandleReadString(const picojson::value& args, picoj
   } else {
     // Async logic
     double callback_id = args.get("callbackId").get<double>();
-    this->worker.add_job([this, callback_id, logic] {
+    this->file_worker.add_job([this, callback_id, logic] {
       picojson::value response = picojson::value(picojson::object());
       picojson::object& async_out = response.get<picojson::object>();
       async_out["callbackId"] = picojson::value(callback_id);
@@ -2053,7 +2053,7 @@ void FilesystemInstance::FileHandleWriteString(const picojson::value& args, pico
   } else {
     // Async logic
     double callback_id = args.get("callbackId").get<double>();
-    this->worker.add_job([this, callback_id, logic] {
+    this->file_worker.add_job([this, callback_id, logic] {
       picojson::value response = picojson::value(picojson::object());
       picojson::object& async_out = response.get<picojson::object>();
       async_out["callbackId"] = picojson::value(callback_id);
@@ -2120,7 +2120,7 @@ void FilesystemInstance::FileHandleReadData(const picojson::value& args, picojso
   } else {
     // Async logic
     double callback_id = args.get("callbackId").get<double>();
-    this->worker.add_job([this, callback_id, logic] {
+    this->file_worker.add_job([this, callback_id, logic] {
       picojson::value response = picojson::value(picojson::object());
       picojson::object& async_out = response.get<picojson::object>();
       async_out["callbackId"] = picojson::value(callback_id);
@@ -2166,7 +2166,7 @@ void FilesystemInstance::FileHandleWriteData(const picojson::value& args, picojs
   } else {
     // Async logic
     double callback_id = args.get("callbackId").get<double>();
-    this->worker.add_job([this, callback_id, logic] {
+    this->file_worker.add_job([this, callback_id, logic] {
       picojson::value response = picojson::value(picojson::object());
       picojson::object& async_out = response.get<picojson::object>();
       async_out["callbackId"] = picojson::value(callback_id);
@@ -2207,7 +2207,7 @@ void FilesystemInstance::FileHandleFlush(const picojson::value& args, picojson::
   } else {
     // Async logic
     double callback_id = args.get("callbackId").get<double>();
-    this->worker.add_job([this, callback_id, logic] {
+    this->file_worker.add_job([this, callback_id, logic] {
       picojson::value response = picojson::value(picojson::object());
       picojson::object& async_out = response.get<picojson::object>();
       async_out["callbackId"] = picojson::value(callback_id);
@@ -2248,7 +2248,7 @@ void FilesystemInstance::FileHandleSync(const picojson::value& args, picojson::o
   } else {
     // Async logic
     double callback_id = args.get("callbackId").get<double>();
-    this->worker.add_job([this, callback_id, logic] {
+    this->file_worker.add_job([this, callback_id, logic] {
       picojson::value response = picojson::value(picojson::object());
       picojson::object& async_out = response.get<picojson::object>();
       async_out["callbackId"] = picojson::value(callback_id);
@@ -2294,18 +2294,18 @@ void FilesystemInstance::FileHandleClose(const picojson::value& args, picojson::
     bool done = false;
     std::mutex mutex;
     std::condition_variable conditional_variable;
-    // adding empty job to worker's queue, in order to wait for all jobs to be done before closing
-    // FILE*
-    this->worker.add_job([] {},
-                         [&conditional_variable, &mutex, &ready, &done, logic, &out] {
-                           // wait for close
-                           std::unique_lock<std::mutex> lock(mutex);
-                           conditional_variable.wait(lock, [&ready] { return ready; });
-
-                           logic(out);
-                           done = true;
-                           conditional_variable.notify_one();
-                         });
+    // adding empty job to file_worker's queue, in order to wait for all jobs to
+    // be done before closing FILE*
+    this->file_worker.add_job([] {},
+                              [&conditional_variable, &mutex, &ready, &done, logic, &out] {
+                                // wait for close
+                                std::unique_lock<std::mutex> lock(mutex);
+                                conditional_variable.wait(lock, [&ready] { return ready; });
+
+                                logic(out);
+                                done = true;
+                                conditional_variable.notify_one();
+                              });
 
     {
       // let know that close is ready
@@ -2315,7 +2315,7 @@ void FilesystemInstance::FileHandleClose(const picojson::value& args, picojson::
     conditional_variable.notify_one();
 
     {
-      // wait for worker
+      // wait for file_worker
       std::unique_lock<std::mutex> lock(mutex);
       conditional_variable.wait(lock, [&done] { return done; });
     }
@@ -2323,7 +2323,7 @@ void FilesystemInstance::FileHandleClose(const picojson::value& args, picojson::
   } else {
     // Async logic
     double callback_id = args.get("callbackId").get<double>();
-    this->worker.add_job([this, callback_id, logic] {
+    this->file_worker.add_job([this, callback_id, logic] {
       picojson::value response = picojson::value(picojson::object());
       picojson::object& async_out = response.get<picojson::object>();
       async_out["callbackId"] = picojson::value(callback_id);

diff --git a/src/filesystem/filesystem_instance.h b/src/filesystem/filesystem_instance.h
index 30ea02e..5a7ed81 100644 (file)
--- a/src/filesystem/filesystem_instance.h
+++ b/src/filesystem/filesystem_instance.h
@@ -55,7 +55,8 @@ class FilesystemInstance : public common::ParsedInstance, FilesystemStateChangeL
 
  private:
   FileHandleMap opened_files;
-  common::Worker worker;
+  common::Worker global_worker;
+  common::Worker file_worker;
 
   void FileCreateSync(const picojson::value& args, picojson::object& out);
   void FileRename(const picojson::value& args, picojson::object& out);