Don't NULL-check GlobalHandle::Node::object_

If it's Smi::FromInt(0), the NULL check would trigger. Instead, use the handle-zap value to mean "not set". BUG=v8:3647,chromium:580651 [email protected] LOG=y Review URL: https://codereview.chromium.org/1628173002 Cr-Commit-Position: refs/heads/master@{#33492}
crosswalk-project · Jan 25, 2016 · 85f32f1 · 85f32f1
1 parent 5259af6
commit 85f32f1
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 3 deletions.
diff --git a/src/global-handles.cc b/src/global-handles.cc
@@ -78,6 +78,7 @@ class GlobalHandles::Node {
 #endif
 
   void Initialize(int index, Node** first_free) {
+    object_ = reinterpret_cast<Object*>(kGlobalHandleZapValue);
     index_ = static_cast<uint8_t>(index);
     DCHECK(static_cast<int>(index_) == index);
     set_state(FREE);
@@ -250,9 +251,9 @@ class GlobalHandles::Node {
   }
 
   void MakeWeak(void* parameter, WeakCallback weak_callback) {
-    DCHECK(weak_callback != NULL);
+    DCHECK(weak_callback != nullptr);
     DCHECK(IsInUse());
-    CHECK(object_ != NULL);
+    CHECK_NE(object_, reinterpret_cast<Object*>(kGlobalHandleZapValue));
     set_state(WEAK);
     set_weakness_type(NORMAL_WEAK);
     set_parameter(parameter);
@@ -264,7 +265,7 @@ class GlobalHandles::Node {
                 v8::WeakCallbackType type) {
     DCHECK(phantom_callback != nullptr);
     DCHECK(IsInUse());
-    CHECK(object_ != nullptr);
+    CHECK_NE(object_, reinterpret_cast<Object*>(kGlobalHandleZapValue));
     set_state(WEAK);
     switch (type) {
       case v8::WeakCallbackType::kParameter:

diff --git a/test/cctest/test-global-handles.cc b/test/cctest/test-global-handles.cc
@@ -401,3 +401,19 @@ TEST(PersistentBaseGetLocal) {
   CHECK(o == g.Get(isolate));
   CHECK(v8::Local<v8::Object>::New(isolate, g) == g.Get(isolate));
 }
+
+
+void WeakCallback(const v8::WeakCallbackInfo<void>& data) {}
+
+
+TEST(WeakPersistentSmi) {
+  CcTest::InitializeVM();
+  v8::Isolate* isolate = CcTest::isolate();
+
+  v8::HandleScope scope(isolate);
+  v8::Local<v8::Number> n = v8::Number::New(isolate, 0);
+  v8::Global<v8::Number> g(isolate, n);
+
+  // Should not crash.
+  g.SetWeak<void>(nullptr, &WeakCallback, v8::WeakCallbackType::kParameter);
+}