Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes for memory leaks and QObject* ownership bugs #201

Open
wants to merge 62 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
62 commits
Select commit Hold shift + click to select a range
5c3d2f7
Fix for QObject* ownership bugs
412b May 20, 2015
7591602
added missed parents
412b May 24, 2015
ad91813
Merge remote-tracking branch 'upstream/master' into qt-qml-fixing-own…
412b May 24, 2015
f3b8e77
not that easy there, leaving a FIXME for the next round
412b May 24, 2015
d04434e
added missing destructors
412b May 28, 2015
43dd963
fixing leaks
412b May 28, 2015
2b46f13
ownership, concurrency and and destructor for parser backend manager and
412b May 28, 2015
7b97514
clearJourney() is a more proper name
412b May 31, 2015
3b16f28
adding clearJourney to EFA parser
412b May 31, 2015
32243d8
fixing ParserXmlVasttrafikSe memory leaks
412b May 31, 2015
cf3ad03
one more small fix to vasttrafikse
412b May 31, 2015
486eb79
fixing ninetwo memory leaks
412b May 31, 2015
52a8675
fixing resrobot memory leaks
412b May 31, 2015
8741cfa
providing clearJourney at the top level
412b May 31, 2015
12260d3
Fix for QObject* ownership bugs
412b May 20, 2015
c01f35f
added missed parents
412b May 24, 2015
90dc51c
not that easy there, leaving a FIXME for the next round
412b May 24, 2015
491f62f
added missing destructors
412b May 28, 2015
1c97540
fixing leaks
412b May 28, 2015
ca2363e
ownership, concurrency and and destructor for parser backend manager and
412b May 28, 2015
6ab36b3
clearJourney() is a more proper name
412b May 31, 2015
f564acb
adding clearJourney to EFA parser
412b May 31, 2015
4e9d150
fixing ParserXmlVasttrafikSe memory leaks
412b May 31, 2015
88e23bd
one more small fix to vasttrafikse
412b May 31, 2015
7f7411d
fixing ninetwo memory leaks
412b May 31, 2015
ae0c3f5
fixing resrobot memory leaks
412b May 31, 2015
9578c88
providing clearJourney at the top level
412b May 31, 2015
17bc132
added missed parents
412b May 24, 2015
a8d240c
not that easy there, leaving a FIXME for the next round
412b May 24, 2015
253877b
fixing leaks
412b May 28, 2015
bb6c564
clearJourney() is a more proper name
412b May 31, 2015
c766afa
fixing ParserXmlVasttrafikSe memory leaks
412b May 31, 2015
d2be213
one more small fix to vasttrafikse
412b May 31, 2015
26a0b54
Merge branch 'qt-qml-fixing-ownership-rebased' of github.com:412b/
412b Jun 1, 2015
adff4e2
Fix for QObject* ownership bugs
412b May 20, 2015
b3a2b76
added missed parents
412b May 24, 2015
98201ac
not that easy there, leaving a FIXME for the next round
412b May 24, 2015
35db43b
added missing destructors
412b May 28, 2015
bfdeedc
fixing leaks
412b May 28, 2015
02eff80
ownership, concurrency and and destructor for parser backend manager and
412b May 28, 2015
95604c5
clearJourney() is a more proper name
412b May 31, 2015
2cea328
adding clearJourney to EFA parser
412b May 31, 2015
bc82fa0
fixing ParserXmlVasttrafikSe memory leaks
412b May 31, 2015
9076fb0
one more small fix to vasttrafikse
412b May 31, 2015
3a00949
fixing ninetwo memory leaks
412b May 31, 2015
420764f
fixing resrobot memory leaks
412b May 31, 2015
fe93c97
providing clearJourney at the top level
412b May 31, 2015
57a638d
added missed parents
412b May 24, 2015
42c88a8
not that easy there, leaving a FIXME for the next round
412b May 24, 2015
a43589c
fixing leaks
412b May 28, 2015
92be2e1
clearJourney() is a more proper name
412b May 31, 2015
ea6fcbb
fixing ParserXmlVasttrafikSe memory leaks
412b May 31, 2015
4897391
one more small fix to vasttrafikse
412b May 31, 2015
6930cf9
upstreaming
Jun 13, 2015
c301e1d
Merge branch 'qt-qml-fixing-ownership-rebased' of github.com:412b/fah…
412b Jun 13, 2015
1a1eb14
added clear journey api to be able to free memory as soon as possible
412b Jun 13, 2015
208ad3e
additional control on destruction
412b Jun 13, 2015
ab694db
Merge remote-tracking branch 'refs/remotes/upstream/master' into qt-q…
412b Jun 13, 2015
6ab413e
minor ownership fixes and memory leak fixes
412b Jun 19, 2015
4db74e8
cleaning up lastJourneyResultList initialization in ctors
412b Jun 19, 2015
16fc039
finalizing cleanups and fixes
412b Jun 19, 2015
0479942
ensure calling clearJourney() in all the paths (search, search earlier,
412b Jun 23, 2015
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions rpm/harbour-fahrplan2.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,10 @@ PkgConfigBR:
- Qt5Quick
- Qt5Qml
- Qt5Core
- sailfishapp
Requires:
- sailfishsilica-qt5 >= 0.10.9
- qt5-qtdeclarative-import-positioning
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, my guess is, this also auto installs the position lib in emulator.
Only worry is, that this also get catched by harbour store validator :) (as far as i know using gps is still against the harbour rules)

If you have any insights on this, or should we wait and see? :D

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is this change relevant to QObject ownership change at all?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that change got there by mistake mostly, but w/o it you can not run it in emu w/o manually installing packages. it can also affect users, which have no positioning dependant apps installed (if that is possible at all :))
that said, that change can be undone.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Files:
- '%{_datadir}/icons/hicolor/86x86/apps/%{name}.png'
- '%{_datadir}/applications/%{name}.desktop'
Expand Down
50 changes: 48 additions & 2 deletions src/fahrplan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,12 @@ Fahrplan::Fahrplan(QObject *parent)
, m_mode(DepartureMode)
, m_dateTime(QDateTime::currentDateTime())
{
settings = new QSettings(FAHRPLAN_SETTINGS_NAMESPACE, "fahrplan2");
settings = new QSettings(FAHRPLAN_SETTINGS_NAMESPACE, "fahrplan2", this);
setMode(static_cast<Mode>(settings->value("mode", DepartureMode).toInt()));

if (!m_parser_manager) {
int currentBackend = settings->value("currentBackend", 0).toInt();
m_parser_manager = new FahrplanBackendManager(currentBackend);
m_parser_manager = new FahrplanBackendManager(currentBackend, this);
}
connect(m_parser_manager, SIGNAL(parserChanged(const QString &, int)), this, SLOT(onParserChanged(const QString &, int)));

Expand All @@ -73,6 +73,41 @@ Fahrplan::Fahrplan(QObject *parent)
}
}

Fahrplan::~Fahrplan()
{
qDebug() << this;

if (m_trainrestrictions) {
delete m_trainrestrictions;
}

if (m_timetable) {
delete m_timetable;
}

if (m_stationSearchResults) {
disconnect(m_stationSearchResults, SIGNAL(stationSelected(Fahrplan::StationType,Station)),
this, SLOT(setStation(Fahrplan::StationType,Station)));
delete m_stationSearchResults;
}

if (m_favorites) {
disconnect(m_favorites, SIGNAL(stationSelected(Fahrplan::StationType,Station)),
this, SLOT(setStation(Fahrplan::StationType,Station)));
delete m_favorites;
}

if (m_parser_manager) {
unbindParserSignals();
disconnect(m_parser_manager, SIGNAL(parserChanged(const QString &, int)), this, SLOT(onParserChanged(const QString &, int)));
delete m_parser_manager;
}

if (settings) {
delete settings;
}
}

void Fahrplan::bindParserSignals()
{
if (m_parser_manager->getParser()) {
Expand All @@ -84,6 +119,17 @@ void Fahrplan::bindParserSignals()
}
}

void Fahrplan::unbindParserSignals()
{
if (m_parser_manager->getParser()) {
disconnect(m_parser_manager->getParser(), SIGNAL(stationsResult(StationsList)), this, SLOT(onStationSearchResults(StationsList)));
disconnect(m_parser_manager->getParser(), SIGNAL(journeyResult(JourneyResultList*)), this, SIGNAL(parserJourneyResult(JourneyResultList*)));
disconnect(m_parser_manager->getParser(), SIGNAL(errorOccured(QString)), this, SIGNAL(parserErrorOccured(QString)));
disconnect(m_parser_manager->getParser(), SIGNAL(journeyDetailsResult(JourneyDetailResultList*)), this, SIGNAL(parserJourneyDetailsResult(JourneyDetailResultList*)));
disconnect(m_parser_manager->getParser(), SIGNAL(timeTableResult(TimetableEntriesList)), this, SLOT(onTimetableResult(TimetableEntriesList)));
}
}

void Fahrplan::storeSettingsValue(const QString &key, const QString &value)
{
settings->setValue(key, value);
Expand Down
2 changes: 2 additions & 0 deletions src/fahrplan.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ class Fahrplan : public QObject
};

explicit Fahrplan(QObject *parent = 0);
virtual ~Fahrplan();
FahrplanParserThread *parser();
Favorites *favorites() const;
QString parserName() const;
Expand Down Expand Up @@ -141,6 +142,7 @@ class Fahrplan : public QObject
void onStationSearchResults(const StationsList &result);
void onTimetableResult(const TimetableEntriesList &timetableEntries);
void bindParserSignals();
void unbindParserSignals();

private:
static FahrplanBackendManager *m_parser_manager;
Expand Down
10 changes: 9 additions & 1 deletion src/fahrplan_backend_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,14 @@ FahrplanBackendManager::FahrplanBackendManager(int defaultParser, QObject *paren
currentParserIndex = defaultParser;
}

FahrplanBackendManager::~FahrplanBackendManager()
{
if (m_parser) {
// Parser object will be autodeleted after the thread quits.
m_parser->quit();
}
}

QStringList FahrplanBackendManager::getParserList()
{
QStringList result;
Expand Down Expand Up @@ -68,7 +76,7 @@ void FahrplanBackendManager::setParser(int index)
m_parser->quit();
}

m_parser = new FahrplanParserThread();
m_parser = new FahrplanParserThread(this);
m_parser->init(index);

emit parserChanged(m_parser->name(), currentParserIndex);
Expand Down
1 change: 1 addition & 0 deletions src/fahrplan_backend_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ class FahrplanBackendManager : public QObject

public:
explicit FahrplanBackendManager(int defaultParser, QObject *parent = 0);
virtual ~FahrplanBackendManager();
QStringList getParserList();
void setParser(int index);
FahrplanParserThread *getParser();
Expand Down
6 changes: 6 additions & 0 deletions src/fahrplan_parser_thread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,11 @@ void FahrplanParserThread::cancelRequest()
emit requestCancelRequest();
}

void FahrplanParserThread::clearJourney()
{
emit requestClearJourney();
}

QString FahrplanParserThread::name() {
return m_name;
}
Expand Down Expand Up @@ -182,6 +187,7 @@ void FahrplanParserThread::run()
connect(this, SIGNAL(requestSearchJourney(Station,Station,Station,QDateTime,ParserAbstract::Mode,int)), m_parser, SLOT(searchJourney(Station,Station,Station,QDateTime,ParserAbstract::Mode,int)), Qt::QueuedConnection);
connect(this, SIGNAL(requestSearchJourneyEarlier()), m_parser, SLOT(searchJourneyEarlier()), Qt::QueuedConnection);
connect(this, SIGNAL(requestSearchJourneyLater()), m_parser, SLOT(searchJourneyLater()), Qt::QueuedConnection);
connect(this, SIGNAL(requestClearJourney()), m_parser, SLOT(clearJourney()), Qt::QueuedConnection);

//Connect parser responses with threads corresponding results
connect(m_parser, SIGNAL(errorOccured(QString)), this, SIGNAL(errorOccured(QString)), Qt::QueuedConnection);
Expand Down
4 changes: 3 additions & 1 deletion src/fahrplan_parser_thread.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ class FahrplanParserThread : public QThread
void requestSearchJourneyEarlier();
void requestGetJourneyDetails(const QString &id);
void requestCancelRequest();
void requestClearJourney();

//Real ones
void stationsResult(const StationsList &result);
Expand All @@ -75,6 +76,7 @@ public slots:
void searchJourneyEarlier();
void getJourneyDetails(const QString &id);
void cancelRequest();
void clearJourney();

bool supportsGps();
bool supportsVia();
Expand All @@ -89,7 +91,7 @@ public slots:
void run();

private:
bool m_ready;
volatile bool m_ready;
int i_parser;

QStringList m_trainrestrictions;
Expand Down
10 changes: 8 additions & 2 deletions src/parser/parser_abstract.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,14 @@
#endif

ParserAbstract::ParserAbstract(QObject *parent) :
QObject(parent)
QObject(parent), lastRequest(NULL)
{
NetworkManager = new QNetworkAccessManager(this);
connect(NetworkManager, SIGNAL(finished(QNetworkReply*)), this, SLOT(networkReplyFinished(QNetworkReply*)));

currentRequestState = FahrplanNS::noneRequest;

requestTimeout = new QTimer();
requestTimeout = new QTimer(this);

connect(requestTimeout, SIGNAL(timeout()), this, SLOT(networkReplyTimedOut()));

Expand All @@ -52,10 +52,16 @@ ParserAbstract::ParserAbstract(QObject *parent) :

ParserAbstract::~ParserAbstract()
{
clearJourney();
delete requestTimeout;
delete NetworkManager;
}

void ParserAbstract::clearJourney()
{

}

void ParserAbstract::networkReplyFinished(QNetworkReply *networkReply)
{
FahrplanNS::curReqStates internalRequestState = currentRequestState;
Expand Down
3 changes: 2 additions & 1 deletion src/parser/parser_abstract.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class ParserAbstract : public QObject
enum Mode { Departure = 0, Arrival = 1 };

explicit ParserAbstract(QObject *parent = 0);
~ParserAbstract();
virtual ~ParserAbstract();

static QString getName() { return "Abstract"; }
virtual QString name() { return getName(); }
Expand All @@ -58,6 +58,7 @@ public slots:
virtual bool supportsTimeTableDirection();
virtual QStringList getTrainRestrictions();
void cancelRequest();
virtual void clearJourney();

signals:
void stationsResult(const StationsList &result);
Expand Down
29 changes: 29 additions & 0 deletions src/parser/parser_definitions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,16 @@ TimetableEntry::TimetableEntry()

//------------- JourneyResultList

JourneyResultList::JourneyResultList(QObject * parent) : QObject(parent)
{

}

JourneyResultList::~JourneyResultList()
{
qDeleteAll(m_items);
}

qreal JourneyResultList::itemcount()
{
return m_items.count();
Expand Down Expand Up @@ -127,6 +137,10 @@ void JourneyResultList::setTimeInfo(const QString &timeInfo)
}

//------------- JourneyResultItem
JourneyResultItem::JourneyResultItem(QObject *parent) : QObject(parent)
{

}

QString JourneyResultItem::id() const
{
Expand Down Expand Up @@ -230,6 +244,16 @@ void JourneyResultItem::setInternalData2(const QString &internalData2)

//------------- JourneyDetailResultList

JourneyDetailResultList::JourneyDetailResultList(QObject * parent) : QObject(parent)
{

}

JourneyDetailResultList::~JourneyDetailResultList()
{
qDeleteAll(m_items);
}

QString JourneyDetailResultList::id() const
{
return m_id;
Expand Down Expand Up @@ -327,6 +351,11 @@ void JourneyDetailResultList::setDuration(const QString &duration)

//------------- JourneyDetailResultItem

JourneyDetailResultItem::JourneyDetailResultItem(QObject *parent) : QObject(parent)
{

}

QString JourneyDetailResultItem::departureStation() const
{
return m_departureStation;
Expand Down
6 changes: 6 additions & 0 deletions src/parser/parser_definitions.h
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ class JourneyResultItem : public QObject
Q_PROPERTY(QString internalData2 READ internalData2 WRITE setInternalData2)

public:
explicit JourneyResultItem(QObject * parent = 0);
QString id() const;
void setId(const QString &);
QDate date() const;
Expand Down Expand Up @@ -140,6 +141,8 @@ class JourneyResultList : public QObject
public slots:
JourneyResultItem *getItem(int);
public:
explicit JourneyResultList(QObject * parent = 0);
virtual ~JourneyResultList();
void appendItem(JourneyResultItem *item);
qreal itemcount();
QString departureStation() const;
Expand Down Expand Up @@ -176,6 +179,7 @@ class JourneyDetailResultItem : public QObject
Q_PROPERTY(QString internalData1 READ internalData1 WRITE setInternalData1)
Q_PROPERTY(QString internalData2 READ internalData2 WRITE setInternalData2)
public:
explicit JourneyDetailResultItem(QObject * parent = 0);
QString departureStation() const;
void setDepartureStation(const QString &);
QString departureInfo() const;
Expand Down Expand Up @@ -228,6 +232,8 @@ class JourneyDetailResultList : public QObject
public slots:
JourneyDetailResultItem *getItem(int);
public:
explicit JourneyDetailResultList(QObject * parent = 0);
virtual ~JourneyDetailResultList();
void appendItem(JourneyDetailResultItem *item);
qreal itemcount();
QString id() const;
Expand Down
Loading