src: always clear wrap before persistent Reset()

trevnorris · trevnorris · commit 9dd5b93c42a2 · 2016-05-24T14:38:34.000-06:00
In case the handle is stored and accessed after the associated C++ class was destructed, set the internal pointer to nullptr so any getters/setters can return accordingly without aborting the application. PR-URL: nodejs#6184 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Anna Henningsen <anna@addaleax.net>
diff --git a/src/cares_wrap.cc b/src/cares_wrap.cc
@@ -9,6 +9,7 @@
 #include "req-wrap-inl.h"
 #include "tree.h"
 #include "util.h"
+#include "util-inl.h"
 #include "uv.h"
 
 #include <errno.h>
@@ -258,6 +259,7 @@ class QueryWrap : public AsyncWrap {
 
   virtual ~QueryWrap() override {
     CHECK_EQ(false, persistent().IsEmpty());
+    ClearWrap(object());
     persistent().Reset();
   }
 
diff --git a/src/handle_wrap.cc b/src/handle_wrap.cc
@@ -98,7 +98,7 @@ void HandleWrap::OnClose(uv_handle_t* handle) {
   if (have_close_callback)
     wrap->MakeCallback(env->onclose_string(), 0, nullptr);
 
-  wrap->object()->SetAlignedPointerInInternalField(0, nullptr);
+  ClearWrap(wrap->object());
   wrap->persistent().Reset();
   delete wrap;
 }
diff --git a/src/node_crypto.cc b/src/node_crypto.cc
@@ -5023,6 +5023,7 @@ class PBKDF2Request : public AsyncWrap {
 
   ~PBKDF2Request() override {
     release();
+    ClearWrap(object());
     persistent().Reset();
   }
 
@@ -5288,6 +5289,7 @@ class RandomBytesRequest : public AsyncWrap {
   }
 
   ~RandomBytesRequest() override {
+    ClearWrap(object());
     persistent().Reset();
   }
 
diff --git a/src/req-wrap-inl.h b/src/req-wrap-inl.h
@@ -28,6 +28,7 @@ template <typename T>
 ReqWrap<T>::~ReqWrap() {
   CHECK_EQ(req_.data, this);  // Assert that someone has called Dispatched().
   CHECK_EQ(false, persistent().IsEmpty());
+  ClearWrap(object());
   persistent().Reset();
 }
 

Original file line number	Diff line number	Diff line change
`@@ -98,7 +98,7 @@ void HandleWrap::OnClose(uv_handle_t* handle) {`
`98`	`98`	`if (have_close_callback)`
`99`	`99`	`wrap->MakeCallback(env->onclose_string(), 0, nullptr);`
`100`	`100`
`101`		`- wrap->object()->SetAlignedPointerInInternalField(0, nullptr);`
	`101`	`+ ClearWrap(wrap->object());`
`102`	`102`	`wrap->persistent().Reset();`
`103`	`103`	`delete wrap;`
`104`	`104`	`}`
Original file line number	Diff line number	Diff line change
`@@ -5023,6 +5023,7 @@ class PBKDF2Request : public AsyncWrap {`
`5023`	`5023`
`5024`	`5024`	`~PBKDF2Request() override {`
`5025`	`5025`	`release();`
	`5026`	`+ ClearWrap(object());`
`5026`	`5027`	`persistent().Reset();`
`5027`	`5028`	`}`
`5028`	`5029`
`@@ -5288,6 +5289,7 @@ class RandomBytesRequest : public AsyncWrap {`
`5288`	`5289`	`}`
`5289`	`5290`
`5290`	`5291`	`~RandomBytesRequest() override {`
	`5292`	`+ ClearWrap(object());`
`5291`	`5293`	`persistent().Reset();`
`5292`	`5294`	`}`
`5293`	`5295`
Original file line number	Diff line number	Diff line change
`@@ -28,6 +28,7 @@ template <typename T>`
`28`	`28`	`ReqWrap<T>::~ReqWrap() {`
`29`	`29`	`CHECK_EQ(req_.data, this); // Assert that someone has called Dispatched().`
`30`	`30`	`CHECK_EQ(false, persistent().IsEmpty());`
	`31`	`+ ClearWrap(object());`
`31`	`32`	`persistent().Reset();`
`32`	`33`	`}`
`33`	`34`