Skip to content

Commit

Permalink
Fix use after free issue
Browse files Browse the repository at this point in the history
Fixes #443

This is a different approach on #442. Thank you @orgads for doing the
legwork on tracking down the issue.
  • Loading branch information
laverdet committed Jan 19, 2024
1 parent 478815b commit f6cc843
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 3 deletions.
10 changes: 8 additions & 2 deletions src/external_copy/string.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@ class ExternalString final : public v8::String::ExternalStringResource {
ExternalString(const ExternalString&) = delete;

~ExternalString() final {
IsolateEnvironment::GetCurrent().AdjustExtraAllocatedMemory(-static_cast<int>(this->value->size()));
auto* environment = Executor::GetCurrentEnvironment();
if (environment != nullptr) {
environment->AdjustExtraAllocatedMemory(-static_cast<int>(this->value->size()));
}
}

auto operator= (const ExternalString&) = delete;
Expand All @@ -46,7 +49,10 @@ class ExternalStringOneByte final : public v8::String::ExternalOneByteStringReso
ExternalStringOneByte(const ExternalStringOneByte&) = delete;

~ExternalStringOneByte() final {
IsolateEnvironment::GetCurrent().AdjustExtraAllocatedMemory(-static_cast<int>(this->value->size()));
auto* environment = Executor::GetCurrentEnvironment();
if (environment != nullptr) {
environment->AdjustExtraAllocatedMemory(-static_cast<int>(this->value->size()));
}
}

auto operator= (const ExternalStringOneByte&) = delete;
Expand Down
7 changes: 7 additions & 0 deletions src/isolate/executor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ Executor::Executor(IsolateEnvironment& env) :
default_executor{*(current_executor == nullptr ? (current_executor = this) : &current_executor->default_executor)},
default_thread{&default_executor == this ? std::this_thread::get_id() : default_executor.default_thread} {}

Executor::~Executor() {
if (this == &default_executor) {
assert(current_executor == &default_executor);
current_executor = nullptr;
}
}

auto Executor::MayRunInlineTasks(IsolateEnvironment& env) -> bool {
if (current_executor == &env.executor) {
if (env.nodejs_isolate) {
Expand Down
2 changes: 1 addition & 1 deletion src/isolate/executor.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class Executor { // "En taro adun"
public:
explicit Executor(IsolateEnvironment& env);
Executor(const Executor&) = delete;
~Executor() = default;
~Executor();
auto operator= (const Executor&) = delete;

static auto GetCurrentEnvironment() -> IsolateEnvironment*;
Expand Down

0 comments on commit f6cc843

Please sign in to comment.