Commit 37ff587e authored by Igor Kushnir's avatar Igor Kushnir

Safely finalize and destroy DebugSession objects

DebugController is responsible for destroying debug sessions and it can
not do so past its own destructor. Currently, depending on the relative
timing of KDevelop exit and the 5 second interval of the singleshot
timer in MIDebugSession::stopDebugger(), a DebugSession may be destroyed
safely in time or not destroyed at all and possibly cause a crash by
accessing the already destroyed DebugController or its children.

Let us kill debugger processes and thus finalize the debug sessions'
states in ~DebugController(). We delay the killing of debugger processes
for as long as possible to give them a chance to exit on their own.

Check ICore::documentController() for nullptr in
BreakpointModel::updateMarks() and
DebugController::clearExecutionPoint() to prevent crashes when these
slots are invoked by DebugSession's state transition signals from
inside ~DebugController(), which is called after ~DocumentController().

BUG: 425991
FIXED-IN: 5.6.1
parent c60e8303
......@@ -477,11 +477,18 @@ void KDevelop::BreakpointModel::updateMarks()
if (d->dontUpdateMarks)
return;
const auto* const documentController = ICore::self()->documentController();
if (!documentController) {
qCDebug(DEBUGGER) << "Cannot update marks without the document controller. "
"KDevelop must be exiting and the document controller already destroyed.";
return;
}
//add marks
for (Breakpoint* breakpoint : qAsConst(d->breakpoints)) {
if (breakpoint->kind() != Breakpoint::CodeBreakpoint) continue;
if (breakpoint->line() == -1) continue;
IDocument *doc = ICore::self()->documentController()->documentForUrl(breakpoint->url());
IDocument *doc = documentController->documentForUrl(breakpoint->url());
if (!doc) continue;
KTextEditor::MarkInterface *mark = qobject_cast<KTextEditor::MarkInterface*>(doc->textDocument());
if (!mark) continue;
......@@ -502,7 +509,7 @@ void KDevelop::BreakpointModel::updateMarks()
}
//remove marks
const auto documents = ICore::self()->documentController()->openDocuments();
const auto documents = documentController->openDocuments();
for (IDocument* doc : documents) {
KTextEditor::MarkInterface *mark = qobject_cast<KTextEditor::MarkInterface*>(doc->textDocument());
if (!mark) continue;
......
......@@ -136,6 +136,8 @@ public:
public Q_SLOTS:
virtual void restartDebugger() = 0;
virtual void stopDebugger() = 0;
/// @brief Kills the debugger process synchronously if it is still running.
virtual void killDebuggerNow() = 0;
virtual void interruptDebugger() = 0;
virtual void run() = 0;
virtual void runToCursor() = 0;
......
......@@ -159,6 +159,17 @@ void DebugController::cleanup()
DebugController::~DebugController()
{
// The longest possible time interval has been allotted for previous and
// the current debug sessions to stop their debugger processes: stopDebugger()
// was called on the sessions in addSession() and cleanup() respectively.
// Now is the last safe moment for a DebugSession to finalize its state and
// invoke DebugController::debuggerStateChanged(), which in turn schedules the
// session's deletion. This finalization not only ensures that debug sessions
// are destroyed, but also prevents crashes: a DebugSession's state transition
// signals lead to accesses to DebugController and its children, such as a
// call to BreakpointModel::updateState(). Therefore we must force all debug
// sessions to synchronously finalize their states now.
emit killAllDebuggersNow();
}
BreakpointModel* DebugController::breakpointModel()
......@@ -300,6 +311,7 @@ void DebugController::addSession(IDebugSession* session)
connect(session, &IDebugSession::showStepInSource, this, &DebugController::showStepInSource);
connect(session, &IDebugSession::clearExecutionPoint, this, &DebugController::clearExecutionPoint);
connect(session, &IDebugSession::raiseFramestackViews, this, &DebugController::raiseFramestackViews);
connect(this, &DebugController::killAllDebuggersNow, session, &IDebugSession::killDebuggerNow);
updateDebuggerState(session->state(), session);
......@@ -319,8 +331,14 @@ void DebugController::addSession(IDebugSession* session)
void DebugController::clearExecutionPoint()
{
qCDebug(SHELL);
const auto documents = KDevelop::ICore::self()->documentController()->openDocuments();
const auto* const documentController = KDevelop::ICore::self()->documentController();
if (!documentController) {
qCDebug(SHELL) << "Cannot clear execution point without the document controller. "
"KDevelop must be exiting and the document controller already destroyed.";
return;
}
const auto documents = documentController->openDocuments();
for (KDevelop::IDocument* document : documents) {
auto* iface = qobject_cast<KTextEditor::MarkInterface*>(document->textDocument());
if (!iface)
......
......@@ -89,6 +89,7 @@ private Q_SLOTS:
Q_SIGNALS:
void raiseFramestackViews();
void killAllDebuggersNow();
private:
void setupActions();
......
......@@ -105,6 +105,11 @@ void TestDebugSession::stopDebugger()
m_sessionState = StoppedState;
}
void TestDebugSession::killDebuggerNow()
{
m_sessionState = StoppedState;
}
void TestDebugSession::interruptDebugger()
{
m_sessionState = StoppedState;
......
......@@ -90,6 +90,7 @@ public:
public Q_SLOTS:
void restartDebugger() override;
void stopDebugger() override;
void killDebuggerNow() override;
void interruptDebugger() override;
void run() override;
void runToCursor() override;
......
......@@ -564,15 +564,29 @@ void MIDebugSession::stopDebugger()
QTimer::singleShot(5000, this, [this]() {
if (!debuggerStateIsOn(s_programExited) && debuggerStateIsOn(s_shuttingDown)) {
qCDebug(DEBUGGERCOMMON) << "debugger not shutdown - killing";
m_debugger->kill();
setDebuggerState(s_dbgNotStarted | s_appNotStarted);
raiseEvent(debugger_exited);
killDebuggerImpl();
}
});
emit reset();
}
void MIDebugSession::killDebuggerNow()
{
if (!debuggerStateIsOn(s_dbgNotStarted)) {
qCDebug(DEBUGGERCOMMON) << "killing debugger now";
killDebuggerImpl();
}
}
void MIDebugSession::killDebuggerImpl()
{
Q_ASSERT(m_debugger);
m_debugger->kill();
setDebuggerState(s_dbgNotStarted | s_appNotStarted);
raiseEvent(debugger_exited);
}
void MIDebugSession::interruptDebugger()
{
Q_ASSERT(m_debugger);
......
......@@ -139,6 +139,7 @@ public:
public Q_SLOTS:
void restartDebugger() override;
void stopDebugger() override;
void killDebuggerNow() override;
void interruptDebugger() override;
void run() override;
void runToCursor() override;
......@@ -360,6 +361,9 @@ protected:
QMap<QString, MIVariable*> m_allVariables;
QPointer<MIDebuggerPlugin> m_plugin;
private:
void killDebuggerImpl();
};
template<class Handler>
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment