Issue #11 CEventLoop uses QPointer trick to guard against use-after-free

See https://blogs.kde.org/2009/03/26/how-crash-almost-every-qtkde-application-and-how-fix-it-0
This commit is contained in:
Mat Sutcliffe
2021-09-28 18:49:05 +01:00
parent 02d0f40c6d
commit 2ac222f700
4 changed files with 28 additions and 5 deletions

View File

@@ -182,7 +182,7 @@ namespace BlackCore
// with this little trick we try to make an asynchronous signal / slot based approach // with this little trick we try to make an asynchronous signal / slot based approach
// a synchronous return value // a synchronous return value
CEventLoop eventLoop; CEventLoop eventLoop(this);
eventLoop.stopWhen(m_fsdClient, &CFSDClient::flightPlanReceived, [ = ](const auto &cs, const auto &) { return cs == callsign; }); eventLoop.stopWhen(m_fsdClient, &CFSDClient::flightPlanReceived, [ = ](const auto &cs, const auto &) { return cs == callsign; });
if (eventLoop.exec(1500)) if (eventLoop.exec(1500))
{ {

View File

@@ -426,7 +426,7 @@ namespace BlackCore
CStatusMessageList CApplication::waitForSetup(int timeoutMs) CStatusMessageList CApplication::waitForSetup(int timeoutMs)
{ {
if (!m_setupReader) { return CStatusMessage(this).error(u"No setup reader"); } if (!m_setupReader) { return CStatusMessage(this).error(u"No setup reader"); }
CEventLoop eventLoop; CEventLoop eventLoop(this);
eventLoop.stopWhen(this, &CApplication::setupHandlingCompleted); eventLoop.stopWhen(this, &CApplication::setupHandlingCompleted);
if (!m_setupReader->isSetupAvailable()) if (!m_setupReader->isSetupAvailable())
{ {
@@ -435,6 +435,11 @@ namespace BlackCore
// setup handling completed with success or failure, or we run into time out // setup handling completed with success or failure, or we run into time out
CStatusMessageList msgs; CStatusMessageList msgs;
if (!eventLoop.isGuardAlive())
{
msgs.push_back(CStatusMessage(this).error(u"Setup not available, already shutting down."));
return msgs;
}
bool forced = false; bool forced = false;
if (!m_setupReader->isSetupAvailable()) if (!m_setupReader->isSetupAvailable())
{ {

View File

@@ -152,7 +152,7 @@ namespace BlackCore::Db
{ {
// just give the system some time to relax, consolidation is time consuming // just give the system some time to relax, consolidation is time consuming
if (!this->doWorkCheck()) { return; } if (!this->doWorkCheck()) { return; }
CEventLoop eventLoop; CEventLoop eventLoop(this);
eventLoop.exec(1000); eventLoop.exec(1000);
} }
} }

View File

@@ -15,6 +15,7 @@
#include <QObject> #include <QObject>
#include <QEventLoop> #include <QEventLoop>
#include <QPointer>
#include <QTimer> #include <QTimer>
namespace BlackMisc namespace BlackMisc
@@ -25,6 +26,16 @@ namespace BlackMisc
class CEventLoop class CEventLoop
{ {
public: public:
//! Constructor.
CEventLoop() : m_guard(&m_eventLoop) {}
//! Constructor. Guard object must exist, and will be checked again when the loop quits.
CEventLoop(QObject *guard) : m_guard(guard)
{
Q_ASSERT(guard);
QObject::connect(guard, &QObject::destroyed, &m_eventLoop, [this] { m_eventLoop.exit(TimedOut); });
}
//! Event loop will stop if the given signal is received. //! Event loop will stop if the given signal is received.
template <typename T, typename F> template <typename T, typename F>
void stopWhen(const T *sender, F signal) void stopWhen(const T *sender, F signal)
@@ -43,14 +54,20 @@ namespace BlackMisc
} }
//! Begin processing events until the timeout or stop condition occurs. //! Begin processing events until the timeout or stop condition occurs.
//! \return True if the signal was received, false if it timed out. //! \return True if the signal was received, false if it timed out or the guard object died.
bool exec(int timeoutMs) bool exec(int timeoutMs)
{ {
if (timeoutMs >= 0) if (timeoutMs >= 0)
{ {
QTimer::singleShot(timeoutMs, &m_eventLoop, [this] { m_eventLoop.exit(TimedOut); }); QTimer::singleShot(timeoutMs, &m_eventLoop, [this] { m_eventLoop.exit(TimedOut); });
} }
return m_eventLoop.exec() == GotSignal; return m_eventLoop.exec() == GotSignal && isGuardAlive();
}
//! True if the guard object still exists.
bool isGuardAlive() const
{
return m_guard;
} }
private: private:
@@ -60,6 +77,7 @@ namespace BlackMisc
TimedOut, TimedOut,
}; };
QEventLoop m_eventLoop; QEventLoop m_eventLoop;
QPointer<QObject> m_guard;
}; };
} // ns } // ns