Commit 33da5683 authored by Clemens Backes's avatar Clemens Backes Committed by Commit Bot

[inspector][test] Remove memory leak via Vectors

The {ToV8Vector} method returns a {i::Vector} pointing to heap-allocated
memory, but that memory was never free'd. Since we already have a
{ToVector} method returning a {std::vector}, this CL switches to that
one instead.

R=szuend@chromium.org

Bug: chromium:1142437, v8:11107
Change-Id: I8ee0177f7dcfe2ecb435e684674b0cda6f613658
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2523198Reviewed-by: 's avatarSimon Zünd <szuend@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71040}
parent ce8840d0
......@@ -167,9 +167,10 @@
##############################################################################
['asan == True', {
# There are lots of memory leaks, thus skip all inspector tests for now
# There are still memory leaks in some inspector tests
# (https://crbug.com/v8/11107).
'*': [SKIP],
'runtime/evaluate-async': [SKIP],
'debugger/step-snapshot': [SKIP],
}], # asan == True
]
......@@ -17,13 +17,6 @@ namespace {
const int kIsolateDataIndex = 2;
const int kContextGroupIdIndex = 3;
// TODO(clemensb): This is a memory leak; remove this helper.
Vector<uint16_t> ToV8Vector(v8::Isolate* isolate, v8::Local<v8::String> str) {
Vector<uint16_t> buffer = Vector<uint16_t>::New(str->Length());
str->Write(isolate, buffer.begin(), 0, str->Length());
return buffer;
}
void Print(v8::Isolate* isolate, const v8_inspector::StringView& string) {
v8::Local<v8::String> v8_string = ToV8String(isolate, string);
v8::String::Utf8Value utf8_string(isolate, v8_string);
......@@ -118,7 +111,7 @@ int IsolateData::GetContextGroupId(v8::Local<v8::Context> context) {
}
void IsolateData::RegisterModule(v8::Local<v8::Context> context,
Vector<uint16_t> name,
std::vector<uint16_t> name,
v8::ScriptCompiler::Source* source) {
v8::Local<v8::Module> module;
if (!v8::ScriptCompiler::CompileModule(isolate(), source).ToLocal(&module))
......@@ -138,7 +131,7 @@ v8::MaybeLocal<v8::Module> IsolateData::ModuleResolveCallback(
v8::Local<v8::Module> referrer) {
IsolateData* data = IsolateData::FromContext(context);
std::string str = *v8::String::Utf8Value(data->isolate(), specifier);
return data->modules_[ToV8Vector(data->isolate(), specifier)].Get(
return data->modules_[ToVector(data->isolate(), specifier)].Get(
data->isolate());
}
......@@ -274,15 +267,15 @@ int IsolateData::HandleMessage(v8::Local<v8::Message> message,
column_number = message->GetStartColumn(context).FromJust() + 1;
v8_inspector::StringView detailed_message;
Vector<uint16_t> message_text_string = ToV8Vector(isolate, message->Get());
v8_inspector::StringView message_text(message_text_string.begin(),
message_text_string.length());
Vector<uint16_t> url_string;
std::vector<uint16_t> message_text_string = ToVector(isolate, message->Get());
v8_inspector::StringView message_text(message_text_string.data(),
message_text_string.size());
std::vector<uint16_t> url_string;
if (message->GetScriptOrigin().ResourceName()->IsString()) {
url_string = ToV8Vector(
url_string = ToVector(
isolate, message->GetScriptOrigin().ResourceName().As<v8::String>());
}
v8_inspector::StringView url(url_string.begin(), url_string.length());
v8_inspector::StringView url(url_string.data(), url_string.size());
v8::SealHandleScope seal_handle_scope(isolate);
return inspector->exceptionThrown(
......@@ -471,14 +464,14 @@ namespace {
class StringBufferImpl : public v8_inspector::StringBuffer {
public:
StringBufferImpl(v8::Isolate* isolate, v8::Local<v8::String> string)
: data_(ToV8Vector(isolate, string)) {}
: data_(ToVector(isolate, string)) {}
v8_inspector::StringView string() const override {
return v8_inspector::StringView(data_.begin(), data_.length());
return v8_inspector::StringView(data_.data(), data_.size());
}
private:
Vector<uint16_t> data_;
std::vector<uint16_t> data_;
};
} // anonymous namespace
......@@ -489,8 +482,7 @@ std::unique_ptr<v8_inspector::StringBuffer> IsolateData::resourceNameToUrl(
v8::Local<v8::String> name = ToV8String(isolate(), resourceName);
v8::Local<v8::String> prefix = resource_name_prefix_.Get(isolate());
v8::Local<v8::String> url = v8::String::Concat(isolate(), prefix, name);
return std::unique_ptr<StringBufferImpl>(
new StringBufferImpl(isolate(), url));
return std::make_unique<StringBufferImpl>(isolate(), url);
}
} // namespace internal
......
......@@ -44,7 +44,7 @@ class IsolateData : public v8_inspector::V8InspectorClient {
v8::Local<v8::Context> GetDefaultContext(int context_group_id);
int GetContextGroupId(v8::Local<v8::Context> context);
void RegisterModule(v8::Local<v8::Context> context,
v8::internal::Vector<uint16_t> name,
std::vector<uint16_t> name,
v8::ScriptCompiler::Source* source);
// Working with V8Inspector api.
......@@ -87,15 +87,6 @@ class IsolateData : public v8_inspector::V8InspectorClient {
void SetResourceNamePrefix(v8::Local<v8::String> prefix);
private:
struct VectorCompare {
bool operator()(const v8::internal::Vector<uint16_t>& lhs,
const v8::internal::Vector<uint16_t>& rhs) const {
for (int i = 0; i < lhs.length() && i < rhs.length(); ++i) {
if (lhs[i] != rhs[i]) return lhs[i] < rhs[i];
}
return false;
}
};
static v8::MaybeLocal<v8::Module> ModuleResolveCallback(
v8::Local<v8::Context> context, v8::Local<v8::String> specifier,
v8::Local<v8::Module> referrer);
......@@ -143,9 +134,7 @@ class IsolateData : public v8_inspector::V8InspectorClient {
std::unique_ptr<v8_inspector::V8Inspector> inspector_;
int last_context_group_id_ = 0;
std::map<int, std::vector<v8::Global<v8::Context>>> contexts_;
std::map<v8::internal::Vector<uint16_t>, v8::Global<v8::Module>,
VectorCompare>
modules_;
std::map<std::vector<uint16_t>, v8::Global<v8::Module>> modules_;
int last_session_id_ = 0;
std::map<int, std::unique_ptr<v8_inspector::V8InspectorSession>> sessions_;
std::map<v8_inspector::V8InspectorSession*, int> context_group_by_session_;
......
......@@ -45,12 +45,7 @@ void ExecuteStringTask::Run(IsolateData* data) {
v8::MaybeLocal<v8::Value> result;
result = script->Run(context);
} else {
// Register Module takes ownership of {buffer}, so we need to make a copy.
int length = static_cast<int>(name_.size());
v8::internal::Vector<uint16_t> buffer =
v8::internal::Vector<uint16_t>::New(length);
std::copy(name_.begin(), name_.end(), buffer.begin());
data->RegisterModule(context, buffer, &scriptSource);
data->RegisterModule(context, name_, &scriptSource);
}
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment