diff --git a/platform/node/src/node_map.cpp b/platform/node/src/node_map.cpp index 7ed826275..e56f31b0e 100644 --- a/platform/node/src/node_map.cpp +++ b/platform/node/src/node_map.cpp @@ -528,12 +528,13 @@ std::unique_ptr NodeMap::request(const mbgl::Resource& resou Nan::HandleScope scope; auto requestHandle = NodeRequest::Create(resource, callback_)->ToObject(); + auto request = Nan::ObjectWrap::Unwrap(requestHandle); auto callbackHandle = Nan::New(NodeRequest::Respond, requestHandle); v8::Local argv[] = { requestHandle, callbackHandle }; Nan::MakeCallback(handle()->GetInternalField(1)->ToObject(), "request", 2, argv); - return std::make_unique(); + return std::make_unique(request); } -} +} // namespace node_mbgl diff --git a/platform/node/src/node_request.cpp b/platform/node/src/node_request.cpp index d1a40a908..fa560ed4e 100644 --- a/platform/node/src/node_request.cpp +++ b/platform/node/src/node_request.cpp @@ -45,6 +45,14 @@ v8::Handle NodeRequest::Create(const mbgl::Resource& resource, mbgl: NAN_METHOD(NodeRequest::Respond) { using Error = mbgl::Response::Error; + // Move out of the object so callback() can only be fired once. + auto request = Nan::ObjectWrap::Unwrap(info.Data().As()); + auto callback = std::move(request->callback); + if (!callback) { + info.GetReturnValue().SetUndefined(); + return; + } + mbgl::Response response; if (info.Length() < 1) { @@ -114,14 +122,39 @@ NAN_METHOD(NodeRequest::Respond) { } // Send the response object to the NodeFileSource object - Nan::ObjectWrap::Unwrap(info.Data().As())->callback(response); + callback(response); info.GetReturnValue().SetUndefined(); } //////////////////////////////////////////////////////////////////////////////////////////////// // Instance -NodeRequest::NodeRequest(mbgl::FileSource::Callback callback_) - : callback(callback_) {} - +NodeRequest::NodeAsyncRequest::NodeAsyncRequest(NodeRequest* request_) : request(request_) { + assert(request); + // Make sure the JS object has a pointer to this so that it can remove its pointer in the + // destructor + request->asyncRequest = this; } + +NodeRequest::NodeAsyncRequest::~NodeAsyncRequest() { + if (request) { + // Remove the callback function because the AsyncRequest was canceled and we are no longer + // interested in the result. + request->callback = {}; + request->asyncRequest = nullptr; + } +} + +NodeRequest::NodeRequest(mbgl::FileSource::Callback callback_) + : callback(callback_) { +} + +NodeRequest::~NodeRequest() { + // When this object gets garbage collected, make sure that the AsyncRequest can no longer + // attempt to remove the callback function this object was holding (it can't be fired anymore). + if (asyncRequest) { + asyncRequest->request = nullptr; + } +} + +} // namespace node_mbgl diff --git a/platform/node/src/node_request.hpp b/platform/node/src/node_request.hpp index 8e57cb30e..2d307a3f1 100644 --- a/platform/node/src/node_request.hpp +++ b/platform/node/src/node_request.hpp @@ -12,6 +12,7 @@ namespace node_mbgl { class NodeFileSource; +class NodeRequest; class NodeRequest : public Nan::ObjectWrap { public: @@ -24,9 +25,17 @@ public: static Nan::Persistent constructor; NodeRequest(mbgl::FileSource::Callback); + ~NodeRequest(); + + struct NodeAsyncRequest : public mbgl::AsyncRequest { + NodeAsyncRequest(NodeRequest*); + ~NodeAsyncRequest() override; + NodeRequest* request; + }; private: mbgl::FileSource::Callback callback; + NodeAsyncRequest* asyncRequest = nullptr; }; } diff --git a/platform/node/test/js/map.test.js b/platform/node/test/js/map.test.js index 12b17126f..e8434bc77 100644 --- a/platform/node/test/js/map.test.js +++ b/platform/node/test/js/map.test.js @@ -220,6 +220,26 @@ test('Map', function(t) { t.end(); }); + t.test('returns an error delayed', function(t) { + var delay = 0; + var map = new mbgl.Map({ + request: function(req, callback) { + delay += 100; + setTimeout(function() { + callback(new Error('not found')); + }, delay); + }, + ratio: 1 + }); + map.load(style); + map.render({ zoom: 1 }, function(err, data) { + map.release(); + + t.ok(err, 'returns error'); + t.end(); + }); + }); + t.test('returns an error', function(t) { var map = new mbgl.Map(options); map.load(style);