Skip to content

Commit

Permalink
fix: fix crash caused by multi-threaded access
Browse files Browse the repository at this point in the history
Remove the thread in `EventAdaptor` and use the main thread timer to
dispatch events directly. `EventSource_GENL` asynchronously calls
`MountCacher::updateMountPoints` to avoid multi-threaded access. Use
`Q_GLOBAL_STATIC` to create a singleton, and do not allow static
acquisition.

Log: fix deepin-anything-server crash
Bug: https://pms.uniontech.com/bug-view-264761.html
  • Loading branch information
wangrong1069 committed Jul 18, 2024
1 parent dc7daa8 commit 66975b3
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 134 deletions.
44 changes: 30 additions & 14 deletions src/server/backend/eventsource_genl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -195,28 +195,44 @@ void update_vfs_unnamed_device(const QSet<QByteArray> &news)

void EventSource_GENL::updatePartitions()
{
// 有mount/unmount事件,更新mount cache
MountCacher::instance()->updateMountPoints();

QList<MountPoint> mountList = MountCacher::instance()->getMountPointsByRoot("/");
if (mountList.isEmpty()) {
nWarning("getMountPointsByRoot(/) return empty");
/* Invokes updateMountPoints in the event loop of MountCacher to avoid multi-threaded access */
QMetaObject::invokeMethod(MountCacher::instance(), "updateMountPoints", Qt::QueuedConnection);

/*
* No use`MountCacher::instance()->getMountPointsByRoot("/")` to get mount list.
* This is to avoid multi-threaded access, and locks may cause dead locks.
*/
QString file_mountinfo_path("/proc/self/mountinfo");
QFile file_mountinfo(file_mountinfo_path);
if (!file_mountinfo.open(QIODevice::ReadOnly | QIODevice::Text)) {
QByteArray ba = file_mountinfo_path.toLatin1();
nWarning("open file failed: %s.", ba.data());
return;
}
QByteArray mount_info;
mount_info = file_mountinfo.readAll();
file_mountinfo.close();

unsigned int major, minor;
char mp[256], root[256], type[256], *line = mount_info.data();
QSet<QByteArray> dlnfs_devs;
QByteArray ba;
partitions.clear();
nInfo("updatePartitions start.");
for(MountPoint info : mountList) {
unsigned long devno = info.deviceId;
major = major(devno);
minor = minor(devno);
partitions.insert(MKDEV(major, minor), info.mountTarget.toLocal8Bit());
if (info.mountType == "fuse.dlnfs") {
ba.setNum(minor);
dlnfs_devs.insert(ba);
while (sscanf(line, "%*d %*d %u:%u %250s %250s %*s %*s %*s %250s %*s %*s\n", &major, &minor, root, mp, type) == 5) {
line = strchr(line, '\n') + 1;

if (!major && strcmp(type, "fuse.dlnfs"))
continue;

if (!strcmp(root, "/")) {
partitions.insert(MKDEV(major, minor), QByteArray(mp));
nInfo("%u:%u, %s", major, minor, mp);
/* add monitoring for dlnfs device */
if (!major && !strcmp(type, "fuse.dlnfs")) {
ba.setNum(minor);
dlnfs_devs.insert(ba);
}
}
}
update_vfs_unnamed_device(dlnfs_devs);
Expand Down
70 changes: 22 additions & 48 deletions src/server/backend/lib/eventadaptor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,9 @@ DAS_BEGIN_NAMESPACE
EventAdaptor::EventAdaptor(QObject *parent)
: QObject(parent)
{
action_buffers.clear();

connect(&handle_timer, &QTimer::timeout, this, &EventAdaptor::onHandleEvent, Qt::QueuedConnection);
// 每500ms 触发一次事件
connect(&handle_timer, &QTimer::timeout, this, &EventAdaptor::onHandleEvent);
handle_timer.setInterval(500);
handle_timer.start();
jobFinished = true;
}

EventAdaptor::~EventAdaptor()
Expand All @@ -26,55 +22,22 @@ EventAdaptor::~EventAdaptor()

void EventAdaptor::pushEvent(QPair<QByteArray, QByteArray> &action)
{
QMutexLocker locker(&mutex); // 加锁
QMutexLocker locker(&mutex);
action_buffers.enqueue(action);
waitCondition.wakeAll();
}

bool EventAdaptor::popEvent(QPair<QByteArray, QByteArray> *action)
{
QMutexLocker locker(&mutex);
if (action_buffers.isEmpty()) {
return false;
}
QMutexLocker locker(&mutex); // 加锁
*action = action_buffers.dequeue();
waitCondition.wakeAll();
return true;
}

void EventAdaptor::onHandleEvent()
{
// 单线程保证索引串行更新
if (!jobFinished) {
return;
}
QMetaObject::invokeMethod(this, "startWork", Qt::QueuedConnection);
}


bool EventAdaptor::ignoreAction(QByteArray &strArr, bool ignored)
{
QString strPath = QString::fromLocal8Bit(strArr);
if (strPath.endsWith(".longname")) {
// 长文件名记录文件,直接忽略
return true;
}

//没有标记忽略前一条,则检查是否长文件目录
if (!ignored) {
// 向上找到一个当前文件的挂载点且匹配文件系统类型
if (MountCacher::instance()->pathMatchType(strPath, "fuse.dlnfs")) {
// 长文件目录,标记此条事件被忽略
return true;
}
}
return false;
}

void EventAdaptor::startWork()
{
jobFinished = false;

bool pop = false;
QList<QPair<QByteArray, QByteArray>> tmpActions;
bool ignored = false;
Expand All @@ -92,17 +55,28 @@ void EventAdaptor::startWork()
}
} while (pop);

// Start the worker thread
TaskThread* taskThread = new TaskThread(this);
connect(taskThread, &TaskThread::workFinished, this, &EventAdaptor::handleTaskFinish);
connect(taskThread, &QThread::finished, taskThread, &QThread::deleteLater);
taskThread->setData(onHandler, tmpActions);
taskThread->start();
if (!tmpActions.isEmpty()) {
onHandler(tmpActions);
}
}

void EventAdaptor::handleTaskFinish()
bool EventAdaptor::ignoreAction(QByteArray &strArr, bool ignored)

Check warning on line 63 in src/server/backend/lib/eventadaptor.cpp

View workflow job for this annotation

GitHub Actions / cppcheck

Parameter 'strArr' can be declared with const
{
jobFinished = true;
QString strPath = QString::fromLocal8Bit(strArr);
if (strPath.endsWith(".longname")) {
// 长文件名记录文件,直接忽略
return true;
}

//没有标记忽略前一条,则检查是否长文件目录
if (!ignored) {
// 向上找到一个当前文件的挂载点且匹配文件系统类型
if (MountCacher::instance()->pathMatchType(strPath, "fuse.dlnfs")) {
// 长文件目录,标记此条事件被忽略
return true;
}
}
return false;
}

DAS_END_NAMESPACE
43 changes: 0 additions & 43 deletions src/server/backend/lib/eventadaptor.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,7 @@
#include <QObject>
#include <QTimer>
#include <QMutex>
#include <QWaitCondition>
#include <QQueue>
#include <QThread>

DAS_BEGIN_NAMESPACE

Expand All @@ -31,10 +29,6 @@ class EventAdaptor : public QObject
void pushEvent(QPair<QByteArray, QByteArray> &action);
bool popEvent(QPair<QByteArray, QByteArray> *action);

public slots:
void startWork();
void handleTaskFinish();

public:
OnHandleEvent onHandler;

Expand All @@ -46,45 +40,8 @@ private slots:

private:
QMutex mutex;
QWaitCondition waitCondition;
QQueue<QPair<QByteArray, QByteArray>> action_buffers;
QTimer handle_timer;
// 用于事件更新,串行进行
bool jobFinished = true;
};

class TaskThread : public QThread
{
Q_OBJECT
public:
explicit TaskThread(QObject *parent = nullptr) : QThread(parent) {}
~TaskThread() override {
handleFunc = nullptr;
actionList.clear();
deleteLater();
}
void run() override
{
// 后台回调处理事件,更新索引
if (handleFunc)
handleFunc(actionList);

emit workFinished();
}

public slots:
void setData(OnHandleEvent callbackFunc, QList<QPair<QByteArray, QByteArray>> &actions)
{
handleFunc = callbackFunc;
actionList = actions;
}

signals:
void workFinished();

private:
OnHandleEvent handleFunc = nullptr;
QList<QPair<QByteArray, QByteArray>> actionList;
};

DAS_END_NAMESPACE
Expand Down
10 changes: 4 additions & 6 deletions src/server/backend/lib/lftdisktool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,17 @@ namespace LFTDiskTool {
Q_GLOBAL_STATIC(DDiskManager, _global_diskManager)
Q_LOGGING_CATEGORY(logN, "anything.normal.disktool", DEFAULT_MSG_TYPE)

static deepin_anything_server::MountCacher *s_mountCacher = deepin_anything_server::MountCacher::instance();

QByteArray pathToSerialUri(const QString &path)
{
// 避免使用QDir/QStorageInfo, 如果IO被阻塞(U盘正在被扫描或网络盘断网),则导致卡顿或假死
// 向上找到路径的真实挂载点
const QString mountPoint = s_mountCacher->findMountPointByPath(path);
const QString mountPoint = deepin_anything_server::MountCacher::instance()->findMountPointByPath(path);
if (mountPoint.isEmpty()) {
nWarning() << "pathToSerialUri findMountPointByPath NULL for:" << path;

Check warning on line 28 in src/server/backend/lib/lftdisktool.cpp

View workflow job for this annotation

GitHub Actions / cppcheck

syntax error
return QByteArray();
}

const QString device = s_mountCacher->getDeviceByPoint(mountPoint);
const QString device = deepin_anything_server::MountCacher::instance()->getDeviceByPoint(mountPoint);
if (!device.startsWith("/dev/") || device.startsWith("/dev/loop") || device.compare("/dev/fuse") == 0) {
nWarning() << "ingore device:" << device;
return QByteArray();
Expand All @@ -51,7 +49,7 @@ QByteArray pathToSerialUri(const QString &path)
if (block_id.isEmpty())
return QByteArray();

const auto mount_info_map = s_mountCacher->getRootsByPoints({mountPoint.toLocal8Bit()});
const auto mount_info_map = deepin_anything_server::MountCacher::instance()->getRootsByPoints({mountPoint.toLocal8Bit()});
QByteArray mount_root_path;

// 只获取一个,返回就取第一个值
Expand Down Expand Up @@ -94,7 +92,7 @@ QByteArrayList fromSerialUri(const QByteArray &uri)

if (_block_id == block_id) {
const QByteArrayList &mount_points = block_obj->mountPoints();
const auto mount_point_infos = s_mountCacher->getRootsByPoints(mount_points);
const auto mount_point_infos = deepin_anything_server::MountCacher::instance()->getRootsByPoints(mount_points);
QByteArrayList pathList;

for (QByteArray mount_point : mount_points) {
Expand Down
24 changes: 5 additions & 19 deletions src/server/backend/lib/mountcacher.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ extern "C" {

DAS_BEGIN_NAMESPACE

class _MountCacher : public MountCacher {};
Q_GLOBAL_STATIC(_MountCacher, mountCacherGlobal)

/* error callback */
static int parser_errcb(libmnt_table *tb, const char *filename, int line)
{
Expand All @@ -34,14 +37,13 @@ struct SPMntTableDeleter

MountCacher *MountCacher::instance()
{
static MountCacher ins;
return &ins;
return mountCacherGlobal;
}

MountCacher::MountCacher(QObject *parent)
: QObject(parent)
{
mountPointList.clear();
updateMountPoints();
}

MountCacher::~MountCacher()
Expand Down Expand Up @@ -103,10 +105,6 @@ QString MountCacher::findMountPointByPath(const QString &path, bool hardreal)
{
QString result;
QString result_path = path;
if (hardreal) {
// 如果跳过虚拟,查找真实设备的挂载点,须检查挂载信息是否已获取
checkCurrentMounts();
}

Q_FOREVER {
char *checkpath = QFile::encodeName(result_path).data();
Expand Down Expand Up @@ -158,7 +156,6 @@ bool MountCacher::pathMatchType(const QString &path, const QString &type)
{
bool result= false;
QString point = findMountPointByPath(path);
checkCurrentMounts();

for (MountPoint info: mountPointList) {
if (point == info.mountTarget && type == info.mountType) {

Check warning on line 161 in src/server/backend/lib/mountcacher.cpp

View workflow job for this annotation

GitHub Actions / cppcheck

Consider using std::any_of algorithm instead of a raw loop.
Expand All @@ -173,7 +170,6 @@ bool MountCacher::pathMatchType(const QString &path, const QString &type)
QMap<QByteArray, QString> MountCacher::getRootsByPoints(const QByteArrayList &pointList)

Check warning on line 170 in src/server/backend/lib/mountcacher.cpp

View workflow job for this annotation

GitHub Actions / cppcheck

The function 'getRootsByPoints' is never used.
{
QMap<QByteArray, QString> map;
checkCurrentMounts();

for (const QByteArray &point : pointList) {
const QString target = QString(point);
Expand All @@ -191,7 +187,6 @@ QMap<QByteArray, QString> MountCacher::getRootsByPoints(const QByteArrayList &po
QString MountCacher::getDeviceByPoint(const QString &point)

Check warning on line 187 in src/server/backend/lib/mountcacher.cpp

View workflow job for this annotation

GitHub Actions / cppcheck

The function 'getDeviceByPoint' is never used.
{
QString device;
checkCurrentMounts();

for (MountPoint info: mountPointList) {
if (point == info.mountTarget) {

Check warning on line 192 in src/server/backend/lib/mountcacher.cpp

View workflow job for this annotation

GitHub Actions / cppcheck

Consider using std::find_if algorithm instead of a raw loop.
Expand All @@ -205,7 +200,6 @@ QString MountCacher::getDeviceByPoint(const QString &point)
// 获取所有根为指定的挂载点, 不指定则返回全部挂载点信息
QList<MountPoint> MountCacher::getMountPointsByRoot(const QString &root)

Check warning on line 201 in src/server/backend/lib/mountcacher.cpp

View workflow job for this annotation

GitHub Actions / cppcheck

The function 'getMountPointsByRoot' is never used.
{
checkCurrentMounts();
if (root == nullptr || !root.startsWith('/')) {
return mountPointList;
}
Expand All @@ -219,14 +213,6 @@ QList<MountPoint> MountCacher::getMountPointsByRoot(const QString &root)
return list;
}

void MountCacher::checkCurrentMounts()
{
if (mountPointList.isEmpty()) {
nWarning() << "mountPointList is empty, updat it first.";
updateMountPoints();
}
}

QDebug operator<<(QDebug debug, const MountPoint &mp)
{
QDebugStateSaver saver(debug);
Expand Down
9 changes: 5 additions & 4 deletions src/server/backend/lib/mountcacher.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ class MountCacher : public QObject
Q_DISABLE_COPY(MountCacher)

public:
~MountCacher();
static MountCacher *instance();
bool updateMountPoints();
QString findMountPointByPath(const QString &path, bool hardreal = false);
bool pathMatchType(const QString &path, const QString &type);

Expand All @@ -43,11 +43,12 @@ class MountCacher : public QObject
// 获取所有根为指定的挂载点, 不指定则返回全部挂载点信息
QList<MountPoint> getMountPointsByRoot(const QString &root = nullptr);

private:
public slots:
bool updateMountPoints();

protected:
explicit MountCacher(QObject *parent = nullptr);
~MountCacher();

void checkCurrentMounts();
private:
QList<MountPoint> mountPointList;
};
Expand Down

0 comments on commit 66975b3

Please sign in to comment.