Issue 1549 - initialize v8::V8

src/JsEngine.cpp

 /*
  * This file is part of Adblock Plus <http://adblockplus.org/>,
  * Copyright (C) 2006-2014 Eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 23 matching lines @@
     }
     else
       return v8::Script::Compile(v8Source);
   }
 
   void CheckTryCatch(const v8::TryCatch& tryCatch)
   {
     if (tryCatch.HasCaught())
       throw AdblockPlus::JsError(tryCatch.Exception(), tryCatch.Message());
   }
+
+  class V8Initializer
+  {
+    V8Initializer()
+    {
+      v8::V8::Initialize();
+    }
+
+    ~V8Initializer()
+    {
+      v8::V8::Dispose();
+    }
+  public:
+    static void Init()
+    {
+      // it's threadsafe since C++11 and it will be instantiated only once and
+      // destroyed at the application exit
+      static V8Initializer initializer;
+    }

[Wladimir Palant, 2014/11/11 20:55:40]

Wouldn't it make more sense (meaning more obvious and less code) to have a
static variable initialized at startup instead of the explicit method call? In
case you didn't want to call V8::Initialize() if JsEngine isn't being used at
all - in that scenario the linker should optimize this entire file away and
V8Initializer won't be created anyway.

[sergei, 2014/11/12 10:11:32]

Nit: I am not sure whether the linker can optimize it out. I would say it has
quite big side effects and I expect the linker to keep it even if it's not
referenced. I certainly remember regarding such things that linker can drop the
whole static library if nothing is referenced from it.

But what bothers me, the order of initialization of global static is undefined.
I'm afraid that there can be something else static in v8 and I think it's safer
to call `v8::V8::Initialize();` after all that stuff is initialized. Thus v8's
internal static is initialized before `main` and then we can safely call v8's
API.

[Wladimir Palant, 2014/11/12 10:18:36]

Yes, order of initialization is something I wasn't sure about either. So - fine
with me, we can keep this as an explicit call.

+  };
 }
 
 AdblockPlus::JsEngine::JsEngine()
   : isolate(v8::Isolate::GetCurrent())
 {
 }
 
 AdblockPlus::JsEnginePtr AdblockPlus::JsEngine::New(const AppInfo& appInfo)
 {
+  V8Initializer::Init();
   JsEnginePtr result(new JsEngine());
 
   const v8::Locker locker(result->isolate);
   const v8::HandleScope handleScope;
 
   result->context.reset(result->isolate, v8::Context::New(result->isolate));
   v8::Local<v8::Object> globalContext = v8::Local<v8::Context>::New(
     result->isolate, result->context)->Global();
   AdblockPlus::GlobalJsObject::Setup(result, appInfo,
     JsValuePtr(new JsValue(result, globalContext)));
@@ skipping 132 matching lines @@
   return logSystem;
 }
 
 void AdblockPlus::JsEngine::SetLogSystem(AdblockPlus::LogSystemPtr val)
 {
   if (!val)
     throw std::runtime_error("LogSystem cannot be null");
 
   logSystem = val;
 }