From 92b2155ff0c4a12306c418f7824a278f6e10ff5a Mon Sep 17 00:00:00 2001 From: "Piotr Kosko/Tizen API (PLT) /SRPOL/Engineer/Samsung Electronics" Date: Tue, 8 Jun 2021 07:41:41 +0200 Subject: [PATCH] [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 Signed-off-by: Piotr Kosko/Tizen API (PLT) /SRPOL/Engineer/Samsung Electronics --- src/filesystem/filesystem_instance.cc | 80 +++++++++++++++++------------------ src/filesystem/filesystem_instance.h | 3 +- 2 files changed, 42 insertions(+), 41 deletions(-) diff --git a/src/filesystem/filesystem_instance.cc b/src/filesystem/filesystem_instance.cc index 010a202..a07c7ce 100644 --- 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(); 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(); 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(); 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(); 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(); - 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(); @@ -1434,7 +1434,7 @@ void FilesystemInstance::FileSystemManagerCopyFile(const picojson::value& args, CHECK_STORAGE_ACCESS(destination_path, &out); bool overwrite = args.get("overwrite").get(); - 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(); obj["callbackId"] = picojson::value(callback_id); @@ -1477,7 +1477,7 @@ void FilesystemInstance::FileSystemManagerCopyDirectory(const picojson::value& a double callback_id = args.get("callbackId").get(); bool overwrite = args.get("overwrite").get(); - 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(); @@ -1523,7 +1523,7 @@ void FilesystemInstance::FileSystemManagerMoveFile(const picojson::value& args, double callback_id = args.get("callbackId").get(); bool overwrite = args.get("overwrite").get(); - 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(); @@ -1575,7 +1575,7 @@ void FilesystemInstance::FileSystemManagerMoveDirectory(const picojson::value& a CHECK_STORAGE_ACCESS(destination_path, &out); bool overwrite = args.get("overwrite").get(); - 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(); obj["callbackId"] = picojson::value(callback_id); @@ -1622,7 +1622,7 @@ void FilesystemInstance::FileSystemManagerRename(const picojson::value& args, double callback_id = args.get("callbackId").get(); const std::string& new_name = args.get("newName").get(); - 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(); @@ -1679,7 +1679,7 @@ void FilesystemInstance::FileSystemManagerListDirectory(const picojson::value& a const picojson::object& filter = args.get("filter").get(); 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(); @@ -1895,7 +1895,7 @@ void FilesystemInstance::FileHandleSeek(const picojson::value& args, picojson::o } else { // Async logic double callback_id = args.get("callbackId").get(); - 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(); 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(); - 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(); 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(); - 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(); 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(); - 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(); 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(); - 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(); 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(); - 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(); 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(); - 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(); 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 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 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 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(); - 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(); 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 --- 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); -- 2.7.4