Commit 0316b4fc authored by Camillo Bruni's avatar Camillo Bruni Committed by Commit Bot

[d8] Fix leak detection for DynamicImportData from Modules

1) Since we collect a stack trace for unhandled promises we might end up
invoking code right before the shutdown phase.
2) Any dynamic module import that happens in this phase will enqueue a
microtask job with a freshly allocated DynamicImportData object. It
only gets deleted when fully emptying the microtask queue.
3) Since we're exiting we might end up with a non-empty microtask queue.
4) LSAN detects this as a leak on shutdown.

To make LSAN happy again d8 now keeps track of DynamicImportData to
free them on destructing PerIsolateData.

Bug: chromium:1158223
Change-Id: I9bb21f71bffc75a0d5f4ffc5bf0727c7b4cbab88
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2599755
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: 's avatarShu-yu Guo <syg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#72008}
parent 55865f77
...@@ -947,8 +947,6 @@ MaybeLocal<Module> Shell::FetchModuleTree(Local<Module> referrer, ...@@ -947,8 +947,6 @@ MaybeLocal<Module> Shell::FetchModuleTree(Local<Module> referrer,
return module; return module;
} }
namespace {
struct DynamicImportData { struct DynamicImportData {
DynamicImportData(Isolate* isolate_, Local<String> referrer_, DynamicImportData(Isolate* isolate_, Local<String> referrer_,
Local<String> specifier_, Local<String> specifier_,
...@@ -965,6 +963,7 @@ struct DynamicImportData { ...@@ -965,6 +963,7 @@ struct DynamicImportData {
Global<Promise::Resolver> resolver; Global<Promise::Resolver> resolver;
}; };
namespace {
struct ModuleResolutionData { struct ModuleResolutionData {
ModuleResolutionData(Isolate* isolate_, Local<Value> module_namespace_, ModuleResolutionData(Isolate* isolate_, Local<Value> module_namespace_,
Local<Promise::Resolver> resolver_) Local<Promise::Resolver> resolver_)
...@@ -1030,6 +1029,7 @@ MaybeLocal<Promise> Shell::HostImportModuleDynamically( ...@@ -1030,6 +1029,7 @@ MaybeLocal<Promise> Shell::HostImportModuleDynamically(
if (maybe_resolver.ToLocal(&resolver)) { if (maybe_resolver.ToLocal(&resolver)) {
DynamicImportData* data = new DynamicImportData( DynamicImportData* data = new DynamicImportData(
isolate, referrer->GetResourceName().As<String>(), specifier, resolver); isolate, referrer->GetResourceName().As<String>(), specifier, resolver);
PerIsolateData::Get(isolate)->AddDynamicImportData(data);
isolate->EnqueueMicrotask(Shell::DoHostImportModuleDynamically, data); isolate->EnqueueMicrotask(Shell::DoHostImportModuleDynamically, data);
return resolver->GetPromise(); return resolver->GetPromise();
} }
...@@ -1056,8 +1056,9 @@ void Shell::HostInitializeImportMetaObject(Local<Context> context, ...@@ -1056,8 +1056,9 @@ void Shell::HostInitializeImportMetaObject(Local<Context> context,
} }
void Shell::DoHostImportModuleDynamically(void* import_data) { void Shell::DoHostImportModuleDynamically(void* import_data) {
std::unique_ptr<DynamicImportData> import_data_( DynamicImportData* import_data_ =
static_cast<DynamicImportData*>(import_data)); static_cast<DynamicImportData*>(import_data);
Isolate* isolate(import_data_->isolate); Isolate* isolate(import_data_->isolate);
HandleScope handle_scope(isolate); HandleScope handle_scope(isolate);
...@@ -1066,6 +1067,8 @@ void Shell::DoHostImportModuleDynamically(void* import_data) { ...@@ -1066,6 +1067,8 @@ void Shell::DoHostImportModuleDynamically(void* import_data) {
Local<Promise::Resolver> resolver(import_data_->resolver.Get(isolate)); Local<Promise::Resolver> resolver(import_data_->resolver.Get(isolate));
PerIsolateData* data = PerIsolateData::Get(isolate); PerIsolateData* data = PerIsolateData::Get(isolate);
PerIsolateData::Get(isolate)->DeleteDynamicImportData(import_data_);
Local<Context> realm = data->realms_[data->realm_current_].Get(isolate); Local<Context> realm = data->realms_[data->realm_current_].Get(isolate);
Context::Scope context_scope(realm); Context::Scope context_scope(realm);
...@@ -1213,6 +1216,11 @@ PerIsolateData::~PerIsolateData() { ...@@ -1213,6 +1216,11 @@ PerIsolateData::~PerIsolateData() {
if (i::FLAG_expose_async_hooks) { if (i::FLAG_expose_async_hooks) {
delete async_hooks_wrapper_; // This uses the isolate delete async_hooks_wrapper_; // This uses the isolate
} }
#if defined(LEAK_SANITIZER)
for (DynamicImportData* data : import_data_) {
delete data;
}
#endif
} }
void PerIsolateData::SetTimeout(Local<Function> callback, void PerIsolateData::SetTimeout(Local<Function> callback,
...@@ -1276,6 +1284,18 @@ int PerIsolateData::HandleUnhandledPromiseRejections() { ...@@ -1276,6 +1284,18 @@ int PerIsolateData::HandleUnhandledPromiseRejections() {
return static_cast<int>(i); return static_cast<int>(i);
} }
void PerIsolateData::AddDynamicImportData(DynamicImportData* data) {
#if defined(LEAK_SANITIZER)
import_data_.insert(data);
#endif
}
void PerIsolateData::DeleteDynamicImportData(DynamicImportData* data) {
#if defined(LEAK_SANITIZER)
import_data_.erase(data);
#endif
delete data;
}
PerIsolateData::RealmScope::RealmScope(PerIsolateData* data) : data_(data) { PerIsolateData::RealmScope::RealmScope(PerIsolateData* data) : data_(data) {
data_->realm_count_ = 1; data_->realm_count_ = 1;
data_->realm_current_ = 0; data_->realm_current_ = 0;
...@@ -2402,13 +2422,10 @@ void Shell::PromiseRejectCallback(v8::PromiseRejectMessage data) { ...@@ -2402,13 +2422,10 @@ void Shell::PromiseRejectCallback(v8::PromiseRejectMessage data) {
} }
if (!exception->IsNativeError() && if (!exception->IsNativeError() &&
(message.IsEmpty() || message->GetStackTrace().IsEmpty())) { (message.IsEmpty() || message->GetStackTrace().IsEmpty())) {
// If there is no real Error object, manually throw and catch a stack trace. // If there is no real Error object, manually create a stack trace.
v8::TryCatch try_catch(isolate); exception = v8::Exception::Error(
try_catch.SetVerbose(true); v8::String::NewFromUtf8Literal(isolate, "Unhandled Promise."));
isolate->ThrowException(v8::Exception::Error( message = Exception::CreateMessage(isolate, exception);
v8::String::NewFromUtf8Literal(isolate, "Unhandled Promise.")));
message = try_catch.Message();
exception = try_catch.Exception();
} }
isolate->SetCaptureStackTraceForUncaughtExceptions(capture_exceptions); isolate->SetCaptureStackTraceForUncaughtExceptions(capture_exceptions);
......
...@@ -30,6 +30,8 @@ namespace internal { ...@@ -30,6 +30,8 @@ namespace internal {
class CancelableTaskManager; class CancelableTaskManager;
} // namespace internal } // namespace internal
struct DynamicImportData;
// A single counter in a counter collection. // A single counter in a counter collection.
class Counter { class Counter {
public: public:
...@@ -262,6 +264,11 @@ class PerIsolateData { ...@@ -262,6 +264,11 @@ class PerIsolateData {
Local<Value> exception); Local<Value> exception);
int HandleUnhandledPromiseRejections(); int HandleUnhandledPromiseRejections();
// Keep track of DynamicImportData so we can properly free it on shutdown
// when LEAK_SANITIZER is active.
void AddDynamicImportData(DynamicImportData*);
void DeleteDynamicImportData(DynamicImportData*);
private: private:
friend class Shell; friend class Shell;
friend class RealmScope; friend class RealmScope;
...@@ -277,6 +284,9 @@ class PerIsolateData { ...@@ -277,6 +284,9 @@ class PerIsolateData {
std::vector<std::tuple<Global<Promise>, Global<Message>, Global<Value>>> std::vector<std::tuple<Global<Promise>, Global<Message>, Global<Value>>>
unhandled_promises_; unhandled_promises_;
AsyncHooks* async_hooks_wrapper_; AsyncHooks* async_hooks_wrapper_;
#if defined(LEAK_SANITIZER)
std::unordered_set<DynamicImportData*> import_data_;
#endif
int RealmIndexOrThrow(const v8::FunctionCallbackInfo<v8::Value>& args, int RealmIndexOrThrow(const v8::FunctionCallbackInfo<v8::Value>& args,
int arg_offset); int arg_offset);
......
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