FGFS-4.1/simgear
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

HTTPRepo: Fix ownership of HTTPDirectory

Browse Source
This commit is contained in:
James Turner 2020-08-22 21:27:38 +01:00
parent 3298260f11
commit 8b14e56f88
1 changed files with 13 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

37
simgear/io/HTTPRepository.cxx
View File

@ -49,6 +49,7 @@ namespace simgear
{ {
class HTTPDirectory; class HTTPDirectory;
using HTTPDirectory_ptr = std::unique_ptr<HTTPDirectory>;
class HTTPRepoGetRequest : public HTTP::Request class HTTPRepoGetRequest : public HTTP::Request
{ {
@ -137,7 +138,7 @@ public:
SGPath basePath; SGPath basePath;
bool isUpdating; bool isUpdating;
HTTPRepository::ResultCode status; HTTPRepository::ResultCode status;
HTTPDirectory* rootDir; HTTPDirectory_ptr rootDir;
size_t totalDownloaded; size_t totalDownloaded;
HTTP::Request_ptr updateFile(HTTPDirectory* dir, const std::string& name, HTTP::Request_ptr updateFile(HTTPDirectory* dir, const std::string& name,
@ -165,7 +166,7 @@ public:
HTTPDirectory* getOrCreateDirectory(const std::string& path); HTTPDirectory* getOrCreateDirectory(const std::string& path);
bool deleteDirectory(const std::string& relPath, const SGPath& absPath); bool deleteDirectory(const std::string& relPath, const SGPath& absPath);
typedef std::vector<HTTPDirectory*> DirectoryVector; typedef std::vector<HTTPDirectory_ptr> DirectoryVector;
DirectoryVector directories; DirectoryVector directories;
SGPath installedCopyPath; SGPath installedCopyPath;
@ -597,7 +598,7 @@ private:
void removeChild(SGPath path) void removeChild(SGPath path)
{ {
bool ok; bool ok;
SG_LOG(SG_TERRASYNC, SG_WARN, "Removing:" << path); SG_LOG(SG_TERRASYNC, SG_INFO, "Removing:" << path);
std::string fpath = _relativePath + "/" + path.file(); std::string fpath = _relativePath + "/" + path.file();
if (path.isDir()) { if (path.isDir()) {
@ -631,14 +632,7 @@ HTTPRepository::HTTPRepository(const SGPath& base, HTTP::Client *cl) :
{ {
_d->http = cl; _d->http = cl;
_d->basePath = base; _d->basePath = base;
// REVIEW: Memory Leak - 885,598 (3,072 direct, 882,526 indirect) bytes in 48 blocks are definitely lost _d->rootDir.reset(new HTTPDirectory(_d.get(), ""));
// _d->rootDir should be changed to a shared_ptr
// this object is passed to several chained methods, so destruction within ~HTTPRepoPrivate may be
// an easier fix assuming sole ownership would apply
// REVIEW: _d is only partially initialized when it is being passed to the HTTPDirectory ctor.
// The HTTPDirectory object is taking ownership of the shared HTTPRepoPrivate object.
// Mismatch between _d being a unique_ptr, yet also cloned into HTTPDirectory.
_d->rootDir = new HTTPDirectory(_d.get(), "");
_d->parseHashCache(); _d->parseHashCache();
} }
@ -675,7 +669,7 @@ void HTTPRepository::update()
_d->status = REPO_NO_ERROR; _d->status = REPO_NO_ERROR;
_d->isUpdating = true; _d->isUpdating = true;
_d->failures.clear(); _d->failures.clear();
_d->updateDir(_d->rootDir, std::string(), 0); _d->updateDir(_d->rootDir.get(), {}, 0);
} }
bool HTTPRepository::isDoingSync() const bool HTTPRepository::isDoingSync() const
@ -943,10 +937,7 @@ HTTPRepository::failure() const
http->cancelRequest(*rq, "Repository object deleted"); http->cancelRequest(*rq, "Repository object deleted");
} }
DirectoryVector::iterator it; directories.clear(); // wil delete them all
for (it=directories.begin(); it != directories.end(); ++it) {
delete *it;
}
} }
HTTP::Request_ptr HTTPRepoPrivate::updateFile(HTTPDirectory* dir, const std::string& name, size_t sz) HTTP::Request_ptr HTTPRepoPrivate::updateFile(HTTPDirectory* dir, const std::string& name, size_t sz)
@ -1109,7 +1100,7 @@ HTTPRepository::failure() const
{ {
public: public:
DirectoryWithPath(const std::string& p) : path(p) {} DirectoryWithPath(const std::string& p) : path(p) {}
bool operator()(const HTTPDirectory* entry) const bool operator()(const HTTPDirectory_ptr& entry) const
{ return entry->relativePath() == path; } { return entry->relativePath() == path; }
private: private:
std::string path; std::string path;
@ -1118,14 +1109,13 @@ HTTPRepository::failure() const
HTTPDirectory* HTTPRepoPrivate::getOrCreateDirectory(const std::string& path) HTTPDirectory* HTTPRepoPrivate::getOrCreateDirectory(const std::string& path)
{ {
DirectoryWithPath p(path); DirectoryWithPath p(path);
DirectoryVector::iterator it = std::find_if(directories.begin(), directories.end(), p); auto it = std::find_if(directories.begin(), directories.end(), p);
if (it != directories.end()) { if (it != directories.end()) {
return *it; return it->get();
} }
HTTPDirectory* d = new HTTPDirectory(this, path); directories.emplace_back(new HTTPDirectory(this, path));
directories.push_back(d); return directories.back().get();
return d;
} }
bool HTTPRepoPrivate::deleteDirectory(const std::string& relPath, const SGPath& absPath) bool HTTPRepoPrivate::deleteDirectory(const std::string& relPath, const SGPath& absPath)
@ -1133,10 +1123,9 @@ HTTPRepository::failure() const
DirectoryWithPath p(relPath); DirectoryWithPath p(relPath);
auto it = std::find_if(directories.begin(), directories.end(), p); auto it = std::find_if(directories.begin(), directories.end(), p);
if (it != directories.end()) { if (it != directories.end()) {
HTTPDirectory* d = *it; HTTPDirectory* d = it->get();
assert(d->absolutePath() == absPath); assert(d->absolutePath() == absPath);
directories.erase(it); directories.erase(it);
delete d;
} else { } else {
// we encounter this code path when deleting an orphaned directory // we encounter this code path when deleting an orphaned directory
} }