Add Isolate::DiscardThreadSpecificMetadata method to embedder API.

If many threads use the same Isolate (or many Isolates) and then
terminate, their PerIsolateThreadData objects are never cleaned
up, resulting in a slow memory leak and, worse, the
PerIsolateThreadData chain getting larger and larger, adversely
affecting performance.

In this situation, embedders will now be encouraged to apply
DiscardThreadSpecificMetadata against any Isolate a thread is
done with, especially if the thread is about to terminate.

Note that it is harmless to run DiscardThreadSpecificMetadata
against an Isolate for which a thread has no thread data and
per-Isolate thread data can be reestablished if a thread starts
using an Isolate again after running DiscardThreadSpecificMetadata
against it.

It is, however, an embedder error to run
DiscardThreadSpecificMetadata against an Isolate in thread with a
Locker for the Isolate in the stack or against an Entered Isolate.

This change cannot cause any change in behavior in existing apps
as the only added coded can only be reached via the new
DiscardThreadSpecificMetadata method.

R=Jakob, jochen
BUG=

Review URL: https://codereview.chromium.org/1522703002

Cr-Commit-Position: refs/heads/master@{#32909}
This commit is contained in:
akodat 2015-12-16 07:49:28 -08:00 committed by Commit bot
parent 2358a5be4c
commit aeb8073c4a
6 changed files with 59 additions and 18 deletions

View File

@ -32,6 +32,7 @@ StrongLoop, Inc. <*@strongloop.com>
Aaron Bieber <deftly@gmail.com>
Abdulla Kamar <abdulla.kamar@gmail.com>
Akinori MUSHA <knu@FreeBSD.org>
Alex Kodat <akodat@rocketsoftware.com>
Alexander Botero-Lowry <alexbl@FreeBSD.org>
Alexander Karpinsky <homm86@gmail.com>
Alexandre Vassalotti <avassalotti@gmail.com>

View File

@ -5489,6 +5489,15 @@ class V8_EXPORT Isolate {
*/
void Dispose();
/**
* Discards all V8 thread-specific data for the Isolate. Should be used
* if a thread is terminating and it has used an Isolate that will outlive
* the thread -- all thread-specific data for an Isolate is discarded when
* an Isolate is disposed so this call is pointless if an Isolate is about
* to be Disposed.
*/
void DiscardThreadSpecificMetadata();
/**
* Associate embedder-specific data with the isolate. |slot| has to be
* between 0 and GetNumberOfDataSlots() - 1.

View File

@ -7162,6 +7162,12 @@ void Isolate::Dispose() {
}
void Isolate::DiscardThreadSpecificMetadata() {
i::Isolate* isolate = reinterpret_cast<i::Isolate*>(this);
isolate->DiscardPerThreadDataForThisThread();
}
void Isolate::Enter() {
i::Isolate* isolate = reinterpret_cast<i::Isolate*>(this);
isolate->Enter();

View File

@ -135,6 +135,22 @@ Isolate::PerIsolateThreadData*
}
void Isolate::DiscardPerThreadDataForThisThread() {
int thread_id_int = base::Thread::GetThreadLocalInt(Isolate::thread_id_key_);
if (thread_id_int) {
ThreadId thread_id = ThreadId(thread_id_int);
DCHECK(!thread_manager_->mutex_owner_.Equals(thread_id));
base::LockGuard<base::Mutex> lock_guard(thread_data_table_mutex_.Pointer());
PerIsolateThreadData* per_thread =
thread_data_table_->Lookup(this, thread_id);
if (per_thread) {
DCHECK(!per_thread->thread_state_);
thread_data_table_->Remove(per_thread);
}
}
}
Isolate::PerIsolateThreadData* Isolate::FindPerThreadDataForThisThread() {
ThreadId thread_id = ThreadId::Current();
return FindPerThreadDataForThread(thread_id);

View File

@ -518,6 +518,10 @@ class Isolate {
// If one does not yet exist, return null.
PerIsolateThreadData* FindPerThreadDataForThread(ThreadId thread_id);
// Discard the PerThread for this particular (isolate, thread) combination
// If one does not yet exist, no-op.
void DiscardPerThreadDataForThisThread();
// Returns the key used to store the pointer to the current isolate.
// Used internally for V8 threads that do not execute JavaScript but still
// are part of the domain of an isolate (like the context switcher).

View File

@ -347,26 +347,31 @@ class LockerUnlockerThread : public JoinableThread {
}
virtual void Run() {
v8::Locker lock(isolate_);
v8::Isolate::Scope isolate_scope(isolate_);
v8::HandleScope handle_scope(isolate_);
v8::Local<v8::Context> context = v8::Context::New(isolate_);
isolate_->DiscardThreadSpecificMetadata(); // No-op
{
v8::Context::Scope context_scope(context);
CalcFibAndCheck(context);
}
{
LockIsolateAndCalculateFibSharedContextThread thread(isolate_, context);
isolate_->Exit();
v8::Unlocker unlocker(isolate_);
thread.Start();
thread.Join();
}
isolate_->Enter();
{
v8::Context::Scope context_scope(context);
CalcFibAndCheck(context);
v8::Locker lock(isolate_);
v8::Isolate::Scope isolate_scope(isolate_);
v8::HandleScope handle_scope(isolate_);
v8::Local<v8::Context> context = v8::Context::New(isolate_);
{
v8::Context::Scope context_scope(context);
CalcFibAndCheck(context);
}
{
LockIsolateAndCalculateFibSharedContextThread thread(isolate_, context);
isolate_->Exit();
v8::Unlocker unlocker(isolate_);
thread.Start();
thread.Join();
}
isolate_->Enter();
{
v8::Context::Scope context_scope(context);
CalcFibAndCheck(context);
}
}
isolate_->DiscardThreadSpecificMetadata();
isolate_->DiscardThreadSpecificMetadata(); // No-op
}
private: