Merge pull request #125 from obastemur/remove_shared

chakrashim: Remove shared_ptr usage
nodejs · Oct 3, 2016 · 2968f50 · 2968f50
2 parents cecbc36 + 478e5c1
commit 2968f50
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 37 deletions.
diff --git a/common.gypi b/common.gypi
@@ -389,6 +389,7 @@
           'GCC_DYNAMIC_NO_PIC': 'NO',               # No -mdynamic-no-pic
                                                     # (Equivalent to -fPIC)
           'GCC_ENABLE_CPP_EXCEPTIONS': 'NO',        # -fno-exceptions
+          'GCC_ENABLE_CPP_RTTI': 'NO',              # -fno-rtti
           'GCC_ENABLE_PASCAL_STRINGS': 'NO',        # No -mpascal-strings
           'GCC_THREADSAFE_STATICS': 'NO',           # -fno-threadsafe-statics
           'PREBINDING': 'NO',                       # No -Wl,-prebind
@@ -421,15 +422,6 @@
           ['target_arch=="x64"', {
             'xcode_settings': {'ARCHS': ['x86_64']},
           }],
-          ['node_engine=="chakracore"', {
-            'xcode_settings': {
-              'GCC_ENABLE_CPP_RTTI': 'YES',     # tr1 shared_ptr uses typeid
-            }
-          }, {
-            'xcode_settings': {
-              'GCC_ENABLE_CPP_RTTI': 'NO',      # -fno-rtti
-            }
-          }],
           ['clang==1', {
             'xcode_settings': {
               'GCC_VERSION': 'com.apple.compilers.llvm.clang.1_0',

diff --git a/deps/chakrashim/include/v8.h b/deps/chakrashim/include/v8.h
@@ -62,14 +62,7 @@
 #endif
 #endif
 
-#ifdef OSX_SDK_TR1
-#include <tr1/memory>
-#define STD_SHARED_PTR std::tr1::shared_ptr
-#else
 #include <memory>
-#define STD_SHARED_PTR std::shared_ptr
-#endif
-
 #include "ChakraCore.h"
 #include "v8-version.h"
 #include "v8config.h"
@@ -460,23 +453,19 @@ struct WeakReferenceCallbackWrapper {
   };
   bool isWeakCallbackInfo;
 };
-#ifdef _WIN32
-// xplat-todo: Is this still needed? Fail to compile xplat.
-template class V8_EXPORT STD_SHARED_PTR<WeakReferenceCallbackWrapper>;
-#endif
 
 // A helper method for setting an object with a WeakReferenceCallback. The
 // callback will be called before the object is released.
 V8_EXPORT void SetObjectWeakReferenceCallback(
   JsValueRef object,
   WeakCallbackInfo<void>::Callback callback,
   void* parameters,
-  STD_SHARED_PTR<WeakReferenceCallbackWrapper>* weakWrapper);
+  WeakReferenceCallbackWrapper** weakWrapper);
 V8_EXPORT void SetObjectWeakReferenceCallback(
   JsValueRef object,
   WeakCallbackData<Value, void>::Callback callback,
   void* parameters,
-  STD_SHARED_PTR<WeakReferenceCallbackWrapper>* weakWrapper);
+  WeakReferenceCallbackWrapper** weakWrapper);
 // A helper method for turning off the WeakReferenceCallback that was set using
 // the previous method
 V8_EXPORT void ClearObjectWeakReferenceCallback(JsValueRef object, bool revive);
@@ -544,7 +533,7 @@ class PersistentBase {
   template<class F> friend class Local;
   template<class F1, class F2> friend class Persistent;
 
-  explicit V8_INLINE PersistentBase(T* val) : val_(val) {}
+  explicit V8_INLINE PersistentBase(T* val) : val_(val), _weakWrapper(nullptr) {}
   PersistentBase(PersistentBase& other) = delete;  // NOLINT
   void operator=(PersistentBase&) = delete;
   V8_INLINE static T* New(Isolate* isolate, T* that);
@@ -553,7 +542,7 @@ class PersistentBase {
   void SetWeakCommon(P* parameter, Callback callback);
 
   T* val_;
-  STD_SHARED_PTR<chakrashim::WeakReferenceCallbackWrapper> _weakWrapper;
+  chakrashim::WeakReferenceCallbackWrapper* _weakWrapper;
 };
 
 
@@ -688,7 +677,7 @@ class Global : public PersistentBase<T> {
   V8_INLINE Global(Global&& other) : PersistentBase<T>(other.val_) {
     this->_weakWrapper = other._weakWrapper;
     other.val_ = nullptr;
-    other._weakWrapper.reset();
+    other._weakWrapper = nullptr;
   }
 
   V8_INLINE ~Global() { this->Reset(); }
@@ -701,7 +690,7 @@ class Global : public PersistentBase<T> {
       this->val_ = rhs.val_;
       this->_weakWrapper = rhs._weakWrapper;
       rhs.val_ = nullptr;
-      rhs._weakWrapper.reset();
+      rhs._weakWrapper = nullptr;
     }
     return *this;
   }
@@ -2511,10 +2500,11 @@ void PersistentBase<T>::Reset() {
   if (this->IsEmpty() || V8::IsDead()) return;
 
   if (IsWeak()) {
-    if (_weakWrapper.unique()) {
+    if (_weakWrapper) {
       chakrashim::ClearObjectWeakReferenceCallback(val_, /*revive*/false);
+      delete _weakWrapper;
+      _weakWrapper = nullptr;
     }
-    _weakWrapper.reset();
   } else {
     JsRelease(val_, nullptr);
   }
@@ -2578,10 +2568,11 @@ P* PersistentBase<T>::ClearWeak() {
   if (!IsWeak()) return nullptr;
 
   P* parameters = reinterpret_cast<P*>(_weakWrapper->parameters);
-  if (_weakWrapper.unique()) {
+  if (_weakWrapper) {
     chakrashim::ClearObjectWeakReferenceCallback(val_, /*revive*/true);
+    delete _weakWrapper;
+    _weakWrapper = nullptr;
   }
-  _weakWrapper.reset();
 
   JsAddRef(val_, nullptr);
   return parameters;

diff --git a/deps/chakrashim/src/v8persistent.cc b/deps/chakrashim/src/v8persistent.cc
@@ -61,17 +61,22 @@ template <class Callback, class Func>
 void SetObjectWeakReferenceCallbackCommon(
     JsValueRef object,
     Callback callback,
-    STD_SHARED_PTR<WeakReferenceCallbackWrapper>* weakWrapper,
+    WeakReferenceCallbackWrapper** weakWrapper,
     const Func& initWrapper) {
   if (callback == nullptr || object == JS_INVALID_REFERENCE) {
     return;
   }
 
-  if (!*weakWrapper) {
-    (*weakWrapper).reset(new WeakReferenceCallbackWrapper());
-  }
+  // This will be called once per instance
+  // We do not share the _weakWrapper.
+  // The memory is allocated per request and released similarly.
+  // previous shared_ptr interface was actually doing the same.
+  // however in case the instance was shared(which is not in motion)
+  // it was keeping the callback until the shared counter reaches to unique state.
+  assert(!*weakWrapper && "This should be nullptr.");
+  *weakWrapper = new WeakReferenceCallbackWrapper();
 
-  WeakReferenceCallbackWrapper *callbackWrapper = (*weakWrapper).get();
+  WeakReferenceCallbackWrapper *callbackWrapper = (*weakWrapper);
   initWrapper(callbackWrapper);
 
   JsSetObjectBeforeCollectCallback(
@@ -83,7 +88,7 @@ void SetObjectWeakReferenceCallback(
     JsValueRef object,
     WeakCallbackInfo<void>::Callback callback,
     void* parameters,
-    STD_SHARED_PTR<WeakReferenceCallbackWrapper>* weakWrapper) {
+    WeakReferenceCallbackWrapper** weakWrapper) {
   SetObjectWeakReferenceCallbackCommon(
     object, callback, weakWrapper,
     [=](WeakReferenceCallbackWrapper *callbackWrapper) {
@@ -97,7 +102,7 @@ void SetObjectWeakReferenceCallback(
     JsValueRef object,
     WeakCallbackData<Value, void>::Callback callback,
     void* parameters,
-    STD_SHARED_PTR<WeakReferenceCallbackWrapper>* weakWrapper) {
+    WeakReferenceCallbackWrapper** weakWrapper) {
   SetObjectWeakReferenceCallbackCommon(
     object, callback, weakWrapper,
     [=](WeakReferenceCallbackWrapper *callbackWrapper) {