diff --git a/js_concurrent_module/taskpool/task.cpp b/js_concurrent_module/taskpool/task.cpp index da288836f5efdc6d4676bb3bf6cfd54bad2a1bc7..fdcd4976d9a4f93519bbb4317979e49e7f47b965 100644 --- a/js_concurrent_module/taskpool/task.cpp +++ b/js_concurrent_module/taskpool/task.cpp @@ -80,11 +80,13 @@ napi_value Task::TaskConstructor(napi_env env, napi_callback_info cbinfo) Task* task = GenerateTask(env, thisVar, func, name, args, argc); napi_status status = napi_wrap(env, thisVar, task, TaskDestructor, nullptr, nullptr); - if (status != napi_ok) { + if (status != napi_ok) { // LOCV_EXCL_BR_LINE HILOG_ERROR("taskpool::TaskConstructor napi_wrap return value is %{public}d", status); - TaskManager::GetInstance().RemoveTask(task->taskId_); - delete task; - task = nullptr; + task->SetValid(false); + if (TaskManager::GetInstance().RemoveTask(task->taskId_)) { + delete task; + task = nullptr; + } return nullptr; } napi_create_reference(env, thisVar, 0, &task->taskRef_); @@ -113,25 +115,33 @@ void Task::TaskDestructor(napi_env env, void* data, [[maybe_unused]] void* hint) if (!task->IsMainThreadTask()) { napi_remove_env_cleanup_hook(env, Task::CleanupHookFunc, task); } + task->SetValid(false); // for performance, do not lock first if (task->IsMainThreadTask() || task->lifecycleCount_ == 0) { TaskManager::GetInstance().ReleaseTaskData(env, task); napi_delete_reference(env, task->taskRef_); - delete task; + if (TaskManager::GetInstance().RemoveTask(task->taskId_)) { + delete task; + task = nullptr; + } else { // LOCV_EXCL_BR_LINE + HILOG_DEBUG("taskpool:: task may be in progress"); + } return; } bool shouldDelete = false; { std::lock_guard lock(task->taskMutex_); - task->SetValid(false); if (task->lifecycleCount_ == 0) { shouldDelete = true; } TaskManager::GetInstance().ReleaseTaskData(env, task, shouldDelete); napi_delete_reference(env, task->taskRef_); } - if (shouldDelete) { + if (shouldDelete && TaskManager::GetInstance().RemoveTask(task->taskId_)) { delete task; + task = nullptr; + } else { // LOCV_EXCL_BR_LINE + HILOG_DEBUG("taskpool:: task may be in progress"); } } diff --git a/js_concurrent_module/taskpool/task_group.cpp b/js_concurrent_module/taskpool/task_group.cpp index f5d61fe71a9d6222ebad2dbd2f92d6f8803db1b6..f42e153a7cedac72c4286364756ff815dc8a6bba 100644 --- a/js_concurrent_module/taskpool/task_group.cpp +++ b/js_concurrent_module/taskpool/task_group.cpp @@ -125,11 +125,13 @@ napi_value TaskGroup::AddTask(napi_env env, napi_callback_info cbinfo) } task->groupId_ = groupId; napi_status status = napi_wrap(env, napiTask, task, Task::TaskDestructor, nullptr, nullptr); - if (status != napi_ok) { + if (status != napi_ok) { // LOCV_EXCL_BR_LINE HILOG_ERROR("taskpool::AddTask napi_wrap return value is %{public}d", status); - TaskManager::GetInstance().RemoveTask(task->taskId_); - delete task; - task = nullptr; + task->SetValid(false); + if (TaskManager::GetInstance().RemoveTask(task->taskId_)) { + delete task; + task = nullptr; + } return nullptr; } napi_create_reference(env, napiTask, 1, &task->taskRef_); diff --git a/js_concurrent_module/taskpool/task_manager.cpp b/js_concurrent_module/taskpool/task_manager.cpp index 087eca56e75f94a867bded18785cc38908245168..f30c9bb36d8423cc8825d55d3e486c5f05451192 100644 --- a/js_concurrent_module/taskpool/task_manager.cpp +++ b/js_concurrent_module/taskpool/task_manager.cpp @@ -132,6 +132,7 @@ TaskManager::~TaskManager() task = nullptr; } tasks_.clear(); + runningTasks_.clear(); } CountTraceForWorker(); } @@ -1391,11 +1392,6 @@ void TaskManager::TerminateTask(uint32_t taskId) void TaskManager::ReleaseTaskData(napi_env env, Task* task, bool shouldDeleteTask) { - uint32_t taskId = task->taskId_; - if (shouldDeleteTask) { - RemoveTask(taskId); - } - task->ReleaseData(); task->CancelPendingTask(env); @@ -1407,6 +1403,7 @@ void TaskManager::ReleaseTaskData(napi_env env, Task* task, bool shouldDeleteTas if (!task->IsMainThreadTask()) { task->SetValid(false); } + uint32_t taskId = task->taskId_; DecreaseSendDataRefCount(env, taskId, task); RemoveTaskDuration(taskId); RemovePendingTaskInfo(taskId); @@ -1466,10 +1463,23 @@ void TaskManager::StoreTask(Task* task) tasks_.emplace(taskId, task); } -void TaskManager::RemoveTask(uint32_t taskId) +bool TaskManager::RemoveTask(uint32_t taskId) { std::lock_guard lock(tasksMutex_); + bool res = true; + auto runningIter = runningTasks_.find(taskId); + if (runningIter != runningTasks_.end()) { + res = false; + } + runningTasks_.erase(taskId); tasks_.erase(taskId); + return res; +} + +void TaskManager::RemoveRunningTask(uint32_t taskId) +{ + std::lock_guard lock(tasksMutex_); + runningTasks_.erase(taskId); } Task* TaskManager::GetTask(uint32_t taskId) @@ -1482,6 +1492,17 @@ Task* TaskManager::GetTask(uint32_t taskId) return iter->second; } +Task* TaskManager::GetTaskForPerform(uint32_t taskId) +{ + std::lock_guard lock(tasksMutex_); + auto iter = tasks_.find(taskId); + if (iter == tasks_.end()) { + return nullptr; + } + runningTasks_.emplace(taskId, iter->second); + return iter->second; +} + #if defined(ENABLE_TASKPOOL_FFRT) void TaskManager::UpdateSystemAppFlag() { @@ -1588,10 +1609,10 @@ void TaskManager::RemoveDependentTaskByTaskId(uint32_t taskId) if (task->currentTaskInfo_ != nullptr && EraseWaitingTaskId(task->taskId_, task->currentTaskInfo_->priority)) { delete task->currentTaskInfo_; task->currentTaskInfo_ = nullptr; + task->DecreaseTaskLifecycleCount(); } if (task->currentTaskInfo_ == nullptr) { reinterpret_cast(task->env_)->DecreaseSubEnvCounter(); - task->DecreaseTaskLifecycleCount(); DecreaseSendDataRefCount(task->env_, task->taskId_, task); napi_reference_unref(task->env_, task->taskRef_, nullptr); } diff --git a/js_concurrent_module/taskpool/task_manager.h b/js_concurrent_module/taskpool/task_manager.h index a8037fe2831aeece93607461bcdea085a219ecdc..bb7eeaa21b3e1ac89463cde16826363a3e5522cd 100644 --- a/js_concurrent_module/taskpool/task_manager.h +++ b/js_concurrent_module/taskpool/task_manager.h @@ -60,8 +60,10 @@ public: static TaskManager& GetInstance(); void StoreTask(Task* task); - void RemoveTask(uint32_t taskId); + bool RemoveTask(uint32_t taskId); + void RemoveRunningTask(uint32_t taskId); Task* GetTask(uint32_t taskId); + Task* GetTaskForPerform(uint32_t taskId); void EnqueueTaskId(uint32_t taskId, Priority priority = Priority::DEFAULT); bool EraseWaitingTaskId(uint32_t taskId, Priority priority); std::pair DequeueTaskId(); @@ -198,6 +200,7 @@ private: // std::unordered_map tasks_ {}; + std::unordered_map runningTasks_ {}; std::recursive_mutex tasksMutex_; // >, update when removeDependency or executeTask diff --git a/js_concurrent_module/taskpool/taskpool.cpp b/js_concurrent_module/taskpool/taskpool.cpp index 3f81c4ae040d8c2c4e99fc19c4c87c18e87007be..34969a67c8887e176306eef132a927cd628811ef 100644 --- a/js_concurrent_module/taskpool/taskpool.cpp +++ b/js_concurrent_module/taskpool/taskpool.cpp @@ -236,9 +236,11 @@ napi_value TaskPool::Execute(napi_env env, napi_callback_info cbinfo) napi_value promise = NapiHelper::CreatePromise(env, &task->currentTaskInfo_->deferred); if (promise == nullptr) { // LOCV_EXCL_BR_LINE task->ReleaseData(); - TaskManager::GetInstance().RemoveTask(task->GetTaskId()); - delete task; - task = nullptr; + task->SetValid(false); + if (TaskManager::GetInstance().RemoveTask(task->GetTaskId())) { + delete task; + task = nullptr; + } std::string err = "create promise failed, maybe has exception."; ErrorHelper::ThrowError(env, ErrorHelper::TYPE_ERROR, err.c_str()); HILOG_ERROR("taskpool:: Execute %{public}s", err.c_str()); @@ -418,6 +420,7 @@ void TaskPool::HandleTaskResult(Task* task) { HILOG_DEBUG("taskpool:: HandleTaskResult task"); HITRACE_HELPER_METER_NAME(__PRETTY_FUNCTION__); + TaskManager::GetInstance().RemoveRunningTask(task->taskId_); // update task execution info if (!task->IsMainThreadTask()) { if (task->ShouldDeleteTask(false)) { delete task; diff --git a/js_concurrent_module/taskpool/test/test_taskpool.cpp b/js_concurrent_module/taskpool/test/test_taskpool.cpp index 475328f2976ce784b5e989459d2d1a965edc3e7b..b23a79d28a1b01a5958db2c500289379d6f95bbc 100644 --- a/js_concurrent_module/taskpool/test/test_taskpool.cpp +++ b/js_concurrent_module/taskpool/test/test_taskpool.cpp @@ -7061,4 +7061,25 @@ HWTEST_F(NativeEngineTest, TaskpoolTest340, testing::ext::TestSize.Level0) napi_value exception = nullptr; napi_get_and_clear_last_exception(env, &exception); ASSERT_TRUE(exception == nullptr); +} + +HWTEST_F(NativeEngineTest, TaskpoolTest341, testing::ext::TestSize.Level0) +{ + napi_env env = (napi_env)engine_; + ExceptionScope scope(env); + TaskManager &taskManager = TaskManager::GetInstance(); + + uint32_t taskId = 341; + Task* task1 = taskManager.GetTaskForPerform(taskId); + ASSERT_TRUE(task1 == nullptr); + Task* task2 = new Task(); + taskManager.StoreTask(task2); + Task* res1 = taskManager.GetTaskForPerform(task2->taskId_); + ASSERT_EQ(res1, task2); + taskManager.RemoveRunningTask(task2->taskId_); + bool isFinished = taskManager.RemoveTask(task2->taskId_); + ASSERT_TRUE(isFinished); + Task* res2 = taskManager.GetTaskForPerform(task2->taskId_); + ASSERT_TRUE(res2 == nullptr); + delete task2; } \ No newline at end of file diff --git a/js_concurrent_module/taskpool/worker.cpp b/js_concurrent_module/taskpool/worker.cpp index 52033a403f3f0bc4a097015b3809d604d83eb215..245db9983190f72c7e9f89738e797390b49ed823 100644 --- a/js_concurrent_module/taskpool/worker.cpp +++ b/js_concurrent_module/taskpool/worker.cpp @@ -430,7 +430,7 @@ void Worker::PerformTask(const uv_async_t* req) RunningScope runningScope(worker); WorkerRunningScope workerRunningScope(env); PriorityScope priorityScope(worker, taskInfo.second); - Task* task = TaskManager::GetInstance().GetTask(taskInfo.first); + Task* task = TaskManager::GetInstance().GetTaskForPerform(taskInfo.first); if (task == nullptr) { HILOG_DEBUG("taskpool:: task has been released"); return;