Differential D4484 Diff 22675 plugins/qthelp/qthelpexternalassistant.cpp

Changeset View

Standalone View

View Options

plugins/qthelp/qthelpexternalassistant.cpp

  • This file was added.
1​/* This file is part of KDevelop
2​ Copyright 2017 René J.V. Bertin <rjvbertin@gmail.com>
3​
4​ This library is free software; you can redistribute it and/or
5​ modify it under the terms of the GNU Library General Public
6​ License as published by the Free Software Foundation; either
7​ version 2 of the License, or (at your option) any later version.
8​
9​ This library is distributed in the hope that it will be useful,
10​ but WITHOUT ANY WARRANTY; without even the implied warranty of
11​ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
12​ Library General Public License for more details.
13​
14​ You should have received a copy of the GNU Library General Public License
15​ along with this library; see the file COPYING.LIB. If not, write to
16​ the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
17​ Boston, MA 02110-1301, USA.
18​*/
19​
20​#include "qthelpexternalassistant.h"
21​
22​#include <QString>
23​#include <QUrl>
24​#include <QStandardPaths>
25​#include <QProcess>
26​#include <QApplication>
27​#include <QFileInfo>
28​
29​#include <interfaces/icore.h>
30​
31​#include "debug.h"
32​
33​using namespace KDevelop;
34​
35​QtHelpExternalAssistant* QtHelpExternalAssistant::s_self = nullptr;
36​
37​QtHelpExternalAssistant::QtHelpExternalAssistant(QObject *parent)
38​ : QObject(parent)
39​{
40​}
41​
42​QtHelpExternalAssistant::~QtHelpExternalAssistant()
43​{
44​ if (m_externalViewerProcess) {
45​ // stop monitoring the process. Prevents potential warnings
46​ // (QCoreApplication::postEvent: Unexpected null receiver)
47​ m_externalViewerProcess->disconnect();
48​#ifdef Q_OS_UNIX
49​ qint64 pid = m_externalViewerProcess->processId();
50​ if (pid > 0) {
51​ QProcess* hup = new QProcess();
52​ hup->startDetached(QString::fromLatin1("kill -1 %1").arg(pid));
53​ hup->waitForFinished(50);
54​ hup->deleteLater();
55​ } else
56​#endif
57​ m_externalViewerProcess->terminate();
58​ // give the process a bit of time to react, just to prevent
59​ // "QProcess destroyed while process still running" warnings.
anthonyfieroniUnsubmitted
Not Done

Why not call s_externalViewerProcess->kill ?

anthonyfieroni: Why not call s_externalViewerProcess->kill ?
rjvbbAuthorUnsubmitted
Not Done

I'd stick with QProcess::terminate() if I were to use a one-fits-all approach, but I'm trying to quit the external viewer as gracefully as possible. Traditionally that'd be with a hangup signal (AFAIK that's what the X server still sends to its clients when it is shutting down). The Assistant does have some internal state that's saved under normal operation, so I don't want to kill it off too bluntly.

I've filed a request to add a Quit command to the remote control protocol.

rjvbb: I'd stick with QProcess::terminate() if I were to use a one-fits-all approach, but I'm trying…
60​ m_externalViewerProcess->waitForFinished(50);
61​ m_externalViewerProcess->deleteLater();
62​ m_externalViewerProcess = nullptr;
63​ }
64​ if (this == s_self) {
65​ s_self = nullptr;
66​ }
67​}
68​
69​void QtHelpExternalAssistant::externalViewerExit(int exitCode, QProcess::ExitStatus exitStatus)
70​{
71​ if (this == s_self) {
72​ qCDebug(QTHELP) << "externalViewer" << this << "has exited with code"
73​ << exitCode << "and status" << exitStatus;
74​ deleteLater();
75​ m_externalViewerProcess = nullptr;
76​ }
77​}
78​
79​void QtHelpExternalAssistant::setUseExternalViewer(bool extViewer)
80​{
81​ if (this == s_self) {
82​ m_useExternalViewer = extViewer;
83​ } else {
84​ s_self->setUseExternalViewer(extViewer);
85​ }
86​}
87​
88​void QtHelpExternalAssistant::setExternalViewerExecutable(QString& path)
89​{
90​ if (this == s_self) {
91​ m_externalViewerExecutable = path;
92​ } else {
93​ s_self->setExternalViewerExecutable(path);
94​ }
95​}
96​
97​#ifndef Q_OS_MACOS
98​bool QtHelpExternalAssistant::hasExternalViewer(QString* path)
99​{
100​ bool ret = false;
101​ if (this == s_self) {
102​ if (!m_externalViewerExecutable.isEmpty()) {
103​ QFileInfo info(m_externalViewerExecutable);
104​ if (info.isExecutable()) {
105​ ret = true;
106​ } else {
107​ qCWarning(QTHELP) << "External viewer" << m_externalViewerExecutable << "is not executable!";
108​ }
109​ // always return the configured executable path
110​ if (path) {
111​ *path = m_externalViewerExecutable;
112​ }
113​ } else if (path) {
114​ // look for the application on the current path. The result is not cached so the
115​ // lookup is performed only when we can return the result.
116​ const QString assistant = QStandardPaths::findExecutable(QLatin1String("assistant"));
117​ if (!assistant.isEmpty()) {
118​ ret = true;
119​ *path = assistant;
120​ qCDebug(QTHELP) << "External viewer found at" << assistant;
121​ } else {
122​ qCWarning(QTHELP) << "External viewer not found!";
123​ }
124​ }
125​ } else {
126​ return s_self->hasExternalViewer(path);
127​ }
128​ return ret;
129​}
130​#endif
anthonyfieroniUnsubmitted
Done

Here you should deleted s_instance too or when you use like in openUrl

if (instance()) {

you will hit a crash in dereferencing s_externalViewerProcess

anthonyfieroni: Here you should deleted s_instance too or when you use like in openUrl ``` if (instance()) {…
rjvbbAuthorUnsubmitted
Not Done

Actually, s_instance is being deleted at this very point, but I see what you mean. I missed a few other details due to no longer deriving QProcess but using a QProcess member var instead. Should be fixed now.

Just for the record: this dtor is called very late in the shutdown procedure, in practice it always happens after all projects have been closed. And for me the mainwindow is "long" gone by the time projects are being unloaded. You'd really have to try hard to call openUrl() deliberately after this point.

rjvbb: Actually, `s_instance` is being deleted at this very point, but I see what you mean. I missed a…
131​
132​QtHelpExternalAssistant* QtHelpExternalAssistant::self()
133​{
134​ if (ICore::self()->shuttingDown()) {
135​ // don't create a new instance if we're shutting down.
136​ return s_self;
137​ }
138​
139​ if (!s_self) {
140​ if (!s_self) {
141​ s_self = new QtHelpExternalAssistant(qApp);
142​ }
143​ }
144​ return s_self;
145​}
146​
147​bool QtHelpExternalAssistant::openUrl(const QUrl& url)
148​{
149​ if (ICore::self()->shuttingDown()) {
150​ return false;
151​ }
152​ if (s_self || self()) {
153​ // start the viewer if required (moved here from self())
154​ QString path;
155​ if (!s_self->m_externalViewerProcess && s_self->hasExternalViewer(&path)) {
156​ s_self->m_externalViewerProcess = new QProcess();
157​ connect(s_self->m_externalViewerProcess,
158​ static_cast<void(QProcess::*)(int, QProcess::ExitStatus)>(&QProcess::finished),
159​ s_self, &QtHelpExternalAssistant::externalViewerExit);
160​ QStringList args = {"-enableRemoteControl"};
161​ s_self->m_externalViewerProcess->start(path, args, QIODevice::WriteOnly|QIODevice::Append);
162​ if (!s_self->m_externalViewerProcess->waitForStarted()) {
163​ s_self->m_externalViewerProcess->deleteLater();
164​ s_self->m_externalViewerProcess = nullptr;
165​ }
166​ }
167​ const QByteArray command = QByteArrayLiteral("setSource ")
168​ + url.toString().toUtf8() + QByteArrayLiteral("\n");
169​ if (s_self->m_externalViewerProcess->write(command) < 0
170​ || s_self->m_externalViewerProcess->write("show contents\n") < 0) {
171​ return false;
172​ }
173​ s_self->m_externalViewerProcess->write("syncContents\n");
174​ return true;
175​ }
176​ return false;
177​}