[node] Allow GC in spite of reference loops through the request method

The following produces a reference cycle:

var map = new mbgl.Map({
    request: function () { ... }
});

map has a reference to the request function, which has a reference to
its closure, which has a reference to map.

Storing a the map options in an internal field, rather than a Persistent,
allows V8 to collect the reference cycle when none of its members have
other references.

5 files changed, 35 insertions, 71 deletions

diff --git a/platform/node/src/node_file_source.cpp b/platform/node/src/node_file_source.cpp
deleted file mode 100644
index 5d7973f851..0000000000
--- a/platform/node/src/node_file_source.cpp
+++ /dev/null
@@ -1,38 +0,0 @@
-#include "node_file_source.hpp"
-#include "node_request.hpp"
-#include "node_mapbox_gl_native.hpp"
-
-namespace node_mbgl {
-
-class NodeFileSourceRequest : public mbgl::FileRequest {
-public:
-    std::unique_ptr<mbgl::WorkRequest> workRequest;
-};
-
-NodeFileSource::NodeFileSource(v8::Local<v8::Object> options_) {
-    options.Reset(options_);
-}
-
-NodeFileSource::~NodeFileSource() {
-    options.Reset();
-}
-
-std::unique_ptr<mbgl::FileRequest> NodeFileSource::request(const mbgl::Resource& resource, Callback callback) {
-    auto req = std::make_unique<NodeFileSourceRequest>();
-
-    // This function can be called from any thread. Make sure we're executing the
-    // JS implementation in the node event loop.
-    req->workRequest = NodeRunLoop().invokeWithCallback([this] (mbgl::Resource res, Callback cb) {
-        Nan::HandleScope scope;
-
-        auto requestHandle = NodeRequest::Create(res, cb)->ToObject();
-        auto callbackHandle = Nan::GetFunction(Nan::New<v8::FunctionTemplate>(NodeRequest::Respond, requestHandle)).ToLocalChecked();
-
-        v8::Local<v8::Value> argv[] = { requestHandle, callbackHandle };
-        Nan::MakeCallback(Nan::New(options), "request", 2, argv);
-    }, callback, resource);
-
-    return std::move(req);
-}
-
-}
diff --git a/platform/node/src/node_file_source.hpp b/platform/node/src/node_file_source.hpp
deleted file mode 100644
index 2ce673fb50..0000000000
--- a/platform/node/src/node_file_source.hpp
+++ /dev/null
@@ -1,25 +0,0 @@
-#pragma once
-
-#include <mbgl/storage/file_source.hpp>
-
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wunused-parameter"
-#pragma GCC diagnostic ignored "-Wshadow"
-#pragma GCC diagnostic ignored "-Wnested-anon-types"
-#include <nan.h>
-#pragma GCC diagnostic pop
-
-namespace node_mbgl {
-
-class NodeFileSource : public mbgl::FileSource {
-public:
-    NodeFileSource(v8::Local<v8::Object>);
-    ~NodeFileSource();
-
-    std::unique_ptr<mbgl::FileRequest> request(const mbgl::Resource&, Callback);
-
-private:
-    Nan::Persistent<v8::Object> options;
-};
-
-}
diff --git a/platform/node/src/node_map.cpp b/platform/node/src/node_map.cpp
index 6e9230287e..99fbb919e4 100644
--- a/platform/node/src/node_map.cpp
+++ b/platform/node/src/node_map.cpp
@@ -1,8 +1,11 @@
 #include "node_map.hpp"
+#include "node_request.hpp"
+#include "node_mapbox_gl_native.hpp"
 
 #include <mbgl/platform/default/headless_display.hpp>
 #include <mbgl/map/still_image.hpp>
 #include <mbgl/util/exception.hpp>
+#include <mbgl/util/work_request.hpp>
 
 #include <unistd.h>
 
@@ -42,7 +45,7 @@ const static char* releasedMessage() {
 NAN_MODULE_INIT(NodeMap::Init) {
     v8::Local<v8::FunctionTemplate> tpl = Nan::New<v8::FunctionTemplate>(New);
 
-    tpl->InstanceTemplate()->SetInternalFieldCount(1);
+    tpl->InstanceTemplate()->SetInternalFieldCount(2);
     tpl->SetClassName(Nan::New("Map").ToLocalChecked());
 
     Nan::SetPrototypeMethod(tpl, "load", Load);
@@ -124,6 +127,8 @@ NAN_METHOD(NodeMap::New) {
         return Nan::ThrowError("Options object 'ratio' property must be a number");
     }
 
+    info.This()->SetInternalField(1, options);
+
     try {
         auto nodeMap = new NodeMap(options);
         nodeMap->Wrap(info.This());
@@ -427,8 +432,7 @@ NodeMap::NodeMap(v8::Local<v8::Object> options) :
         Nan::HandleScope scope;
         return Nan::Has(options, Nan::New("ratio").ToLocalChecked()).FromJust() ? Nan::Get(options, Nan::New("ratio").ToLocalChecked()).ToLocalChecked()->NumberValue() : 1.0;
     }()),
-    fs(options),
-    map(std::make_unique<mbgl::Map>(view, fs, mbgl::MapMode::Still)),
+    map(std::make_unique<mbgl::Map>(view, *this, mbgl::MapMode::Still)),
     async(new uv_async_t) {
 
     async->data = this;
@@ -444,4 +448,27 @@ NodeMap::~NodeMap() {
     if (valid) release();
 }
 
+class NodeFileSourceRequest : public mbgl::FileRequest {
+public:
+    std::unique_ptr<mbgl::WorkRequest> workRequest;
+};
+
+std::unique_ptr<mbgl::FileRequest> NodeMap::request(const mbgl::Resource& resource, Callback cb1) {
+    auto req = std::make_unique<NodeFileSourceRequest>();
+
+    // This function can be called from any thread. Make sure we're executing the
+    // JS implementation in the node event loop.
+    req->workRequest = NodeRunLoop().invokeWithCallback([this] (mbgl::Resource res, Callback cb2) {
+        Nan::HandleScope scope;
+
+        auto requestHandle = NodeRequest::Create(res, cb2)->ToObject();
+        auto callbackHandle = Nan::GetFunction(Nan::New<v8::FunctionTemplate>(NodeRequest::Respond, requestHandle)).ToLocalChecked();
+
+        v8::Local<v8::Value> argv[] = { requestHandle, callbackHandle };
+        Nan::MakeCallback(handle()->GetInternalField(1)->ToObject(), "request", 2, argv);
+    }, cb1, resource);
+
+    return std::move(req);
+}
+
 }
diff --git a/platform/node/src/node_map.hpp b/platform/node/src/node_map.hpp
index 91cada17d1..f610408d3f 100644
--- a/platform/node/src/node_map.hpp
+++ b/platform/node/src/node_map.hpp
@@ -1,8 +1,7 @@
 #pragma once
 
-#include "node_file_source.hpp"
-
 #include <mbgl/map/map.hpp>
+#include <mbgl/storage/file_source.hpp>
 #include <mbgl/platform/default/headless_view.hpp>
 
 #pragma GCC diagnostic push
@@ -16,7 +15,8 @@
 
 namespace node_mbgl {
 
-class NodeMap : public Nan::ObjectWrap {
+class NodeMap : public Nan::ObjectWrap,
+                public mbgl::FileSource {
 public:
     struct RenderOptions;
     class RenderWorker;
@@ -43,8 +43,9 @@ public:
     NodeMap(v8::Local<v8::Object>);
     ~NodeMap();
 
+    std::unique_ptr<mbgl::FileRequest> request(const mbgl::Resource&, Callback);
+
     mbgl::HeadlessView view;
-    NodeFileSource fs;
     std::unique_ptr<mbgl::Map> map;
 
     std::exception_ptr error;
diff --git a/platform/node/src/node_request.cpp b/platform/node/src/node_request.cpp
index e08e4ffd17..4575d42a7a 100644
--- a/platform/node/src/node_request.cpp
+++ b/platform/node/src/node_request.cpp
@@ -1,5 +1,4 @@
 #include "node_request.hpp"
-#include "node_file_source.hpp"
 #include <mbgl/storage/response.hpp>
 
 #include <cmath>