Commit 18937f39 authored by David Edmundson's avatar David Edmundson
Browse files

Move CGroup pid fetching callback to the controller

The CGroup class started out as a dumb data store, that did some
fetching of relevant data.

It then gained a more complex async operation. The lifepsan of the
CGgroup object is managed by the model, so could get deleted whilst the
runnable was running. QRunnables and non-qobjects leds to a lot of
potential problems. There was a complex mutex and a wait condition, yet
it still misses a case only solvable with yet more mutexes.

By moving the callback handling logic to the controller, we can guard
everything in a safer more Qt manner without any overhead and with
simpler code.

There is a behavioural change if you call pids whilst things are
loading, but given a signal is emitted when pids load that's fine.

This class is exported, but the header was never installed.
Whilst technically it is an ABI break it pragmantically will have no
impact whatsoever.

BUG: 430615
parent e9cd816f
......@@ -43,7 +43,6 @@ public:
const QString processGroupId;
const KService::Ptr service;
QVector<pid_t> pids;
std::mutex pidsLock;
static KService::Ptr serviceFromAppId(const QString &appId);
......@@ -51,15 +50,13 @@ public:
static QString unescapeName(const QString &cgroupId);
class ReadPidsRunnable;
ReadPidsRunnable *readPids = nullptr;
};
class CGroupPrivate::ReadPidsRunnable : public QRunnable
{
public:
ReadPidsRunnable(CGroupPrivate *cgroup, QPointer<QObject> context, const QString &path, std::function<void()> callback)
: m_cgroupPrivate(cgroup)
, m_context(context)
ReadPidsRunnable(QObject *context, const QString &path, std::function<void(QVector<pid_t>)> callback)
: m_context(context)
, m_path(path)
, m_callback(callback)
{
......@@ -77,34 +74,16 @@ public:
pids.append(line.toLong());
line = stream.readLine();
}
m_cgroupPrivate->pids = pids;
// Ensure we call the callback on the thread the context object lives on.
if (m_context) {
QMetaObject::invokeMethod(m_context, m_callback);
QMetaObject::invokeMethod(m_context, std::bind(m_callback, pids));
}
std::lock_guard<std::mutex> lock{m_lock};
m_finished = true;
m_cgroupPrivate->readPids = nullptr;
m_condition.notify_all();
}
void wait()
{
std::unique_lock<std::mutex> lock{m_lock};
m_condition.wait(lock, [this]() { return m_finished; });
}
private:
CGroupPrivate *m_cgroupPrivate;
QPointer<QObject> m_context;
QString m_path;
std::function<void()> m_callback;
std::mutex m_lock;
std::condition_variable m_condition;
bool m_finished = false;
std::function<void(QVector<pid_t>)> m_callback;
};
class CGroupSystemInformation
......@@ -131,9 +110,6 @@ CGroup::CGroup(const QString &id)
CGroup::~CGroup()
{
if (d->readPids) {
d->readPids->wait();
}
}
QString KSysGuard::CGroup::id() const
......@@ -148,21 +124,19 @@ KService::Ptr KSysGuard::CGroup::service() const
QVector<pid_t> CGroup::pids() const
{
if (d->readPids) {
d->readPids->wait();
}
return d->pids;
}
void CGroup::requestPids(QPointer<QObject> context, std::function<void()> callback)
void CGroup::setPids(const QVector<pid_t> &pids)
{
if (d->readPids) {
d->readPids->wait();
}
d->pids = pids;
}
void CGroup::requestPids(QObject *context, std::function<void(QVector<pid_t>)> callback)
{
QString path = cgroupSysBasePath() + d->processGroupId + QLatin1String("/cgroup.procs");
d->readPids = new CGroupPrivate::ReadPidsRunnable(d.get(), context, path, callback);
QThreadPool::globalInstance()->start(d->readPids);
auto readPidsRunnable = new CGroupPrivate::ReadPidsRunnable(context, path, callback);
QThreadPool::globalInstance()->start(readPidsRunnable);
}
QString CGroupPrivate::unescapeName(const QString &name) {
......@@ -187,8 +161,6 @@ QString CGroupPrivate::unescapeName(const QString &name) {
return rc;
}
KService::Ptr CGroupPrivate::serviceFromAppId(const QString &processGroup)
{
const int lastSlash = processGroup.lastIndexOf(QLatin1Char('/'));
......
......@@ -59,17 +59,24 @@ public:
*/
QVector<pid_t> pids() const;
/**
* @internal
*/
void setPids(const QVector<pid_t> &pids);
/**
* Request fetching the list of processes associated with this cgroup.
*
* This is done in a separate thread. Once it has completed, \p callback is
* called with the list of pids of this cgroup.
*
* It is the callers responsibility to call setPids in response.
*
* \param context An object that is used to track if the caller still exists.
* \param callback A callback that gets called once the list of pids has
* been retrieved.
*/
void requestPids(QPointer<QObject> context, std::function<void()> callback);
void requestPids(QObject *context, std::function<void(QVector<pid_t>)> callback);
/**
* Returns the base path to exposed cgroup information. Either /sys/fs/cgroup or /sys/fs/cgroup/unified as applicable
......
......@@ -429,9 +429,12 @@ void CGroupDataModel::update(CGroup *node)
// Update our own stat info
// This may trigger some dataChanged
node->requestPids(this, [this, node]() {
node->requestPids(this, [this, node](QVector<pid_t> pids) {
auto row = d->m_cGroups.indexOf(node);
Q_EMIT dataChanged(index(row, 0, QModelIndex()), index(row, columnCount()-1, QModelIndex()));
if (row >= 0) {
d->m_cGroups[row]->setPids(pids);
Q_EMIT dataChanged(index(row, 0, QModelIndex()), index(row, columnCount()-1, QModelIndex()));
}
});
......
Supports Markdown
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