From e06c2d1242b9212dc7d830dc302cdfd5a33e49fd Mon Sep 17 00:00:00 2001 From: lukai Date: Mon, 18 Jul 2022 19:06:58 +0800 Subject: [PATCH] workerxts bugfix 1. jspandfile used after free 2. icupath multi-thread problems 3. mmappool used after free 4. condition variable has waiters when delete it issue:https://gitee.com/openharmony/arkcompiler_ets_runtime/issues/I5FVRD?from=project-issue Signed-off-by: lukai --- ecmascript/builtins.cpp | 46 ------------------- ecmascript/builtins.h | 3 -- ecmascript/ecma_vm.cpp | 1 + ecmascript/file_loader.cpp | 23 ++++++++++ ecmascript/file_loader.h | 3 ++ .../jspandafile/js_pandafile_manager.cpp | 37 ++++++++------- ecmascript/jspandafile/js_pandafile_manager.h | 4 +- ecmascript/mem/heap.cpp | 19 ++++++-- ecmascript/mem/heap.h | 1 + ecmascript/mem/mem_map_allocator.h | 2 +- ecmascript/napi/include/jsnapi.h | 1 + ecmascript/napi/jsnapi.cpp | 19 ++++++++ 12 files changed, 85 insertions(+), 74 deletions(-) diff --git a/ecmascript/builtins.cpp b/ecmascript/builtins.cpp index 38242dcb..ad99651c 100644 --- a/ecmascript/builtins.cpp +++ b/ecmascript/builtins.cpp @@ -176,33 +176,6 @@ using CjsRequire = builtins::BuiltinsCjsRequire; using ContainersPrivate = containers::ContainersPrivate; using SharedArrayBuffer = builtins::BuiltinsSharedArrayBuffer; -bool GetAbsolutePath(const std::string &relativePath, std::string &absPath) -{ - if (relativePath.size() >= PATH_MAX) { - return false; - } - char buffer[PATH_MAX] = {0}; -#ifndef PANDA_TARGET_WINDOWS - auto path = realpath(relativePath.c_str(), buffer); - if (path == nullptr) { - return false; - } - absPath = std::string(path); - return true; -#else - auto path = _fullpath(buffer, relativePath.c_str(), sizeof(buffer) - 1); - if (path == nullptr) { - return false; - } - bool valid = PathCanonicalizeA(buffer, path); - if (!valid) { - return false; - } - absPath = std::string(buffer); - return true; -#endif -} - void Builtins::Initialize(const JSHandle &env, JSThread *thread) { thread_ = thread; @@ -351,7 +324,6 @@ void Builtins::Initialize(const JSHandle &env, JSThread *thread) InitializePluralRules(env); InitializeDisplayNames(env); InitializeListFormat(env); - InitializeIcuData(); InitializeModuleNamespace(env, objFuncDynclass); InitializeCjsModule(env); @@ -370,7 +342,6 @@ void Builtins::InitializeForSnapshot(JSThread *thread) vm_ = thread->GetEcmaVM(); factory_ = vm_->GetFactory(); - InitializeIcuData(); // Initialize ArkTools JSRuntimeOptions options = vm_->GetJSOptions(); if (options.EnableArkTools()) { @@ -3353,21 +3324,4 @@ void Builtins::InitializeCjsRequire(const JSHandle &env) const env->SetCjsRequireFunction(thread_, cjsRequireFunction); } - -void Builtins::InitializeIcuData() -{ - ASSERT(vm_ != nullptr); - JSRuntimeOptions options = vm_->GetJSOptions(); - std::string icuPath = options.GetIcuDataPath(); - if (icuPath == "default") { - if (!WIN_OR_MAC_PLATFORM) { - SetHwIcuDirectory(); - } - } else { - std::string absPath; - if (GetAbsolutePath(icuPath, absPath)) { - u_setDataDirectory(absPath.c_str()); - } - } -} } // namespace panda::ecmascript diff --git a/ecmascript/builtins.h b/ecmascript/builtins.h index 47406464..e2b8abb2 100644 --- a/ecmascript/builtins.h +++ b/ecmascript/builtins.h @@ -133,9 +133,6 @@ private: void InitializeDisplayNames(const JSHandle &env); void InitializeListFormat(const JSHandle &env); - // Initialize IcuData Path - void InitializeIcuData(); - void GeneralUpdateError(ErrorParameter *error, EcmaEntrypoint constructor, EcmaEntrypoint method, const char *name, JSType type) const; diff --git a/ecmascript/ecma_vm.cpp b/ecmascript/ecma_vm.cpp index 36cbe22b..be26db60 100644 --- a/ecmascript/ecma_vm.cpp +++ b/ecmascript/ecma_vm.cpp @@ -246,6 +246,7 @@ EcmaVM::~EcmaVM() { LOG(INFO, RUNTIME) << "Destruct ecma_vm, vm address is: " << this; vmInitialized_ = false; + heap_->WaitAllTasksFinished(); Taskpool::GetCurrentTaskpool()->Destroy(); if (runtimeStat_ != nullptr && runtimeStat_->IsRuntimeStatEnabled()) { diff --git a/ecmascript/file_loader.cpp b/ecmascript/file_loader.cpp index 953ca202..45c6ad47 100644 --- a/ecmascript/file_loader.cpp +++ b/ecmascript/file_loader.cpp @@ -267,6 +267,29 @@ void FileLoader::LoadAOTFile(const std::string &fileName) AddAOTPackInfo(aotPackInfo_); } +bool FileLoader::GetAbsolutePath(const std::string &relativePath, std::string &absPath) +{ + if (relativePath.size() >= PATH_MAX) { + return false; + } + char buffer[PATH_MAX] = {0}; +#ifndef PANDA_TARGET_WINDOWS + auto path = realpath(relativePath.c_str(), buffer); + if (path == nullptr) { + return false; + } + absPath = std::string(path); + return true; +#else + auto path = _fullpath(buffer, relativePath.c_str(), sizeof(buffer) - 1); + if (path == nullptr) { + return false; + } + absPath = std::string(buffer); + return true; +#endif +} + void FileLoader::TryLoadSnapshotFile() { const CString snapshotPath(vm_->GetJSOptions().GetSnapshotOutputFile().c_str()); diff --git a/ecmascript/file_loader.h b/ecmascript/file_loader.h index e4cc1c3c..e17df3c0 100644 --- a/ecmascript/file_loader.h +++ b/ecmascript/file_loader.h @@ -224,6 +224,9 @@ public: ~FileLoader() = default; void LoadStubFile(const std::string &fileName); void LoadAOTFile(const std::string &fileName); + + static bool GetAbsolutePath(const std::string &relativePath, std::string &absPath); + void AddAOTPackInfo(AOTModulePackInfo packInfo) { aotPackInfos_.emplace_back(packInfo); diff --git a/ecmascript/jspandafile/js_pandafile_manager.cpp b/ecmascript/jspandafile/js_pandafile_manager.cpp index 0ca7ae93..712e5942 100644 --- a/ecmascript/jspandafile/js_pandafile_manager.cpp +++ b/ecmascript/jspandafile/js_pandafile_manager.cpp @@ -70,10 +70,13 @@ const JSPandaFile *JSPandaFileManager::LoadJSPandaFile(JSThread *thread, const C std::string_view entryPoint) { ECMA_BYTRACE_NAME(HITRACE_TAG_ARK, "JSPandaFileManager::LoadJSPandaFile"); - const JSPandaFile *jsPandaFile = FindJSPandaFile(filename); - if (jsPandaFile != nullptr) { - IncreaseRefJSPandaFile(jsPandaFile); - return jsPandaFile; + { + os::memory::LockHolder lock(jsPandaFileLock_); + const JSPandaFile *jsPandaFile = FindJSPandaFileUnlocked(filename); + if (jsPandaFile != nullptr) { + IncreaseRefJSPandaFileUnlocked(jsPandaFile); + return jsPandaFile; + } } auto pf = panda_file::OpenPandaFileOrZip(filename, panda_file::File::READ_WRITE); @@ -82,7 +85,7 @@ const JSPandaFile *JSPandaFileManager::LoadJSPandaFile(JSThread *thread, const C return nullptr; } - jsPandaFile = GenerateJSPandaFile(thread, pf.release(), filename, entryPoint); + const JSPandaFile *jsPandaFile = GenerateJSPandaFile(thread, pf.release(), filename, entryPoint); return jsPandaFile; } @@ -92,11 +95,13 @@ const JSPandaFile *JSPandaFileManager::LoadJSPandaFile(JSThread *thread, const C if (buffer == nullptr || size == 0) { return nullptr; } - - const JSPandaFile *jsPandaFile = FindJSPandaFile(filename); - if (jsPandaFile != nullptr) { - IncreaseRefJSPandaFile(jsPandaFile); - return jsPandaFile; + { + os::memory::LockHolder lock(jsPandaFileLock_); + const JSPandaFile *jsPandaFile = FindJSPandaFileUnlocked(filename); + if (jsPandaFile != nullptr) { + IncreaseRefJSPandaFileUnlocked(jsPandaFile); + return jsPandaFile; + } } auto pf = panda_file::OpenPandaFileFromMemory(buffer, size); @@ -104,7 +109,7 @@ const JSPandaFile *JSPandaFileManager::LoadJSPandaFile(JSThread *thread, const C LOG_ECMA(ERROR) << "open file " << filename << " error"; return nullptr; } - jsPandaFile = GenerateJSPandaFile(thread, pf.release(), filename, entryPoint); + const JSPandaFile *jsPandaFile = GenerateJSPandaFile(thread, pf.release(), filename, entryPoint); return jsPandaFile; } @@ -118,12 +123,11 @@ JSHandle JSPandaFileManager::GenerateProgram(EcmaVM *vm, const JSPandaF return program; } -const JSPandaFile *JSPandaFileManager::FindJSPandaFile(const CString &filename) +const JSPandaFile *JSPandaFileManager::FindJSPandaFileUnlocked(const CString &filename) { if (filename.empty()) { return nullptr; } - os::memory::LockHolder lock(jsPandaFileLock_); for (const auto &iter : loadedJSPandaFiles_) { const JSPandaFile *pf = iter.first; if (pf->GetJSPandaFileDesc() == filename) { @@ -154,11 +158,10 @@ void JSPandaFileManager::InsertJSPandaFile(const JSPandaFile *jsPandaFile) loadedJSPandaFiles_[jsPandaFile] = 1; } -void JSPandaFileManager::IncreaseRefJSPandaFile(const JSPandaFile *jsPandaFile) +void JSPandaFileManager::IncreaseRefJSPandaFileUnlocked(const JSPandaFile *jsPandaFile) { LOG_ECMA(INFO) << "IncreaseRefJSPandaFile " << jsPandaFile->GetJSPandaFileDesc(); - os::memory::LockHolder lock(jsPandaFileLock_); ASSERT(loadedJSPandaFiles_.find(jsPandaFile) != loadedJSPandaFiles_.end()); loadedJSPandaFiles_[jsPandaFile]++; } @@ -247,9 +250,9 @@ const JSPandaFile *JSPandaFileManager::GenerateJSPandaFile(JSThread *thread, con PandaFileTranslator::TranslateClasses(newJsPandaFile, methodName); { os::memory::LockHolder lock(jsPandaFileLock_); - const JSPandaFile *jsPandaFile = FindJSPandaFile(desc); + const JSPandaFile *jsPandaFile = FindJSPandaFileUnlocked(desc); if (jsPandaFile != nullptr) { - IncreaseRefJSPandaFile(jsPandaFile); + IncreaseRefJSPandaFileUnlocked(jsPandaFile); ReleaseJSPandaFile(newJsPandaFile); return jsPandaFile; } diff --git a/ecmascript/jspandafile/js_pandafile_manager.h b/ecmascript/jspandafile/js_pandafile_manager.h index dc817f14..3348a95c 100644 --- a/ecmascript/jspandafile/js_pandafile_manager.h +++ b/ecmascript/jspandafile/js_pandafile_manager.h @@ -82,9 +82,9 @@ private: std::string_view entryPoint); void ReleaseJSPandaFile(const JSPandaFile *jsPandaFile); const JSPandaFile *GetJSPandaFile(const panda_file::File *pf); - const JSPandaFile *FindJSPandaFile(const CString &filename); + const JSPandaFile *FindJSPandaFileUnlocked(const CString &filename); void InsertJSPandaFile(const JSPandaFile *jsPandaFile); - void IncreaseRefJSPandaFile(const JSPandaFile *jsPandaFile); + void IncreaseRefJSPandaFileUnlocked(const JSPandaFile *jsPandaFile); void DecreaseRefJSPandaFile(const JSPandaFile *jsPandaFile); static void *AllocateBuffer(size_t size); diff --git a/ecmascript/mem/heap.cpp b/ecmascript/mem/heap.cpp index 776eb907..42b32857 100644 --- a/ecmascript/mem/heap.cpp +++ b/ecmascript/mem/heap.cpp @@ -126,7 +126,10 @@ void Heap::Initialize() void Heap::Destroy() { - Prepare(); + if (workManager_ != nullptr) { + delete workManager_; + workManager_ = nullptr; + } if (activeSemiSpace_ != nullptr) { activeSemiSpace_->Destroy(); delete activeSemiSpace_; @@ -167,10 +170,6 @@ void Heap::Destroy() delete hugeObjectSpace_; hugeObjectSpace_ = nullptr; } - if (workManager_ != nullptr) { - delete workManager_; - workManager_ = nullptr; - } if (stwYoungGC_ != nullptr) { delete stwYoungGC_; stwYoungGC_ = nullptr; @@ -624,6 +623,16 @@ void Heap::WaitClearTaskFinished() } } +void Heap::WaitAllTasksFinished() +{ + WaitRunningTaskFinished(); + sweeper_->EnsureAllTaskFinished(); + WaitClearTaskFinished(); + if (concurrentMarker_->IsEnabled() && thread_->IsMarking()) { + concurrentMarker_->WaitMarkingFinished(); + } +} + void Heap::WaitConcurrentMarkingFinished() { concurrentMarker_->WaitMarkingFinished(); diff --git a/ecmascript/mem/heap.h b/ecmascript/mem/heap.h index ede9d15e..4dcc9288 100644 --- a/ecmascript/mem/heap.h +++ b/ecmascript/mem/heap.h @@ -310,6 +310,7 @@ public: inline void ClearSlotsRange(Region *current, uintptr_t freeStart, uintptr_t freeEnd); + void WaitAllTasksFinished(); void WaitConcurrentMarkingFinished(); MemGrowingType GetMemGrowingType() const diff --git a/ecmascript/mem/mem_map_allocator.h b/ecmascript/mem/mem_map_allocator.h index 77a0013d..82f9efb2 100644 --- a/ecmascript/mem/mem_map_allocator.h +++ b/ecmascript/mem/mem_map_allocator.h @@ -169,9 +169,9 @@ public: if (iterate == freeList_.end()) { return MemMap(); } - freeList_.erase(iterate); MemMap memMap = iterate->second; size_t remainderSize = memMap.GetSize() - size; + freeList_.erase(iterate); if (remainderSize >= DEFAULT_REGION_SIZE) { auto next = reinterpret_cast(reinterpret_cast(memMap.GetMem()) + size); freeList_.insert(std::pair(remainderSize, MemMap(next, remainderSize))); diff --git a/ecmascript/napi/include/jsnapi.h b/ecmascript/napi/include/jsnapi.h index 8541310d..54ef92da 100644 --- a/ecmascript/napi/include/jsnapi.h +++ b/ecmascript/napi/include/jsnapi.h @@ -1113,6 +1113,7 @@ public: static void SetHostResolvePathTracker(EcmaVM *vm, std::function cb); static void SetHostEnqueueJob(const EcmaVM* vm, Local cb); + static void InitializeIcuData(const ecmascript::JSRuntimeOptions &options); static void InitializeMemMapAllocator(); static void DestroyMemMapAllocator(); static EcmaVM* CreateEcmaVM(const ecmascript::JSRuntimeOptions &options); diff --git a/ecmascript/napi/jsnapi.cpp b/ecmascript/napi/jsnapi.cpp index a58aef3e..ab915aee 100644 --- a/ecmascript/napi/jsnapi.cpp +++ b/ecmascript/napi/jsnapi.cpp @@ -30,6 +30,7 @@ #include "ecmascript/ecma_runtime_call_info.h" #include "ecmascript/ecma_string.h" #include "ecmascript/ecma_vm.h" +#include "ecmascript/file_loader.h" #include "ecmascript/global_env.h" #include "ecmascript/interpreter/fast_runtime_stub-inl.h" #include "ecmascript/jobs/micro_job_queue.h" @@ -59,6 +60,7 @@ #include "ecmascript/tooling/interface/js_debugger_manager.h" #include "generated/base_options.h" +#include "ohos/init_data.h" #include "utils/pandargs.h" #include "os/mutex.h" @@ -156,6 +158,7 @@ EcmaVM *JSNApi::CreateEcmaVM(const JSRuntimeOptions &options) os::memory::LockHolder lock(mutex); vmCount_++; if (!initialize_) { + InitializeIcuData(options); InitializeMemMapAllocator(); initialize_ = true; } @@ -481,6 +484,22 @@ Local JSNApi::GetExportObject(EcmaVM *vm, const std::string &file, co return JSNApiHelper::ToLocal(exportObj); } +// Initialize IcuData Path +void JSNApi::InitializeIcuData(const JSRuntimeOptions &options) +{ + std::string icuPath = options.GetIcuDataPath(); + if (icuPath == "default") { + if (!WIN_OR_MAC_PLATFORM) { + SetHwIcuDirectory(); + } + } else { + std::string absPath; + if (ecmascript::FileLoader::GetAbsolutePath(icuPath, absPath)) { + u_setDataDirectory(absPath.c_str()); + } + } +} + void JSNApi::InitializeMemMapAllocator() { MemMapAllocator::GetInstance()->Initialize(ecmascript::DEFAULT_REGION_SIZE); -- Gitee