From 2edd677d1a4f1fa68e42024c200848eb91009ae4 Mon Sep 17 00:00:00 2001 From: Alberto Griggio Date: Wed, 7 Jun 2017 10:36:28 +0200 Subject: [PATCH] Fixed various memory leaks as reported by AddressSanitizer --- rtengine/camconst.cc | 11 ++++++++--- rtengine/camconst.h | 2 +- rtengine/dcp.cc | 9 +++++++++ rtengine/dcp.h | 1 + rtengine/iccstore.cc | 14 +++++++++++++ rtengine/lcp.cc | 31 +++++++++++++++++++++++++---- rtengine/lcp.h | 2 ++ rtgui/filebrowser.cc | 47 +++++++++++--------------------------------- rtgui/filebrowser.h | 10 +++------- rtgui/filecatalog.cc | 1 + 10 files changed, 78 insertions(+), 50 deletions(-) diff --git a/rtengine/camconst.cc b/rtengine/camconst.cc index 2449a1cd1..0609b3699 100644 --- a/rtengine/camconst.cc +++ b/rtengine/camconst.cc @@ -27,9 +27,6 @@ CameraConst::CameraConst() white_max = 0; } -CameraConst::~CameraConst() -{ -} bool CameraConst::parseApertureScaling(CameraConst *cc, void *ji_) @@ -709,6 +706,14 @@ CameraConstantsStore::CameraConstantsStore() { } + +CameraConstantsStore::~CameraConstantsStore() +{ + for (auto &p : mCameraConstants) { + delete p.second; + } +} + void CameraConstantsStore::init(Glib::ustring baseDir, Glib::ustring userSettingsDir) { parse_camera_constants_file(Glib::build_filename(baseDir, "camconst.json")); diff --git a/rtengine/camconst.h b/rtengine/camconst.h index 713936bf8..31b65f3e8 100644 --- a/rtengine/camconst.h +++ b/rtengine/camconst.h @@ -26,7 +26,6 @@ private: std::map mApertureScaling; CameraConst(); - ~CameraConst(); static bool parseLevels(CameraConst *cc, int bw, void *ji); static bool parseApertureScaling(CameraConst *cc, void *ji); bool get_Levels(struct camera_const_levels & lvl, int bw, int iso, float fnumber); @@ -55,6 +54,7 @@ private: bool parse_camera_constants_file(Glib::ustring filename); public: + ~CameraConstantsStore(); void init(Glib::ustring baseDir, Glib::ustring userSettingsDir); static CameraConstantsStore *getInstance(void); CameraConst *get(const char make[], const char model[]); diff --git a/rtengine/dcp.cc b/rtengine/dcp.cc index ad0621ce0..62765f812 100644 --- a/rtengine/dcp.cc +++ b/rtengine/dcp.cc @@ -1701,6 +1701,15 @@ DCPStore* DCPStore::getInstance() return &instance; } + +DCPStore::~DCPStore() +{ + for (auto &p : profile_cache) { + delete p.second; + } +} + + void DCPStore::init(const Glib::ustring& rt_profile_dir, bool loadAll) { MyMutex::MyLock lock(mutex); diff --git a/rtengine/dcp.h b/rtengine/dcp.h index 5a6858099..10bbf8f7e 100644 --- a/rtengine/dcp.h +++ b/rtengine/dcp.h @@ -152,6 +152,7 @@ class DCPStore final : public NonCopyable { public: + ~DCPStore(); static DCPStore* getInstance(); void init(const Glib::ustring& rt_profile_dir, bool loadAll = true); diff --git a/rtengine/iccstore.cc b/rtengine/iccstore.cc index 3bcc8515e..9dbeba00e 100644 --- a/rtengine/iccstore.cc +++ b/rtengine/iccstore.cc @@ -276,6 +276,20 @@ public: } } + ~Implementation() + { + for (auto &p : wProfiles) { + if (p.second) { + cmsCloseProfile(p.second); + } + } + for (auto &p : wProfilesGamma) { + if (p.second) { + cmsCloseProfile(p.second); + } + } + } + void init(const Glib::ustring& usrICCDir, const Glib::ustring& rtICCDir, bool loadAll) { // Reads all profiles from the given profiles dir diff --git a/rtengine/lcp.cc b/rtengine/lcp.cc index 821194afe..1c5f12fb4 100644 --- a/rtengine/lcp.cc +++ b/rtengine/lcp.cc @@ -361,6 +361,11 @@ SSEFUNCTION void LCPMapper::processVignetteLine3Channels(int width, int y, float LCPProfile::LCPProfile(const Glib::ustring &fname) { + for (int i = 0; i < MaxPersModelCount; i++) { + aPersModel[i] = nullptr; + } + pCurPersModel = nullptr; + const int BufferSize = 8192; char buf[BufferSize]; @@ -378,10 +383,6 @@ LCPProfile::LCPProfile(const Glib::ustring &fname) isFisheye = inCamProfiles = firstLIDone = inPerspect = inAlternateLensID = inAlternateLensNames = false; sensorFormatFactor = 1; - for (int i = 0; i < MaxPersModelCount; i++) { - aPersModel[i] = nullptr; - } - persModelCount = 0; *inInvalidTag = 0; @@ -412,6 +413,19 @@ LCPProfile::LCPProfile(const Glib::ustring &fname) filterBadFrames(1.5, 100); } + +LCPProfile::~LCPProfile() +{ + if (pCurPersModel) { + delete pCurPersModel; + } + for (int i = 0; i < MaxPersModelCount; i++) { + if (aPersModel[i]) { + delete aPersModel[i]; + } + } +} + // from all frames not marked as bad already, take average and filter out frames with higher deviation than this if there are enough values int LCPProfile::filterBadFrames(double maxAvgDevFac, int minFramesLeft) { @@ -886,6 +900,15 @@ LCPStore* LCPStore::getInstance() return &instance_; } + +LCPStore::~LCPStore() +{ + for (auto &p : profileCache) { + delete p.second; + } +} + + LCPProfile* LCPStore::getProfile (Glib::ustring filename) { if (filename.length() == 0 || !isValidLCPFileName(filename)) { diff --git a/rtengine/lcp.h b/rtengine/lcp.h index 8cfcd5694..f7164117f 100644 --- a/rtengine/lcp.h +++ b/rtengine/lcp.h @@ -110,6 +110,7 @@ public: LCPPersModel* aPersModel[MaxPersModelCount]; // Do NOT use std::list or something, it's buggy in GCC! explicit LCPProfile(const Glib::ustring &fname); + ~LCPProfile(); void calcParams(int mode, float focalLength, float focusDist, float aperture, LCPModelCommon *pCorr1, LCPModelCommon *pCorr2, LCPModelCommon *pCorr3) const; // Interpolates between the persModels frames @@ -124,6 +125,7 @@ class LCPStore std::map profileCache; public: + ~LCPStore(); Glib::ustring getDefaultCommonDirectory() const; bool isValidLCPFileName(Glib::ustring filename) const; LCPProfile* getProfile(Glib::ustring filename); diff --git a/rtgui/filebrowser.cc b/rtgui/filebrowser.cc index dd37a740b..b4238f011 100644 --- a/rtgui/filebrowser.cc +++ b/rtgui/filebrowser.cc @@ -128,11 +128,7 @@ void findOriginalEntries (const std::vector& entries) FileBrowser::FileBrowser () : tbl(nullptr), numFiltered(0) { - - fbih = new FileBrowserIdleHelper; - fbih->fbrowser = this; - fbih->destroyed = false; - fbih->pending = 0; + session_id_ = 0; ProfileStore::getInstance()->addListener(this); @@ -546,37 +542,27 @@ void FileBrowser::doubleClicked (ThumbBrowserEntryBase* entry) void FileBrowser::addEntry (FileBrowserEntry* entry) { struct addparams { - FileBrowserIdleHelper* fbih; - FileBrowserEntry* entry; + FileBrowser *browser; + FileBrowserEntry *entry; + unsigned int session_id; }; - fbih->pending++; - entry->setParent (this); addparams* const ap = new addparams; - ap->fbih = fbih; + entry->setParent (this); + ap->browser = this; ap->entry = entry; + ap->session_id = session_id(); const auto func = [](gpointer data) -> gboolean { addparams* const ap = static_cast(data); - FileBrowserIdleHelper* fbih = ap->fbih; - - if (fbih->destroyed) { - if (fbih->pending == 1) { - delete fbih; - } else { - fbih->pending--; - } - + if (ap->session_id != ap->browser->session_id()) { delete ap->entry; delete ap; - - return 0; + } else { + ap->browser->addEntry_(ap->entry); + delete ap; } - ap->fbih->fbrowser->addEntry_ (ap->entry); - delete ap; - fbih->pending--; - return FALSE; }; @@ -653,16 +639,7 @@ FileBrowserEntry* FileBrowser::delEntry (const Glib::ustring& fname) void FileBrowser::close () { - if (fbih->pending) { - fbih->destroyed = true; - } else { - delete fbih; - } - - fbih = new FileBrowserIdleHelper; - fbih->fbrowser = this; - fbih->destroyed = false; - fbih->pending = 0; + ++session_id_; { MYWRITERLOCK(l, entryRW); diff --git a/rtgui/filebrowser.h b/rtgui/filebrowser.h index 4bad832eb..4efdb7219 100644 --- a/rtgui/filebrowser.h +++ b/rtgui/filebrowser.h @@ -53,12 +53,6 @@ public: } }; -struct FileBrowserIdleHelper { - FileBrowser* fbrowser; - bool destroyed; - int pending; -}; - /* * Class handling actions common to all thumbnails of the file browser */ @@ -71,6 +65,7 @@ private: typedef sigc::signal type_trash_changed; IdleRegister idle_register; + unsigned int session_id_; protected: Gtk::MenuItem* rank[6]; @@ -129,7 +124,6 @@ protected: FileBrowserListener* tbl; BrowserFilter filter; int numFiltered; - FileBrowserIdleHelper* fbih; void toTrashRequested (std::vector tbe); void fromTrashRequested (std::vector tbe); @@ -152,6 +146,8 @@ public: FileBrowserEntry* delEntry (const Glib::ustring& fname); // return the entry if found here return NULL otherwise void close (); + unsigned int session_id() const { return session_id_; } + void setBatchPParamsChangeListener (BatchPParamsChangeListener* l) { bppcl = l; diff --git a/rtgui/filecatalog.cc b/rtgui/filecatalog.cc index 4ac1e0a3d..710450d90 100644 --- a/rtgui/filecatalog.cc +++ b/rtgui/filecatalog.cc @@ -720,6 +720,7 @@ void FileCatalog::previewReady (int dir_id, FileBrowserEntry* fdn) { if ( dir_id != selectedDirectoryId ) { + delete fdn; return; }