From 1803924a9474af03bf24bc00303c6373fdd05487 Mon Sep 17 00:00:00 2001 From: Christian Mollekopf Date: Wed, 6 Jul 2016 17:52:33 +0200 Subject: Fixed a bunch of memory leaks. Found with valgrind --- common/listener.cpp | 5 +++++ common/pipeline.cpp | 8 ++++++-- common/resourceaccess.cpp | 1 + common/resourceaccess.h | 2 +- common/storage.h | 27 ++++----------------------- common/storage_lmdb.cpp | 33 +++++++++++++++++++++++++++++++++ 6 files changed, 50 insertions(+), 26 deletions(-) (limited to 'common') diff --git a/common/listener.cpp b/common/listener.cpp index d2fc510..32c57ac 100644 --- a/common/listener.cpp +++ b/common/listener.cpp @@ -85,6 +85,11 @@ Listener::Listener(const QByteArray &resourceInstanceIdentifier, const QByteArra Listener::~Listener() { + closeAllConnections(); + delete m_resource; + delete m_checkConnectionsTimer; + delete m_clientBufferProcessesTimer; + delete m_server; } void Listener::emergencyAbortAllConnections() diff --git a/common/pipeline.cpp b/common/pipeline.cpp index c6d5297..feceb77 100644 --- a/common/pipeline.cpp +++ b/common/pipeline.cpp @@ -79,6 +79,10 @@ Pipeline::Pipeline(const QString &resourceName, QObject *parent) : QObject(paren Pipeline::~Pipeline() { + d->transaction = Storage::Transaction(); + for (const auto &t : d->processors.keys()) { + qDeleteAll(d->processors.value(t)); + } delete d; } @@ -108,9 +112,9 @@ void Pipeline::startTransaction() Trace() << "Starting transaction."; d->transactionTime.start(); d->transactionItemCount = 0; - d->transaction = std::move(storage().createTransaction(Storage::ReadWrite, [](const Sink::Storage::Error &error) { + d->transaction = storage().createTransaction(Storage::ReadWrite, [](const Sink::Storage::Error &error) { Warning() << error.message; - })); + }); //FIXME this is a temporary measure to recover from a failure to open the named databases correctly. //Once the actual problem is fixed it will be enough to simply crash if we open the wrong database (which we check in openDatabase already). diff --git a/common/resourceaccess.cpp b/common/resourceaccess.cpp index 8297fa5..93f97e8 100644 --- a/common/resourceaccess.cpp +++ b/common/resourceaccess.cpp @@ -643,6 +643,7 @@ Sink::ResourceAccess::Ptr ResourceAccessFactory::getAccess(const QByteArray &ins } if (!mTimer.contains(instanceIdentifier)) { auto timer = new QTimer; + timer->setSingleShot(true); // Drop connection after 3 seconds (which is a random value) QObject::connect(timer, &QTimer::timeout, timer, [this, instanceIdentifier]() { mCache.remove(instanceIdentifier); }); timer->setInterval(3000); diff --git a/common/resourceaccess.h b/common/resourceaccess.h index 5c65998..47b848e 100644 --- a/common/resourceaccess.h +++ b/common/resourceaccess.h @@ -145,7 +145,7 @@ private: * * This avoids constantly recreating connections, and should allow a single process to have one connection per resource. */ -class ResourceAccessFactory +class SINK_EXPORT ResourceAccessFactory { public: static ResourceAccessFactory &instance(); diff --git a/common/storage.h b/common/storage.h index e7b4a3e..4ef20d5 100644 --- a/common/storage.h +++ b/common/storage.h @@ -103,18 +103,8 @@ public: */ bool contains(const QByteArray &uid); - NamedDatabase(NamedDatabase &&other) : d(other.d) - { - d = other.d; - other.d = nullptr; - } - - NamedDatabase &operator=(NamedDatabase &&other) - { - d = other.d; - other.d = nullptr; - return *this; - } + NamedDatabase(NamedDatabase &&other); + NamedDatabase &operator=(NamedDatabase &&other); operator bool() const { @@ -146,17 +136,8 @@ public: NamedDatabase openDatabase(const QByteArray &name = QByteArray("default"), const std::function &errorHandler = std::function(), bool allowDuplicates = false) const; - Transaction(Transaction &&other) : d(other.d) - { - d = other.d; - other.d = nullptr; - } - Transaction &operator=(Transaction &&other) - { - d = other.d; - other.d = nullptr; - return *this; - } + Transaction(Transaction &&other); + Transaction &operator=(Transaction &&other); operator bool() const; diff --git a/common/storage_lmdb.cpp b/common/storage_lmdb.cpp index 3687594..2c0240d 100644 --- a/common/storage_lmdb.cpp +++ b/common/storage_lmdb.cpp @@ -103,6 +103,21 @@ Storage::NamedDatabase::NamedDatabase(NamedDatabase::Private *prv) : d(prv) { } +Storage::NamedDatabase::NamedDatabase(NamedDatabase &&other) : d(nullptr) +{ + *this = std::move(other); +} + +Storage::NamedDatabase &Storage::NamedDatabase::operator=(Storage::NamedDatabase &&other) +{ + if (&other != this) { + delete d; + d = other.d; + other.d = nullptr; + } + return *this; +} + Storage::NamedDatabase::~NamedDatabase() { delete d; @@ -398,6 +413,21 @@ Storage::Transaction::Transaction(Transaction::Private *prv) : d(prv) d->startTransaction(); } +Storage::Transaction::Transaction(Transaction &&other) : d(nullptr) +{ + *this = std::move(other); +} + +Storage::Transaction &Storage::Transaction::operator=(Storage::Transaction &&other) +{ + if (&other != this) { + delete d; + d = other.d; + other.d = nullptr; + } + return *this; +} + Storage::Transaction::~Transaction() { if (d && d->transaction) { @@ -532,6 +562,7 @@ QList Storage::Transaction::getDatabaseNames() const Warning() << "Failed to get a value" << rc; } } + mdb_cursor_close(cursor); } else { Warning() << "Failed to open db" << rc << QByteArray(mdb_strerror(rc)); } @@ -594,6 +625,8 @@ Storage::Private::Private(const QString &s, const QString &n, AccessMode m) : st } else { // FIXME: dynamic resize const size_t dbSize = (size_t)10485760 * (size_t)8000; // 1MB * 8000 + // In order to run valgrind this size must be smaller than half your available RAM + // https://github.com/BVLC/caffe/issues/2404 mdb_env_set_mapsize(env, dbSize); sEnvironments.insert(fullPath, env); } -- cgit v1.2.3