remove circular shared_ptr and a few other memory leaks

This commit is contained in:
Konstantin Käfer
2015-01-19 17:40:24 +01:00
parent 5602ba2f07
commit 31a0d18ca1
14 changed files with 159 additions and 85 deletions

View File

@@ -13,7 +13,7 @@
'outputs': [
'<(SHARED_INTERMEDIATE_DIR)/include/mbgl/util/version.hpp',
],
'action': ['<@(python)', 'scripts/build-version.py', '<(SHARED_INTERMEDIATE_DIR)', '<!@(git describe --tags --always --abbrev=0)', '<!@(git rev-parse HEAD)'],
'action': ['<@(python)', '<@(_inputs)', '<(SHARED_INTERMEDIATE_DIR)', '<!@(git describe --tags --always --abbrev=0)', '<!@(git rev-parse HEAD)'],
}
],
'direct_dependent_settings': {

View File

@@ -31,6 +31,10 @@ public:
// May be called only from the thread the Request was created in.
void cancel();
private:
static void notifyCallback(uv_async_t *async, int);
static void cancelCallback(uv_async_t *async, int);
private:
uv_async_t *notify_async = nullptr;
uv_async_t *destruct_async = nullptr;

View File

@@ -55,6 +55,9 @@
'ldflags': [ '<@(ldflags)' ],
}]
],
'include_dirs': [
'../src'
],
'dependencies': [
'../mapboxgl.gyp:bundle_styles',
'../mapboxgl.gyp:mbgl-core',

View File

@@ -11,10 +11,6 @@
#include <map>
#include <cassert>
dispatch_once_t request_initialize = 0;
NSURLSession *session = nullptr;
NSString *userAgent = nil;
namespace mbgl {
enum class ResponseStatus : uint8_t {
@@ -84,12 +80,39 @@ private:
class HTTPCocoaContext : public HTTPContext<HTTPCocoaContext> {
public:
HTTPCocoaContext(uv_loop_t *loop);
~HTTPCocoaContext();
NSURLSession *session = nil;
NSString *userAgent = nil;
};
template<> pthread_key_t HTTPContext<HTTPCocoaContext>::key{};
template<> pthread_once_t HTTPContext<HTTPCocoaContext>::once = PTHREAD_ONCE_INIT;
HTTPCocoaContext::HTTPCocoaContext(uv_loop_t *loop_) : HTTPContext(loop_) {}
HTTPCocoaContext::HTTPCocoaContext(uv_loop_t *loop_) : HTTPContext(loop_) {
@autoreleasepool {
NSURLSessionConfiguration *sessionConfig =
[NSURLSessionConfiguration defaultSessionConfiguration];
sessionConfig.timeoutIntervalForResource = 30;
sessionConfig.HTTPMaximumConnectionsPerHost = 8;
sessionConfig.requestCachePolicy = NSURLRequestReloadIgnoringLocalCacheData;
sessionConfig.URLCache = nil;
session = [NSURLSession sessionWithConfiguration:sessionConfig];
[session retain];
// Write user agent string
userAgent = @"MapboxGL";
}
}
HTTPCocoaContext::~HTTPCocoaContext() {
[session release];
session = nullptr;
[userAgent release];
userAgent = nullptr;
}
// -------------------------------------------------------------------------------------------------
@@ -128,18 +151,22 @@ void HTTPRequestImpl::start() {
}
}
[req addValue:userAgent forHTTPHeaderField:@"User-Agent"];
[req addValue:context->userAgent forHTTPHeaderField:@"User-Agent"];
task = [session dataTaskWithRequest:req
task = [context->session dataTaskWithRequest:req
completionHandler:^(NSData *data, NSURLResponse *res,
NSError *error) { handleResult(data, res, error); }];
[req release];
[task retain];
[task resume];
}
}
void HTTPRequestImpl::handleResponse() {
task = nullptr;
if (task) {
[task release];
task = nullptr;
}
if (request) {
if (status == ResponseStatus::TemporaryError && attempts < maxAttempts) {
@@ -171,8 +198,11 @@ void HTTPRequestImpl::cancel() {
context->removeRequest(request);
request = nullptr;
[task cancel];
task = nullptr;
if (task) {
[task cancel];
[task release];
task = nullptr;
}
}
HTTPRequestImpl::~HTTPRequestImpl() {
@@ -340,20 +370,6 @@ void HTTPRequestImpl::restart(uv_timer_t *timer, int) {
HTTPRequest::HTTPRequest(DefaultFileSource *source, const Resource &resource)
: SharedRequestBase(source, resource) {
// Global initialization.
dispatch_once(&request_initialize, ^{
NSURLSessionConfiguration *sessionConfig =
[NSURLSessionConfiguration defaultSessionConfiguration];
sessionConfig.timeoutIntervalForResource = 30;
sessionConfig.HTTPMaximumConnectionsPerHost = 8;
sessionConfig.requestCachePolicy = NSURLRequestReloadIgnoringLocalCacheData;
sessionConfig.URLCache = nil;
session = [NSURLSession sessionWithConfiguration:sessionConfig];
// Write user agent string
userAgent = @"MapboxGL";
});
}
HTTPRequest::~HTTPRequest() {

View File

@@ -7,14 +7,13 @@
namespace mbgl {
void NSLogBackend::record(EventSeverity severity, Event event, const std::string &msg) {
NSLog(@"[%s] %s: %@", EventSeverityClass(severity).c_str(), EventClass(event).c_str(),
[[NSString alloc] initWithBytes:msg.data()
length:msg.size()
encoding:NSUTF8StringEncoding]);
NSString *message =
[[NSString alloc] initWithBytes:msg.data() length:msg.size() encoding:NSUTF8StringEncoding];
NSLog(@"[%s] %s: %@", EventSeverityClass(severity).c_str(), EventClass(event).c_str(), message);
[message release];
}
void NSLogBackend::record(EventSeverity severity, Event event, const char* format, ...) {
void NSLogBackend::record(EventSeverity severity, Event event, const char *format, ...) {
va_list args;
va_start(args, format);
const size_t len = vsnprintf(NULL, 0, format, args);
@@ -23,17 +22,22 @@ void NSLogBackend::record(EventSeverity severity, Event event, const char* forma
va_start(args, format);
vsnprintf(buffer.get(), len + 1, format, args);
va_end(args);
NSLog(@"[%s] %s: %s", EventSeverityClass(severity).c_str(), EventClass(event).c_str(), buffer.get());
NSLog(@"[%s] %s: %s", EventSeverityClass(severity).c_str(), EventClass(event).c_str(),
buffer.get());
}
void NSLogBackend::record(EventSeverity severity, Event event, int64_t code) {
NSLog(@"[%s] %s: (%lld)", EventSeverityClass(severity).c_str(), EventClass(event).c_str(), code);
NSLog(@"[%s] %s: (%lld)", EventSeverityClass(severity).c_str(), EventClass(event).c_str(),
code);
}
void NSLogBackend::record(EventSeverity severity, Event event, int64_t code, const std::string &msg) {
NSLog(@"[%s] %s: (%lld) %@", EventSeverityClass(severity).c_str(), EventClass(event).c_str(), code,
[[NSString alloc] initWithBytes:msg.data()
length:msg.size()
encoding:NSUTF8StringEncoding]);
void NSLogBackend::record(EventSeverity severity, Event event, int64_t code,
const std::string &msg) {
NSString *message =
[[NSString alloc] initWithBytes:msg.data() length:msg.size() encoding:NSUTF8StringEncoding];
NSLog(@"[%s] %s: (%lld) %@", EventSeverityClass(severity).c_str(), EventClass(event).c_str(),
code, message);
[message release];
}
}

View File

@@ -6,20 +6,26 @@ namespace mbgl {
namespace platform {
std::string uppercase(const std::string &string) {
NSString *nsstring = [[NSString alloc] initWithBytesNoCopy:const_cast<char *>(string.data()) length:string.size() encoding:NSUTF8StringEncoding freeWhenDone:NO];
NSString *nsstring2 = [nsstring uppercaseString];
[nsstring release];
const std::string result { [nsstring2 cStringUsingEncoding:NSUTF8StringEncoding], [nsstring2 lengthOfBytesUsingEncoding:NSUTF8StringEncoding] };
[nsstring2 release];
NSString *original = [[NSString alloc] initWithBytesNoCopy:const_cast<char *>(string.data())
length:string.size()
encoding:NSUTF8StringEncoding
freeWhenDone:NO];
NSString *uppercase = [original uppercaseString];
const std::string result{[uppercase cStringUsingEncoding : NSUTF8StringEncoding],
[uppercase lengthOfBytesUsingEncoding:NSUTF8StringEncoding]};
[original release];
return result;
}
std::string lowercase(const std::string &string) {
NSString *nsstring = [[NSString alloc] initWithBytesNoCopy:const_cast<char *>(string.data()) length:string.size() encoding:NSUTF8StringEncoding freeWhenDone:NO];
NSString *nsstring2 = [nsstring lowercaseString];
[nsstring release];
const std::string result { [nsstring2 cStringUsingEncoding:NSUTF8StringEncoding], [nsstring2 lengthOfBytesUsingEncoding:NSUTF8StringEncoding] };
[nsstring2 release];
NSString *original = [[NSString alloc] initWithBytesNoCopy:const_cast<char *>(string.data())
length:string.size()
encoding:NSUTF8StringEncoding
freeWhenDone:NO];
NSString *lowercase = [original lowercaseString];
const std::string result{[lowercase cStringUsingEncoding : NSUTF8StringEncoding],
[lowercase lengthOfBytesUsingEncoding:NSUTF8StringEncoding]};
[original release];
return result;
}

View File

@@ -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();

View File

@@ -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());

View File

@@ -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

View File

@@ -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) {} };

View File

@@ -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;
}
}
}

View File

@@ -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;

View File

@@ -11,7 +11,7 @@
#include <mbgl/platform/default/headless_view.hpp>
#include <mbgl/platform/default/headless_display.hpp>
#include <mbgl/storage/caching_http_file_source.hpp>
#include <mbgl/storage/default/default_file_source.hpp>
#include "./fixtures/fixture_log.hpp"
@@ -191,7 +191,7 @@ TEST_P(HeadlessTest, render) {
}
HeadlessView view(env->display);
CachingHTTPFileSource fileSource(platform::defaultCacheDatabase());
mbgl::DefaultFileSource fileSource(nullptr);
Map map(view, fileSource);
map.setClasses(classes);

View File

@@ -8,6 +8,7 @@
'ldflags': [
'<@(uv_ldflags)',
'<@(sqlite3_static_libs)',
'<@(uv_static_libs)',
'<@(sqlite3_ldflags)',
'<@(curl_ldflags)',
'<@(png_ldflags)',
@@ -210,7 +211,7 @@
],
'dependencies': [
'../deps/gtest/gtest.gyp:gtest',
'../mapboxgl.gyp:mbgl-standalone',
'../mapboxgl.gyp:mbgl-core',
'../mapboxgl.gyp:mbgl-headless',
'<(platform_library)',
],