Skip to content

Commit

Permalink
fix(jsc): fix gc finalize private data invalid crash
Browse files Browse the repository at this point in the history
  • Loading branch information
etkmao authored and zealotchen0 committed Aug 9, 2023
1 parent 6bef64a commit 8411371
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 1 deletion.
9 changes: 8 additions & 1 deletion driver/js/include/driver/vm/jsc/jsc_vm.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
#pragma once

#include "driver/vm/js_vm.h"

#include <set>
#include <JavaScriptCore/JavaScriptCore.h>

#include "footstone/string_view.h"
Expand All @@ -49,6 +49,13 @@ class JSCVM : public VM, public std::enable_shared_from_this<JSCVM> {
std::unordered_map<void*, std::unordered_map<JSClassRef, std::unique_ptr<ConstructorData>>> constructor_data_holder_;
JSContextGroupRef vm_;

static void SaveConstructorDataPtr(void* ptr);
static void ClearConstructorDataPtr(void* ptr);
static bool IsValidConstructorDataPtr(void* ptr);

static std::set<void*> constructor_data_ptr_set_;
static std::mutex mutex_;

virtual std::shared_ptr<CtxValue> ParseJson(const std::shared_ptr<Ctx>& ctx, const string_view& json) override;
virtual std::shared_ptr<Ctx> CreateContext() override;

Expand Down
5 changes: 5 additions & 0 deletions driver/js/src/napi/jsc/jsc_ctx.cc
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ JSCCtx::~JSCCtx() {
auto& holder = jsc_vm->constructor_data_holder_[this];
for (auto& [key, item] : holder) {
item->prototype = nullptr;
JSCVM::ClearConstructorDataPtr(item.get());
}
}

Expand Down Expand Up @@ -262,6 +263,9 @@ std::shared_ptr<CtxValue> JSCCtx::DefineClass(const string_view& name,
if (!private_data) {
return;
}
if (!JSCVM::IsValidConstructorDataPtr(private_data)) {
return;
}
auto constructor_data = reinterpret_cast<ConstructorData*>(private_data);
auto weak_callback_wrapper = constructor_data->weak_callback_wrapper;
if (weak_callback_wrapper) {
Expand Down Expand Up @@ -974,6 +978,7 @@ void JSCCtx::SaveConstructorData(std::unique_ptr<ConstructorData> constructor_da
if (it == holder.end()) {
holder[this] = std::unordered_map<JSClassRef, std::unique_ptr<ConstructorData>>{};
}
JSCVM::SaveConstructorDataPtr(constructor_data.get());
holder[this][constructor_data->class_ref] = std::move(constructor_data);
}

Expand Down
18 changes: 18 additions & 0 deletions driver/js/src/vm/jsc/jsc_vm.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ namespace hippy {
inline namespace driver {
inline namespace vm {

std::set<void*> JSCVM::constructor_data_ptr_set_;
std::mutex JSCVM::mutex_;

std::shared_ptr<CtxValue> JSCVM::ParseJson(const std::shared_ptr<Ctx>& ctx, const string_view& json) {
if (footstone::StringViewUtils::IsEmpty(json)) {
return nullptr;
Expand Down Expand Up @@ -90,6 +93,21 @@ JSStringRef JSCVM::CreateJSCString(const string_view& str_view) {
return ret;
}

void JSCVM::SaveConstructorDataPtr(void* ptr) {
std::lock_guard<std::mutex> lock(mutex_);
constructor_data_ptr_set_.insert(ptr);
}

void JSCVM::ClearConstructorDataPtr(void* ptr) {
std::lock_guard<std::mutex> lock(mutex_);
constructor_data_ptr_set_.erase(ptr);
}

bool JSCVM::IsValidConstructorDataPtr(void* ptr) {
std::lock_guard<std::mutex> lock(mutex_);
return constructor_data_ptr_set_.find(ptr) != constructor_data_ptr_set_.end();
}

}
}
}

0 comments on commit 8411371

Please sign in to comment.