Fix v8::Isolate management

[sergz]

Background

• With the current version of v8 (v8-googlesource hg:28969bee9861 git:6bcca144a974f97d9496d3f7b538f90208f89c2e) to obtain v8::Isolate we call v8::Isolate::GetCurrent() in the constructor of AdblockPlus::JsEngine. If there is no any isolate associated with the current thread then a new instance of v8::Isolate is allocated and initialized.
• We also never deallocate that isolate, that's a bug which should be fixed.

As the result, using GetCurrent() tests do share the state (through v8::Isolate) but they should not share any state. That also should be fixed.

What to change

• Find what keeps JsEngine to stay alive, that prevents the isolate from deallocating (some already known leaks are in Blocked by), and eliminate that behaviour. It seems that will be a set of other issues. Here we could add some tests (see Tests notes).
• Switch from using v8::Isolate::GetCurrent() to v8::Isolate::New() and v8::Isolate::Dispose().
• (see Additional notes) Get rid of the hack with shared isolate by JsEngine in tests. We likely don't need to inject isolate into JsEngine, so make v8::Isolate wrapper as a plain JsEngine member.

Additional notes

There is also another issue #1197 which is more important than this one right now. That issue also requires to switch from using v8::Isolate::GetCurrent() to v8::Isolate::New() which is a part of the current issue.

• With the updated v8 (according to #1197) we cannot simply use that approach because 32bit version of tests starting from some point runs out of memory, 64 bit version works.

The callstack

>	tests.exe!v8::base::RandomizedVirtualAlloc(unsigned int size, int action, int protection) Line 758	C++
 	tests.exe!v8::base::VirtualMemory::ReserveRegion(unsigned int size) Line 1254	C++
 	tests.exe!v8::base::VirtualMemory::VirtualMemory(unsigned int size, unsigned int alignment) Line 1190	C++
 	tests.exe!v8::internal::MemoryAllocator::ReserveAlignedMemory(unsigned int size, unsigned int alignment, v8::base::VirtualMemory * controller) Line 394	C++
 	tests.exe!v8::internal::NewSpace::SetUp(int reserved_semispace_capacity, int maximum_semispace_capacity) Line 1232	C++
 	tests.exe!v8::internal::Heap::SetUp() Line 5464	C++
 	tests.exe!v8::internal::Isolate::Init(v8::internal::Deserializer * des) Line 1992	C++
 	tests.exe!v8::internal::Snapshot::Initialize(v8::internal::Isolate * isolate) Line 52	C++
 	tests.exe!v8::Isolate::New(const v8::Isolate::CreateParams & params) Line 6631	C++
 	tests.exe!AdblockPlus::ScopedV8Isolate::ScopedV8Isolate() Line 76	C++
 	tests.exe!std::_Ref_count_obj<AdblockPlus::ScopedV8Isolate>::_Ref_count_obj<AdblockPlus::ScopedV8Isolate><>() Line 932	C++
 	tests.exe!std::make_shared<AdblockPlus::ScopedV8Isolate>() Line 1003	C++
 	tests.exe!AdblockPlus::JsEngine::New(const AdblockPlus::AppInfo & appInfo, const std::shared_ptr<AdblockPlus::ScopedV8Isolate> & isolate) Line 94	C++
...

parameters are: 33554432 == 32 * 2²⁰, MEM_RESERVE, PAGE_NOACCESS, base is nullptr.
GetLastError() == 8, "Not enough storage is available to process this command".

• With the current version of v8 we also cannot simply use it, I suppose because during the allocation of a new isolate v8 finds another one stored in the current TLS, which results in some internal conflict.

Until we eliminate the leaking of JsEngine I propose to use the following hack, which also suits the current version of v8, so we can commit that change independently from the rest of #1197 changes. That simplifies the reviewing of #1197 and makes the commit history cleaner.
The hack continues to share one isolate among most tests, though already not among absolutely all tests, so we don't run out of memory with the newer version of v8 and there is no internal conflict regarding isolate in v8.
One can find it in ​https://codereview.adblockplus.org/6233220328718336/.

Test notes

Eric wrote:

1) A test that instantiates many isolates using New()/Dispose() but that doesn't
call anything with them.

2) A test that instantiates many 'JsEngine' objects. If test (1) passes and test
(2) does not, then we have a leak in one of the other members.

[eric@adblockplus.org]

Review ​https://codereview.adblockplus.org/29370977/, possible after rewriting all the thread management in the library, removes the shared isolate used in tests. Some of the tests need some rework to deal with a "feature" of Google Test: you can't rely on the test destructor to release memory in a timely fashion. On my machine, the test suite completes without running out of memory.

[abpbot]

A commit referencing this issue has landed:
​Issue 3593 - stop sharing v8::Isolate among tests

[sergz]

Recent changes allowed to fix this issue, so #3594, #3595, #4688, #4692 are not blocking it nor it blocks #4826, BTW.