Skip to content

Commit b6dd286

Browse files
authored
Fix #13852 (GUI does not finish analysis properly) (danmar#7633)
Create new `QThread`s for each check, as well as for whole-program analysis, instead of restarting finished threads which is undefined behavior (see https://doc.qt.io/qt-6/qthread.html#create). Fixes Trac #13852.
1 parent 4780cd2 commit b6dd286

File tree

2 files changed

+62
-34
lines changed

2 files changed

+62
-34
lines changed

gui/threadhandler.cpp

Lines changed: 35 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,7 @@
3838

3939
ThreadHandler::ThreadHandler(QObject *parent) :
4040
QObject(parent)
41-
{
42-
setThreadCount(1);
43-
}
41+
{}
4442

4543
ThreadHandler::~ThreadHandler()
4644
{
@@ -54,7 +52,7 @@ void ThreadHandler::clearFiles()
5452
mAnalyseWholeProgram = false;
5553
mCtuInfo.clear();
5654
mAddonsAndTools.clear();
57-
mSuppressions.clear();
55+
mSuppressionsUI.clear();
5856
}
5957

6058
void ThreadHandler::setFiles(const QStringList &files)
@@ -83,6 +81,14 @@ void ThreadHandler::setCheckFiles(const QStringList& files)
8381
}
8482
}
8583

84+
void ThreadHandler::setupCheckThread(CheckThread &thread) const
85+
{
86+
thread.setAddonsAndTools(mCheckAddonsAndTools);
87+
thread.setSuppressions(mSuppressionsUI);
88+
thread.setClangIncludePaths(mClangIncludePaths);
89+
thread.setSettings(mCheckSettings, mCheckSuppressions);
90+
}
91+
8692
void ThreadHandler::check(const Settings &settings, const std::shared_ptr<Suppressions>& supprs)
8793
{
8894
if (mResults.getFileCount() == 0 || mRunningThreadCount > 0 || settings.jobs == 0) {
@@ -91,25 +97,25 @@ void ThreadHandler::check(const Settings &settings, const std::shared_ptr<Suppre
9197
return;
9298
}
9399

94-
setThreadCount(settings.jobs);
100+
mCheckSettings = settings;
101+
mCheckSuppressions = supprs;
102+
103+
createThreads(mCheckSettings.jobs);
95104

96105
mRunningThreadCount = mThreads.size();
97106
mRunningThreadCount = std::min(mResults.getFileCount(), mRunningThreadCount);
98107

99-
QStringList addonsAndTools = mAddonsAndTools;
100-
for (const std::string& addon: settings.addons) {
108+
mCheckAddonsAndTools = mAddonsAndTools;
109+
for (const std::string& addon: mCheckSettings.addons) {
101110
QString s = QString::fromStdString(addon);
102-
if (!addonsAndTools.contains(s))
103-
addonsAndTools << s;
111+
if (!mCheckAddonsAndTools.contains(s))
112+
mCheckAddonsAndTools << s;
104113
}
105114

106115
mCtuInfo.clear();
107116

108117
for (int i = 0; i < mRunningThreadCount; i++) {
109-
mThreads[i]->setAddonsAndTools(addonsAndTools);
110-
mThreads[i]->setSuppressions(mSuppressions);
111-
mThreads[i]->setClangIncludePaths(mClangIncludePaths);
112-
mThreads[i]->setSettings(settings, supprs);
118+
setupCheckThread(*mThreads[i]);
113119
mThreads[i]->start();
114120
}
115121

@@ -123,14 +129,12 @@ void ThreadHandler::check(const Settings &settings, const std::shared_ptr<Suppre
123129

124130
bool ThreadHandler::isChecking() const
125131
{
126-
return mRunningThreadCount > 0;
132+
return mRunningThreadCount > 0 || mAnalyseWholeProgram;
127133
}
128134

129-
void ThreadHandler::setThreadCount(const int count)
135+
void ThreadHandler::createThreads(const int count)
130136
{
131-
if (mRunningThreadCount > 0 ||
132-
count == mThreads.size() ||
133-
count <= 0) {
137+
if (mRunningThreadCount > 0 || count <= 0) {
134138
return;
135139
}
136140

@@ -140,9 +144,9 @@ void ThreadHandler::setThreadCount(const int count)
140144
for (int i = mThreads.size(); i < count; i++) {
141145
mThreads << new CheckThread(mResults);
142146
connect(mThreads.last(), &CheckThread::done,
143-
this, &ThreadHandler::threadDone);
147+
this, &ThreadHandler::threadDone, Qt::QueuedConnection);
144148
connect(mThreads.last(), &CheckThread::fileChecked,
145-
&mResults, &ThreadResult::fileChecked);
149+
&mResults, &ThreadResult::fileChecked, Qt::QueuedConnection);
146150
}
147151
}
148152

@@ -151,7 +155,7 @@ void ThreadHandler::removeThreads()
151155
{
152156
for (CheckThread* thread : mThreads) {
153157
if (thread->isRunning()) {
154-
thread->terminate();
158+
thread->stop();
155159
thread->wait();
156160
}
157161
disconnect(thread, &CheckThread::done,
@@ -162,19 +166,22 @@ void ThreadHandler::removeThreads()
162166
}
163167

164168
mThreads.clear();
165-
mAnalyseWholeProgram = false;
166169
}
167170

168171
void ThreadHandler::threadDone()
169172
{
170-
if (mRunningThreadCount == 1 && mAnalyseWholeProgram) {
173+
mRunningThreadCount--;
174+
175+
if (mRunningThreadCount == 0 && mAnalyseWholeProgram) {
176+
createThreads(1);
177+
mRunningThreadCount = 1;
178+
setupCheckThread(*mThreads[0]);
171179
mThreads[0]->analyseWholeProgram(mLastFiles, mCtuInfo);
172180
mAnalyseWholeProgram = false;
173181
mCtuInfo.clear();
174182
return;
175183
}
176184

177-
mRunningThreadCount--;
178185
if (mRunningThreadCount == 0) {
179186
emit done();
180187

@@ -185,6 +192,9 @@ void ThreadHandler::threadDone()
185192
mLastCheckTime = mCheckStartTime;
186193
mCheckStartTime = QDateTime();
187194
}
195+
196+
mCheckAddonsAndTools.clear();
197+
mCheckSuppressions.reset();
188198
}
189199
}
190200

@@ -215,7 +225,7 @@ void ThreadHandler::initialize(const ResultsView *view)
215225

216226
void ThreadHandler::loadSettings(const QSettings &settings)
217227
{
218-
setThreadCount(settings.value(SETTINGS_CHECK_THREADS, 1).toInt());
228+
createThreads(settings.value(SETTINGS_CHECK_THREADS, 1).toInt());
219229
}
220230

221231
void ThreadHandler::saveSettings(QSettings &settings) const

gui/threadhandler.h

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#ifndef THREADHANDLER_H
2121
#define THREADHANDLER_H
2222

23+
#include "settings.h"
2324
#include "suppressions.h"
2425
#include "threadresult.h"
2526

@@ -37,7 +38,6 @@
3738
class ResultsView;
3839
class CheckThread;
3940
class QSettings;
40-
class Settings;
4141
class ImportProject;
4242
class ErrorItem;
4343

@@ -55,12 +55,6 @@ class ThreadHandler : public QObject {
5555
explicit ThreadHandler(QObject *parent = nullptr);
5656
~ThreadHandler() override;
5757

58-
/**
59-
* @brief Set the number of threads to use
60-
* @param count The number of threads to use
61-
*/
62-
void setThreadCount(int count);
63-
6458
/**
6559
* @brief Initialize the threads (connect all signals to resultsview's slots)
6660
*
@@ -85,7 +79,7 @@ class ThreadHandler : public QObject {
8579
}
8680

8781
void setSuppressions(const QList<SuppressionList::Suppression> &s) {
88-
mSuppressions = s;
82+
mSuppressionsUI = s;
8983
}
9084

9185
void setClangIncludePaths(const QStringList &s) {
@@ -235,12 +229,24 @@ protected slots:
235229
*/
236230
int mScanDuration{};
237231

232+
/**
233+
* @brief Create checker threads
234+
* @param count The number of threads to spawn
235+
*/
236+
void createThreads(int count);
237+
238238
/**
239239
* @brief Function to delete all threads
240240
*
241241
*/
242242
void removeThreads();
243243

244+
/*
245+
* @brief Apply check settings to a checker thread
246+
* @param thread The thread to setup
247+
*/
248+
void setupCheckThread(CheckThread &thread) const;
249+
244250
/**
245251
* @brief Thread results are stored here
246252
*
@@ -259,12 +265,24 @@ protected slots:
259265
*/
260266
int mRunningThreadCount{};
261267

268+
/**
269+
* @brief A whole program check is queued by check()
270+
*/
262271
bool mAnalyseWholeProgram{};
263272
std::string mCtuInfo;
264273

265274
QStringList mAddonsAndTools;
266-
QList<SuppressionList::Suppression> mSuppressions;
275+
QList<SuppressionList::Suppression> mSuppressionsUI;
267276
QStringList mClangIncludePaths;
277+
278+
/// @{
279+
/**
280+
* @brief Settings specific to the current analysis
281+
*/
282+
QStringList mCheckAddonsAndTools;
283+
Settings mCheckSettings;
284+
std::shared_ptr<Suppressions> mCheckSuppressions;
285+
/// @}
268286
private:
269287

270288
/**

0 commit comments

Comments
 (0)