remove circular shared_ptr and a few other memory leaks

6 files changed, 74 insertions, 34 deletions

diff --git a/src/mbgl/map/map.cpp b/src/mbgl/map/map.cpp
index 112499a485..69056cb57a 100644
--- a/src/mbgl/map/map.cpp
+++ b/src/mbgl/map/map.cpp
@@ -192,8 +192,6 @@ void Map::start(bool startPaused) {
         pthread_setname_np("Map");
 #endif
 
-        workers = util::make_unique<uv::worker>(**loop, 4, "Tile Worker");
-
         run();
 
 #ifndef NDEBUG
@@ -276,6 +274,9 @@ void Map::run() {
     }
 
     view.activate();
+
+    workers = util::make_unique<uv::worker>(**loop, 4, "Tile Worker");
+
     setup();
     prepare();
 
diff --git a/src/mbgl/map/vector_tile_data.cpp b/src/mbgl/map/vector_tile_data.cpp
index ca98d2695b..646ad7318a 100644
--- a/src/mbgl/map/vector_tile_data.cpp
+++ b/src/mbgl/map/vector_tile_data.cpp
@@ -36,6 +36,10 @@ void VectorTileData::parse() {
     }
 
     try {
+        if (!style) {
+            throw std::runtime_error("style isn't present in VectorTileData object anymore");
+        }
+
         // Parsing creates state that is encapsulated in TileParser. While parsing,
         // the TileParser object writes results into this objects. All other state
         // is going to be discarded afterwards.
@@ -43,6 +47,9 @@ void VectorTileData::parse() {
                           glyphAtlas, glyphStore,
                           spriteAtlas, sprite,
                           texturePool);
+        // Clear the style so that we don't have a cycle in the shared_ptr references.
+        style.reset();
+
         parser.parse();
     } catch (const std::exception& ex) {
         Log::Error(Event::ParseTile, "Parsing [%d/%d/%d] failed: %s", id.z, id.x, id.y, ex.what());
diff --git a/src/mbgl/storage/request.cpp b/src/mbgl/storage/request.cpp
index c771acb929..1bd58571a1 100644
--- a/src/mbgl/storage/request.cpp
+++ b/src/mbgl/storage/request.cpp
@@ -20,26 +20,29 @@ Request::Request(const Resource &resource_, uv_loop_t *loop, Callback callback_)
     if (loop) {
         notify_async = new uv_async_t;
         notify_async->data = this;
-        uv_async_init(loop, notify_async, [](uv_async_t *async, int) {
-            auto request = reinterpret_cast<Request *>(async->data);
-            uv::close(async);
-
-            if (!request->destruct_async) {
-                // We haven't created a cancel request, so we can safely delete this Request object
-                // since it won't be accessed in the future.
-                assert(request->response);
-                request->callback(*request->response);
-                delete request;
-            } else {
-                // Otherwise, we're waiting for for the destruct notification to be delivered in order
-                // to delete the Request object. We're doing this since we can't know whether the
-                // DefaultFileSource is still sending a cancel event, which means this object must still
-                // exist.
-            }
-        });
+        uv_async_init(loop, notify_async, notifyCallback);
     }
 }
 
+void Request::notifyCallback(uv_async_t *async, int) {
+    auto request = reinterpret_cast<Request *>(async->data);
+    uv::close(async);
+
+    if (!request->destruct_async) {
+        // We haven't created a cancel request, so we can safely delete this Request object
+        // since it won't be accessed in the future.
+        assert(request->response);
+        request->callback(*request->response);
+        delete request;
+    } else {
+        // Otherwise, we're waiting for for the destruct notification to be delivered in order
+        // to delete the Request object. We're doing this since we can't know whether the
+        // DefaultFileSource is still sending a cancel event, which means this object must still
+        // exist.
+    }
+}
+
+
 Request::~Request() {
     if (notify_async) {
         // Request objects can be destructed in other threads when the user didn't supply a loop.
@@ -64,12 +67,14 @@ void Request::cancel() {
     assert(!destruct_async);
     destruct_async = new uv_async_t;
     destruct_async->data = this;
-    uv_async_init(notify_async->loop, destruct_async, [](uv_async_t *async, int) {
-        // The destruct_async will be invoked *after* the notify_async callback has already run.
-        auto request = reinterpret_cast<Request *>(async->data);
-        uv::close(async);
-        delete request;
-    });
+    uv_async_init(notify_async->loop, destruct_async, cancelCallback);
+}
+
+void Request::cancelCallback(uv_async_t *async, int) {
+    // The destruct_async will be invoked *after* the notify_async callback has already run.
+    auto request = reinterpret_cast<Request *>(async->data);
+    uv::close(async);
+    delete request;
 }
 
 // This gets called from the FileSource thread, and will only ever be invoked after cancel() was called
diff --git a/src/mbgl/style/style.hpp b/src/mbgl/style/style.hpp
index 5517a56a71..751a91be62 100644
--- a/src/mbgl/style/style.hpp
+++ b/src/mbgl/style/style.hpp
@@ -21,7 +21,7 @@ class Sprite;
 class StyleLayer;
 class StyleLayerGroup;
 
-class Style {
+class Style : public util::noncopyable {
 public:
     struct exception : std::runtime_error { exception(const char *msg) : std::runtime_error(msg) {} };
 
diff --git a/src/mbgl/style/style_bucket.cpp b/src/mbgl/style/style_bucket.cpp
index 20d5d1147d..6e866f3035 100644
--- a/src/mbgl/style/style_bucket.cpp
+++ b/src/mbgl/style/style_bucket.cpp
@@ -2,12 +2,20 @@
 
 namespace mbgl {
 
-StyleBucket::StyleBucket(StyleLayerType type_) :type(type_) {}
-
 template<> const StyleBucketFill &defaultLayoutProperties() { static StyleBucketFill p; return p; }
 template<> const StyleBucketLine &defaultLayoutProperties() { static StyleBucketLine p; return p; }
 template<> const StyleBucketSymbol &defaultLayoutProperties() { static StyleBucketSymbol p; return p; }
 template<> const StyleBucketRaster &defaultLayoutProperties() { static StyleBucketRaster p; return p; }
 template<> const StyleBucketBackground &defaultLayoutProperties() { static StyleBucketBackground p; return p; }
 
+StyleBucket::StyleBucket(StyleLayerType type_) : type(type_) {
+    switch (type) {
+        case StyleLayerType::Fill: render.set<StyleBucketFill>(); break;
+        case StyleLayerType::Line: render.set<StyleBucketLine>(); break;
+        case StyleLayerType::Symbol: render.set<StyleBucketSymbol>(); break;
+        case StyleLayerType::Raster: render.set<StyleBucketRaster>(); break;
+        default: break;
+    }
+}
+
 }
\ No newline at end of file
diff --git a/src/mbgl/style/style_bucket.hpp b/src/mbgl/style/style_bucket.hpp
index 08884c2e09..eac899cb97 100644
--- a/src/mbgl/style/style_bucket.hpp
+++ b/src/mbgl/style/style_bucket.hpp
@@ -19,10 +19,23 @@ class Source;
 
 class StyleBucketFill {
 public:
+    // Make movable only.
+    StyleBucketFill() = default;
+    StyleBucketFill(StyleBucketFill &&) = default;
+    StyleBucketFill& operator=(StyleBucketFill &&) = default;
+    StyleBucketFill(const StyleBucketFill &) = delete;
+    StyleBucketFill& operator=(const StyleBucketFill &) = delete;
 };
 
 class StyleBucketLine {
 public:
+    // Make movable only.
+    StyleBucketLine() = default;
+    StyleBucketLine(StyleBucketLine &&) = default;
+    StyleBucketLine& operator=(StyleBucketLine &&) = default;
+    StyleBucketLine(const StyleBucketLine &) = delete;
+    StyleBucketLine& operator=(const StyleBucketLine &) = delete;
+
     CapType cap = CapType::Butt;
     JoinType join = JoinType::Miter;
     float miter_limit = 2.0f;
@@ -32,11 +45,11 @@ public:
 class StyleBucketSymbol {
 public:
     // Make movable only.
-    inline StyleBucketSymbol() = default;
-    inline StyleBucketSymbol(StyleBucketSymbol &&) = default;
-    inline StyleBucketSymbol& operator=(StyleBucketSymbol &&) = default;
-    inline StyleBucketSymbol(const StyleBucketSymbol &) = delete;
-    inline StyleBucketSymbol& operator=(const StyleBucketSymbol &) = delete;
+    StyleBucketSymbol() = default;
+    StyleBucketSymbol(StyleBucketSymbol &&) = default;
+    StyleBucketSymbol& operator=(StyleBucketSymbol &&) = default;
+    StyleBucketSymbol(const StyleBucketSymbol &) = delete;
+    StyleBucketSymbol& operator=(const StyleBucketSymbol &) = delete;
 
     PlacementType placement = PlacementType::Point;
     float min_distance = 250.0f;
@@ -79,6 +92,12 @@ public:
 
 class StyleBucketRaster {
 public:
+    // Make movable only.
+    StyleBucketRaster() = default;
+    StyleBucketRaster(StyleBucketRaster &&) = default;
+    StyleBucketRaster& operator=(StyleBucketRaster &&) = default;
+    StyleBucketRaster(const StyleBucketRaster &) = delete;
+    StyleBucketRaster& operator=(const StyleBucketRaster &) = delete;
 };
 
 class StyleBucketBackground {
@@ -89,7 +108,7 @@ typedef mapbox::util::variant<StyleBucketFill, StyleBucketLine, StyleBucketSymbo
                               StyleBucketRaster, StyleBucketBackground, std::false_type> StyleBucketRender;
 
 
-class StyleBucket {
+class StyleBucket : public util::noncopyable {
 public:
     typedef util::ptr<StyleBucket> Ptr;