chakrashim: per isolate message listeners and embedded data

Message listeners and embeddedData should be per isolate
nodejs · Jun 29, 2015 · 07547c2 · 07547c2
1 parent 2bb7fc3
commit 07547c2
Show file tree

Hide file tree

Showing 6 changed files with 62 additions and 46 deletions.
diff --git a/deps/chakrashim/include/v8.h b/deps/chakrashim/include/v8.h
@@ -1147,11 +1147,6 @@ namespace v8
     bool IdleNotification(int idle_time_in_ms);
     void LowMemoryNotification();
     int ContextDisposedNotification();
-
-    // Chakra implementation
-    bool InternalAddMessageListener(MessageCallback that);
-    void InternalRemoveMessageListeners(MessageCallback that);
-    void InternalInvokeMessageListeners(Handle<Message> message, Handle<Value> error);
   };
 
   class EXPORT JitCodeEvent

diff --git a/deps/chakrashim/src/jsrtisolateshim.cc b/deps/chakrashim/src/jsrtisolateshim.cc
@@ -21,6 +21,8 @@
 #include "v8.h"
 #include "jsrtutils.h"
 #include <assert.h>
+#include <vector>
+#include <algorithm>
 
 namespace jsrt
 {
@@ -367,4 +369,37 @@ namespace jsrt
     }
     return false;
   }
+
+  bool IsolateShim::AddMessageListener(void * that)
+  {
+    try
+    {
+      messageListeners.push_back(that);
+      return true;
+    }
+    catch (...)
+    {
+      return false;
+    }
+  }
+
+  void IsolateShim::RemoveMessageListeners(void * that)
+  {
+    auto i = std::remove(messageListeners.begin(), messageListeners.end(), that);
+    messageListeners.erase(i, messageListeners.end());
+  }
+
+  void IsolateShim::SetData(uint32_t slot, void* data)
+  {
+    if (slot >= _countof(this->embeddedData))
+    {
+      CHAKRA_UNIMPLEMENTED_("Invalid embedded data index");
+    }
+    embeddedData[slot] = data;
+  }
+
+  void* IsolateShim::GetData(uint32_t slot)
+  {
+    return slot < _countof(this->embeddedData) ? embeddedData[slot] : nullptr;
+  }
 };
diff --git a/deps/chakrashim/src/jsrtisolateshim.h b/deps/chakrashim/src/jsrtisolateshim.h
@@ -81,6 +81,21 @@ namespace jsrt
     void DisableExecution();
     bool IsExeuctionDisabled();
     void EnableExecution();
+
+
+    bool AddMessageListener(void * that);
+    void RemoveMessageListeners(void * that);
+    template <typename Fn>
+    void ForEachMessageListener(Fn fn)
+    {
+      for (auto i = messageListeners.begin(); i != messageListeners.end(); i++)
+      {
+        fn(*i);
+      }
+    }
+
+    void SetData(unsigned int slot, void* data);
+    void* GetData(unsigned int slot);
   private:
     // Construction/Destruction should go thru New/Dispose
     IsolateShim(JsRuntimeHandle runtime);
@@ -106,7 +121,10 @@ namespace jsrt
 
     std::unordered_map<JsContextRef, ContextShim *> contextShimMap;
     std::unordered_map<JsValueRef, ContextShim *> jsValueRefToContextShimMap;
+    std::vector<void *> messageListeners;
 
+    // Node only has 4 slots (internals::Internals::kNumIsolateDataSlots = 4)
+    void * embeddedData[4];
 
     // CHAKRA-TODO: support multiple shims
     static IsolateShim * s_isolateList;

diff --git a/deps/chakrashim/src/v8isolate.cc b/deps/chakrashim/src/v8isolate.cc
@@ -21,8 +21,6 @@
 #include "v8.h"
 #include "v8-profiler.h"
 #include "jsrtutils.h"
-#include <vector>
-#include <algorithm>
 
 namespace v8
 {
@@ -67,19 +65,14 @@ namespace v8
     return 0;
   }
 
-  static void* s_dummyData[4];
-
   void Isolate::SetData(uint32_t slot, void* data)
   {
-    if (slot < _countof(s_dummyData))
-    {
-      s_dummyData[slot] = data;
-    }
+    return jsrt::IsolateShim::FromIsolate(this)->SetData(slot, data);
   }
 
   void* Isolate::GetData(uint32_t slot)
   {
-    return slot < _countof(s_dummyData)? s_dummyData[slot] : nullptr;
+    return jsrt::IsolateShim::FromIsolate(this)->GetData(slot);
   }
 
   uint32_t Isolate::GetNumberOfDataSlots()
@@ -164,35 +157,6 @@ namespace v8
     return 0;
   }
 
-  static std::vector<MessageCallback> s_messageListeners;
-
-  bool Isolate::InternalAddMessageListener(MessageCallback that)
-  {
-    try
-    {
-      s_messageListeners.push_back(that);
-      return true;
-    }
-    catch (...)
-    {
-      return false;
-    }
-  }
-
-  void Isolate::InternalRemoveMessageListeners(MessageCallback that)
-  {
-    auto i = std::remove(s_messageListeners.begin(), s_messageListeners.end(), that);
-    s_messageListeners.erase(i, s_messageListeners.end());
-  }
-
-  void Isolate::InternalInvokeMessageListeners(Handle<Message> message, Handle<Value> error)
-  {
-    for (auto i = s_messageListeners.begin(); i != s_messageListeners.end(); i++)
-    {
-      (*i)(message, error);
-    }
-  }
-
   void Isolate::GetHeapStatistics(HeapStatistics *heap_statistics)
   {
     size_t memoryUsage;

diff --git a/deps/chakrashim/src/v8trycatch.cc b/deps/chakrashim/src/v8trycatch.cc
@@ -184,7 +184,10 @@ namespace v8
     // we'll report the external exception message.
     if (prev != nullptr && prev->verbose)
     {
-      Isolate::GetCurrent()->InternalInvokeMessageListeners(Message(), Exception());
+      jsrt::IsolateShim::GetCurrent()->ForEachMessageListener([this](void * messageListener)
+      {
+        ((v8::MessageCallback)messageListener)(Message(), Exception());
+      });
     }
     else
     {

diff --git a/deps/chakrashim/src/v8v8.cc b/deps/chakrashim/src/v8v8.cc
@@ -153,13 +153,14 @@ namespace v8
 
   bool V8::AddMessageListener(MessageCallback that, Handle<Value> data)
   {
-    return Isolate::GetCurrent()->InternalAddMessageListener(that);
+    // Ignore data parameter.  Node doesn't use it.
+    return jsrt::IsolateShim::GetCurrent()->AddMessageListener(that);
   }
 
 
   void V8::RemoveMessageListeners(MessageCallback that)
   {
-    Isolate::GetCurrent()->InternalRemoveMessageListeners(that);
+    jsrt::IsolateShim::GetCurrent()->RemoveMessageListeners(that);
   }
 
   void V8::SetJitCodeEventHandler(JitCodeEventOptions options, JitCodeEventHandler event_handler)