From fe3d6bbbd5ac82b7c3b5ae51e33a84268333dfde Mon Sep 17 00:00:00 2001 From: Mike Morris Date: Wed, 20 May 2015 17:19:36 -0400 Subject: [PATCH] expose map.release() method --- .travis.yml | 1 + src/node_map.cpp | 72 +++++++++++++++++++++++++++---------- src/node_map.hpp | 8 ++++- test/js/concurrency.test.js | 2 ++ test/js/gzip.test.js | 4 +++ test/js/map.test.js | 55 ++++++++++++++++++++++++---- test/render.test.js | 2 ++ 7 files changed, 117 insertions(+), 27 deletions(-) diff --git a/.travis.yml b/.travis.yml index 8a7f5c345..d2a5b8530 100644 --- a/.travis.yml +++ b/.travis.yml @@ -100,6 +100,7 @@ install: before_script: - ulimit -c unlimited -S +- ulimit -a script: - "./scripts/travis_test.sh" diff --git a/src/node_map.cpp b/src/node_map.cpp index 4dc39b729..89ba8d3b9 100644 --- a/src/node_map.cpp +++ b/src/node_map.cpp @@ -30,6 +30,10 @@ static std::shared_ptr sharedDisplay() { return display; } +const static char* releasedMessage() { + return "Map resources have already been released"; +} + void NodeMap::Init(v8::Handle target) { NanScope(); @@ -40,6 +44,7 @@ void NodeMap::Init(v8::Handle target) { NODE_SET_PROTOTYPE_METHOD(t, "load", Load); NODE_SET_PROTOTYPE_METHOD(t, "render", Render); + NODE_SET_PROTOTYPE_METHOD(t, "release", Release); NanAssignPersistent(constructorTemplate, t); @@ -72,8 +77,8 @@ NAN_METHOD(NodeMap::New) { try { - auto map = new NodeMap(source); - map->Wrap(args.This()); + auto nodeMap = new NodeMap(source); + nodeMap->Wrap(args.This()); } catch(std::exception &ex) { return NanThrowError(ex.what()); } @@ -81,7 +86,6 @@ NAN_METHOD(NodeMap::New) { NanReturnValue(args.This()); } - const std::string StringifyStyle(v8::Handle styleHandle) { NanScope(); @@ -94,6 +98,10 @@ const std::string StringifyStyle(v8::Handle styleHandle) { NAN_METHOD(NodeMap::Load) { NanScope(); + auto nodeMap = node::ObjectWrap::Unwrap(args.Holder()); + + if (!nodeMap->isValid()) return NanThrowError(releasedMessage()); + if (args.Length() < 1) { return NanThrowError("Requires a map style as first argument"); } @@ -108,10 +116,8 @@ NAN_METHOD(NodeMap::Load) { return NanThrowTypeError("First argument must be a string or object"); } - auto nodeMap = node::ObjectWrap::Unwrap(args.Holder()); - try { - nodeMap->map.setStyleJSON(style, "."); + nodeMap->map->setStyleJSON(style, "."); } catch (const std::exception &ex) { return NanThrowError(ex.what()); } @@ -150,6 +156,10 @@ std::unique_ptr NodeMap::ParseOptions(v8::Local(args.Holder()); + + if (!nodeMap->isValid()) return NanThrowError(releasedMessage()); + if (args.Length() <= 0 || !args[0]->IsObject()) { return NanThrowTypeError("First argument must be an options object"); } @@ -160,8 +170,6 @@ NAN_METHOD(NodeMap::Render) { auto options = ParseOptions(args[0]->ToObject()); - auto nodeMap = node::ObjectWrap::Unwrap(args.Holder()); - assert(!nodeMap->callback); assert(!nodeMap->image); nodeMap->callback = std::unique_ptr(new NanCallback(args[1].As())); @@ -176,12 +184,12 @@ NAN_METHOD(NodeMap::Render) { } void NodeMap::startRender(std::unique_ptr options) { - map.resize(options->width, options->height, options->ratio); - map.setClasses(options->classes); - map.setLatLngZoom(mbgl::LatLng(options->latitude, options->longitude), options->zoom); - map.setBearing(options->bearing); + map->resize(options->width, options->height, options->ratio); + map->setClasses(options->classes); + map->setLatLngZoom(mbgl::LatLng(options->latitude, options->longitude), options->zoom); + map->setBearing(options->bearing); - map.renderStill([this](const std::exception_ptr eptr, std::unique_ptr result) { + map->renderStill([this](const std::exception_ptr eptr, std::unique_ptr result) { if (eptr) { error = std::move(eptr); uv_async_send(async); @@ -261,6 +269,36 @@ void NodeMap::renderFinished() { } } +NAN_METHOD(NodeMap::Release) { + NanScope(); + + auto nodeMap = node::ObjectWrap::Unwrap(args.Holder()); + + if (!nodeMap->isValid()) return NanThrowError(releasedMessage()); + + try { + nodeMap->release(); + } catch (const std::exception &ex) { + return NanThrowError(ex.what()); + } + + NanReturnUndefined(); +} + +void NodeMap::release() { + if (!isValid()) throw mbgl::util::Exception(releasedMessage()); + + valid = false; + + NanDisposePersistent(source); + + uv_close(reinterpret_cast(async), [] (uv_handle_t *handle) { + delete reinterpret_cast(handle); + }); + + map.reset(nullptr); +} + //////////////////////////////////////////////////////////////////////////////////////////////// // Instance @@ -268,7 +306,7 @@ void NodeMap::renderFinished() { NodeMap::NodeMap(v8::Handle source_) : view(sharedDisplay()), fs(*ObjectWrap::Unwrap(source_)), - map(view, fs, mbgl::MapMode::Still), + map(std::make_unique(view, fs, mbgl::MapMode::Still)), async(new uv_async_t) { NanAssignPersistent(source, source_); @@ -287,11 +325,7 @@ NodeMap::NodeMap(v8::Handle source_) : } NodeMap::~NodeMap() { - NanDisposePersistent(source); - - uv_close(reinterpret_cast(async), [] (uv_handle_t *handle) { - delete reinterpret_cast(handle); - }); + if (valid) release(); } } diff --git a/src/node_map.hpp b/src/node_map.hpp index ecba02867..bb238ea88 100644 --- a/src/node_map.hpp +++ b/src/node_map.hpp @@ -27,10 +27,14 @@ public: static NAN_METHOD(New); static NAN_METHOD(Load); static NAN_METHOD(Render); + static NAN_METHOD(Release); void startRender(std::unique_ptr options); void renderFinished(); + void release(); + inline bool isValid() { return valid; } + static std::unique_ptr ParseOptions(v8::Local obj); static v8::Persistent constructorTemplate; @@ -47,7 +51,7 @@ private: mbgl::HeadlessView view; NodeFileSource &fs; - mbgl::Map map; + std::unique_ptr map; std::exception_ptr error; std::unique_ptr image; @@ -55,6 +59,8 @@ private: // Async for delivering the notifications of render completion. uv_async_t *async; + + bool valid = true; }; } diff --git a/test/js/concurrency.test.js b/test/js/concurrency.test.js index e8c373adc..e57767fa7 100644 --- a/test/js/concurrency.test.js +++ b/test/js/concurrency.test.js @@ -31,8 +31,10 @@ function renderTest(style, info, dir, key) { function render() { map.render(info[key], function(err, image) { t.error(err); + t.ok(true, 'render @ ' + ((+new Date) - start) + 'ms'); if (++completed === remaining) { + map.release(); t.end(); } else { render(); diff --git a/test/js/gzip.test.js b/test/js/gzip.test.js index 00f827b68..9b8d3e297 100644 --- a/test/js/gzip.test.js +++ b/test/js/gzip.test.js @@ -69,6 +69,8 @@ test('gzip', function(t) { map.render({}, function(err, data) { t.error(err); + map.release(); + mbgl.removeAllListeners('message'); var filename = filePath('success.png'); @@ -119,6 +121,8 @@ test('gzip', function(t) { map.render({}, function(err, data) { if (!errorEmitted) t.fail('no error emitted'); + map.release(); + mbgl.removeAllListeners('message'); var filename = filePath('unhandled.png'); diff --git a/test/js/map.test.js b/test/js/map.test.js index de5126dc9..cdb3ffe54 100644 --- a/test/js/map.test.js +++ b/test/js/map.test.js @@ -94,19 +94,25 @@ test('Map', function(t) { t.throws(function() { map.load(); }, /Requires a map style as first argument/); + + map.release(); + t.end(); }); }); t.test('expect either an object or array at root', { timeout: 1000 }, function(t) { - mbgl.once('message', function(msg) { - t.equal(msg.severity, 'ERROR'); - t.equal(msg.class, 'ParseStyle'); - t.ok(msg.text.match(/Expect either an object or array at root/)); - t.end(); - }); - setup(fileSource, function(map) { + mbgl.once('message', function(msg) { + t.equal(msg.severity, 'ERROR'); + t.equal(msg.class, 'ParseStyle'); + t.ok(msg.text.match(/Expect either an object or array at root/)); + + map.release(); + + t.end(); + }); + map.load('invalid'); }); }); @@ -119,6 +125,9 @@ test('Map', function(t) { t.doesNotThrow(function() { map.load('{}'); }); + + map.release(); + t.end(); }); }); @@ -132,6 +141,9 @@ test('Map', function(t) { t.doesNotThrow(function() { map.load(style); }); + + map.release(); + t.end(); }); }); @@ -145,6 +157,9 @@ test('Map', function(t) { t.doesNotThrow(function() { map.load(JSON.stringify(style)); }); + + map.release(); + t.end(); }); }); @@ -171,6 +186,8 @@ test('Map', function(t) { map.render('invalid'); }, /First argument must be an options object/); + map.release(); + t.end(); }); }); @@ -185,6 +202,8 @@ test('Map', function(t) { map.render({}, 'invalid'); }, /Second argument must be a callback function/); + map.release(); + t.end(); }); }); @@ -194,14 +213,22 @@ test('Map', function(t) { t.throws(function() { map.render({}, function() {}); }, /Style is not set/); + + map.release(); + t.end(); }); }); t.test('returns an error', function(t) { + mbgl.once('message', function(msg) { + console.log(msg); + }); + setup(fileSource, function(map) { map.load(style); map.render({ zoom: 1 }, function(err, data) { + map.release(); t.ok(err); t.equal(err.message, 'Error rendering image'); t.end(); @@ -209,12 +236,26 @@ test('Map', function(t) { }); }); + t.test('double release', function(t) { + setup(fileSource, function(map) { + map.release(); + + t.throws(function() { + map.release(); + }, /Map resources have already been released/); + + t.end(); + }); + }); + t.test('returns an image', function(t) { setup(fileSource, function(map) { map.load(style); map.render({}, function(err, data) { t.error(err); + map.release(); + var filename = filePath('image.png'); var png = new PNG({ diff --git a/test/render.test.js b/test/render.test.js index 7249cbb7b..f55fa5608 100644 --- a/test/render.test.js +++ b/test/render.test.js @@ -54,6 +54,8 @@ function renderTest(style, info, base, key) { map.render(info[key], function(err, data) { t.error(err); + map.release(); + var expected = path.join(dir, 'expected.png'); var actual = path.join(dir, 'actual.png'); var diff = path.join(dir, 'diff.png');