From b602835626dfc1532bf5c26b9d2c34054a2d8fac Mon Sep 17 00:00:00 2001 From: "Thiago Marcos P. Santos" Date: Mon, 15 Jun 2015 01:55:20 +0300 Subject: [PATCH] Fix invalid NodeRequest reference On the code path built with node v0.12.4, we are storing an invalid reference to NodeRequest on the "pending" list. It will crash when we need to access it like we do when canceling a request. Ideally we should just have a "std::unordered_map" but that is not possible because the move semantics are not fully implemented until V8 v4.3.22. It can be used on a std::vector but not on a std::map. See: https://code.google.com/p/v8/issues/detail?id=3669 The solution for keep using C++11 move semantics is to declare the v8::CopyablePersistentTraits shenanigans and explicitly clear the v8::Persistent (like we do already for node v0.10). When Node gets V8 v4.3.22+ we can go back to v8::UniquePersistent with a more clear implementation. --- src/node_file_source.cpp | 14 +++++--------- src/node_file_source.hpp | 2 +- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/node_file_source.cpp b/src/node_file_source.cpp index 935bac427..3409f03af 100644 --- a/src/node_file_source.cpp +++ b/src/node_file_source.cpp @@ -121,12 +121,9 @@ void NodeFileSource::processAdd(const mbgl::Resource& resource) { auto handle = NanObjectWrapHandle(this); auto requestHandle = NanNew(NodeRequest::Create(handle, resource)); -#if (NODE_MODULE_VERSION > NODE_0_10_MODULE_VERSION) - const v8::UniquePersistent requestPersistent(v8::Isolate::GetCurrent(), requestHandle); -#else + v8::Persistent requestPersistent; NanAssignPersistent(requestPersistent, requestHandle); -#endif pending.emplace(resource, std::move(requestPersistent)); v8::Local argv[] = { requestHandle }; @@ -143,12 +140,9 @@ void NodeFileSource::processCancel(const mbgl::Resource& resource) { } else { #if (NODE_MODULE_VERSION > NODE_0_10_MODULE_VERSION) auto requestHandle = v8::Local::New(v8::Isolate::GetCurrent(), it->second); + it->second.Reset(); #else auto requestHandle = NanNew(it->second); -#endif - - // Dispose and remove the persistent handle -#if (NODE_MODULE_VERSION <= NODE_0_10_MODULE_VERSION) NanDisposePersistent(it->second); #endif pending.erase(it); @@ -173,7 +167,9 @@ void NodeFileSource::notify(const mbgl::Resource& resource, const std::shared_pt // First, remove the request, since it might be destructed at any point now. auto it = pending.find(resource); if (it != pending.end()) { -#if (NODE_MODULE_VERSION <= NODE_0_10_MODULE_VERSION) +#if (NODE_MODULE_VERSION > NODE_0_10_MODULE_VERSION) + it->second.Reset(); +#else NanDisposePersistent(it->second); #endif pending.erase(it); diff --git a/src/node_file_source.hpp b/src/node_file_source.hpp index 5220cd235..06b1c3d32 100644 --- a/src/node_file_source.hpp +++ b/src/node_file_source.hpp @@ -49,7 +49,7 @@ private: private: #if (NODE_MODULE_VERSION > NODE_0_10_MODULE_VERSION) - std::unordered_map &, mbgl::Resource::Hash> pending; + std::unordered_map>, mbgl::Resource::Hash> pending; #else std::unordered_map, mbgl::Resource::Hash> pending; #endif