Skip to content

fix(recordtime-plugin): replace heartbeat polling with QDBusServiceWa…#846

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
dengzhongyuan365-dev:bug-fix-5-21
May 27, 2026
Merged

fix(recordtime-plugin): replace heartbeat polling with QDBusServiceWa…#846
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
dengzhongyuan365-dev:bug-fix-5-21

Conversation

@dengzhongyuan365-dev

Copy link
Copy Markdown
Member

…tcher

The dock plugin previously detected main-process death by counting incoming onRecording() heartbeats from the recorder's emitRecording() loop and starting a 2s checkTimer to compare m_count vs m_nextCount. Any transient stall in the main process (audio capture blocked, IO backpressure, scheduler starvation on low-end machines) silenced the heartbeat and made the plugin falsely conclude the recorder had stopped — triggering onStop() -> clear() and destroying the timer widget, so the next heartbeat would re-create it from zero and the displayed time visibly restarted.

Switch to event-driven liveness: install a QDBusServiceWatcher on "com.deepin.ScreenRecorder" with WatchForUnregistration. dbus-daemon auto-unregisters the bus name only when the owning process actually exits, so a busy-but-alive recorder no longer trips the watchdog. Normal stop still arrives via the explicit DBus onStop() call; the watcher only covers crash cleanup.

Also guard onStop() against a null m_timeWidget — a stop-then-crash sequence can fire the watcher after clear() has already nulled it.

将托盘插件的"主程序存活检测"从基于心跳计数的轮询定时器改为
QDBusServiceWatcher 事件订阅。原方案在主程序因音频卡顿等原因
暂停心跳时会误判为崩溃,触发 onStop -> clear 销毁计时控件,
下一次心跳到来时重新初始化导致显示时间从零重启。改用 watcher
监听 bus name 注销事件后,仅主程序真实退出时才触发清理;同时
为 onStop 增加 m_timeWidget 空指针保护以应对 stop 后再触发的
边缘时序。

Log: 修复录屏过程中主程序短暂卡顿导致任务栏插件计时清零重启的问题

PMS: BUG-330567

Influence: 修复后主程序在音频抓取等场景下短暂卡顿不再触发插件误判停止,
任务栏录制时间显示连续不重置;主程序真实崩溃时仍能由 watcher 及时清理。

…tcher

The dock plugin previously detected main-process death by counting
incoming onRecording() heartbeats from the recorder's emitRecording()
loop and starting a 2s checkTimer to compare m_count vs m_nextCount.
Any transient stall in the main process (audio capture blocked, IO
backpressure, scheduler starvation on low-end machines) silenced the
heartbeat and made the plugin falsely conclude the recorder had
stopped — triggering onStop() -> clear() and destroying the timer
widget, so the next heartbeat would re-create it from zero and the
displayed time visibly restarted.

Switch to event-driven liveness: install a QDBusServiceWatcher on
"com.deepin.ScreenRecorder" with WatchForUnregistration. dbus-daemon
auto-unregisters the bus name only when the owning process actually
exits, so a busy-but-alive recorder no longer trips the watchdog.
Normal stop still arrives via the explicit DBus onStop() call; the
watcher only covers crash cleanup.

Also guard onStop() against a null m_timeWidget — a stop-then-crash
sequence can fire the watcher after clear() has already nulled it.

将托盘插件的"主程序存活检测"从基于心跳计数的轮询定时器改为
QDBusServiceWatcher 事件订阅。原方案在主程序因音频卡顿等原因
暂停心跳时会误判为崩溃,触发 onStop -> clear 销毁计时控件,
下一次心跳到来时重新初始化导致显示时间从零重启。改用 watcher
监听 bus name 注销事件后,仅主程序真实退出时才触发清理;同时
为 onStop 增加 m_timeWidget 空指针保护以应对 stop 后再触发的
边缘时序。

Log: 修复录屏过程中主程序短暂卡顿导致任务栏插件计时清零重启的问题

PMS: BUG-330567

Influence: 修复后主程序在音频抓取等场景下短暂卡顿不再触发插件误判停止,
任务栏录制时间显示连续不重置;主程序真实崩溃时仍能由 watcher 及时清理。
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX。我已仔细审查了你提供的Git Diff。

这次代码变更的核心逻辑是将基于心跳定时器的崩溃检测机制替换为基于 QDBusServiceWatcher 的服务注销监听机制。这是一个非常优秀的重构!原方案通过定时器轮询比对计数来判断主进程是否存活,不仅存在延迟,还可能因为同步D-Bus调用阻塞UI;新方案利用了D-Bus底层的服务注销信号,实现了真正的“事件驱动”,响应更及时,且彻底消除了轮询带来的性能开销。

不过,在语法逻辑、代码质量和安全性方面,还有一些需要改进和注意的地方。以下是详细的审查意见:

一、 语法与逻辑

1. onStop() 中的潜在递归死循环风险(严重)

  • 问题:在 onRecording 中,serviceUnregistered 信号触发了 onStop();而在 onStop() 中,又调用了 clear()clear() 中对 m_serviceWatcher 执行了 deleteLater()
  • 场景推演:如果主进程崩溃,D-Bus服务注销,触发 onStop() -> clear() -> m_serviceWatcher->deleteLater()。但 deleteLater 是在事件循环空闲时才销毁对象。如果在 deleteLater 执行前,D-Bus底层又抛出了相关的注销或清理事件(或者由于Qt事件队列的积压导致信号重入),可能会再次触发 serviceUnregistered,从而再次调用 onStop(),导致递归调用甚至逻辑异常。
  • 改进建议:在 onStop() 被信号触发时,应立刻断开连接或将指针置空,防止重入。由于 onStop() 内部调用了 clear()(会deleteLater并置空),最安全的做法是在 onStop 入口处加一个防重入判断,或者在lambda中调用前先断开信号。

2. onRecording()m_timeWidget 的空指针解引用风险

  • 问题:在 onStop() 中你已经很好地加了 if (m_timeWidget && m_timeWidget->enabled()) 的保护。但在 onRecording() 中,直接使用了 if (m_timeWidget->enabled() && m_bshow)。如果 m_timeWidget 在此之前被置空(例如在其他槽函数或事件中被清理),这里会发生空指针崩溃。
  • 改进建议:保持判空逻辑的一致性,改为 if (m_timeWidget && m_timeWidget->enabled() && m_bshow)

二、 代码质量

1. 对象生命周期管理(智能指针的运用)

  • 问题:代码中依然在使用原始指针 m_serviceWatcher = new ... 并在 clear() 中手动 deleteLater() 和置空。虽然逻辑上没有内存泄漏,但在现代C++(特别是Qt开发)中,手动管理容易遗漏,尤其是在异常流程中。
  • 改进建议:由于 QDBusServiceWatcher 的父对象已经指定为 this(即 RecordTimePlugin),当 RecordTimePlugin 销毁时,它也会自动销毁。因此,完全可以使用 QPointer<QDBusServiceWatcher> m_serviceWatcher;。这样不仅免去了在 clear() 中手动 deleteLater() 的需要(只需调用 delete 或任其自动销毁),而且一旦对象被销毁,QPointer 会自动置为 nullptr,极大提升安全性。

2. 魔法字符串

  • 问题"com.deepin.ScreenRecorder" 作为D-Bus服务名,直接硬编码在源文件中。
  • 改进建议:建议将其提取为头文件中的 static const QString 或宏,方便后续维护和复用,避免拼写错误。

三、 代码性能

1. 定时器精度的优化

  • 问题m_timer->start(600);,注释或原逻辑中提到是“每隔1秒检查”,但实际传入的是600毫秒。虽然这不是本次修改引入的,但既然重构了这部分逻辑,建议一并修正。
  • 改进建议:如果是1秒刷新一次,应改为 m_timer->start(1000);。600ms的刷新率对于时间显示组件来说没有明显必要,还会增加无谓的CPU唤醒和重绘开销。

四、 代码安全

1. D-Bus服务名欺骗/冒用风险

  • 问题QDBusServiceWatcher 监听的是总线上名为 "com.deepin.ScreenRecorder" 的服务注销事件。在Linux桌面会话中,如果总线策略配置不当(未限制该服务名的属主),恶意程序可以注册同名服务然后迅速退出,从而诱骗插件执行 onStop() 逻辑(如隐藏Dock图标等),构成拒绝服务。
  • 改进建议:这属于系统架构层面的安全,代码层面无法完全解决。但建议确认系统D-Bus的 .conf 策略文件中,是否对 com.deepin.ScreenRecorder 做了 <allow own_user="deepin-screenrecorder"/> 类似的用户级限制。在代码注释中可以标明此假设。

综合改进后的代码建议

结合以上意见,我为你修改了部分代码,供参考:

recordtimeplugin.h

// ... 其他代码不变 ...
public slots:
    QPointer<DBusService> m_dBusService;
    bool m_bshow;

    /**
     * @brief 监听主进程服务名的注销事件,替换原心跳定时器。
     * 使用 QPointer 避免悬空指针,无需手动 deleteLater 和置空。
     */
    QPointer<QDBusServiceWatcher> m_serviceWatcher;

    static const QString MAIN_RECORDER_SERVICE; // 避免魔法字符串
// ... 其他代码不变 ...

recordtimeplugin.cpp

// 初始化静态变量
const QString RecordTimePlugin::MAIN_RECORDER_SERVICE = "com.deepin.ScreenRecorder";

RecordTimePlugin::RecordTimePlugin(QObject *parent)
    : QObject(parent)
{
    qCDebug(dsrApp) << "RecordTimePlugin constructor called.";
    m_timer = nullptr;
    m_timeWidget = nullptr;
    // m_serviceWatcher 默认构造为 nullptr,无需手动初始化
}

void RecordTimePlugin::clear()
{
    if (m_timeWidget) {
        qCDebug(dsrApp) << "Deleting time widget";
        m_timeWidget->deleteLater();
        m_timeWidget = nullptr;
    }

    if (m_serviceWatcher) {
        qCDebug(dsrApp) << "Deleting service watcher";
        // QPointer 管理下,直接 delete 即可,QPointer 自动置空
        // 如果依赖事件循环销毁,也可用 deleteLater(),但这里直接 delete 更干净
        delete m_serviceWatcher.data(); 
    }
    qCDebug(dsrApp) << "clear method finished.";
}

void RecordTimePlugin::onStart(bool resetTime)
{
    // ... 前面代码不变 ...
    m_timer->start(1000); // 修正:如果是1秒刷新,改为1000ms
    // ... 后面代码不变 ...
}

void RecordTimePlugin::onStop()
{
    qCDebug(dsrApp) << "onStop method called.";
    // 1. 防止重入:如果已经在处理停止逻辑,直接返回
    // 2. 空指针保护
    if (!m_timeWidget || !m_timeWidget->enabled()) {
        return;
    }

    qInfo() << "unload plugin";
    qCDebug(dsrApp) << "Time widget enabled, unloading plugin.";
    m_proxyInter->itemRemoved(this, pluginName());
    m_bshow = false;
    
    clear();
    qInfo() << "stop record time";
}

void RecordTimePlugin::onRecording()
{
    qCDebug(dsrApp) << "onRecording method called.";

    // 增加 m_timeWidget 的判空保护
    if (m_timeWidget && m_timeWidget->enabled() && m_bshow) {
        qCDebug(dsrApp) << "Time widget enabled and visible.";
        
        if (nullptr == m_serviceWatcher) {
            qCDebug(dsrApp) << "Installing QDBusServiceWatcher for main recorder process";
            m_serviceWatcher = new QDBusServiceWatcher(
                MAIN_RECORDER_SERVICE,
                QDBusConnection::sessionBus(),
                QDBusServiceWatcher::WatchForUnregistration,
                this);
                
            // 使用 Qt5 的 lambda 上下文对象机制,防止生命周期异常导致悬空调用
            connect(m_serviceWatcher, &QDBusServiceWatcher::serviceUnregistered,
                    this, [this](const QString &serviceName) {
                qCInfo(dsrApp) << "Main recorder service unregistered, stopping plugin";
                // 断开连接,防止 D-Bus 信号重入
                if (m_serviceWatcher) {
                    disconnect(m_serviceWatcher, &QDBusServiceWatcher::serviceUnregistered, this, nullptr);
                }
                onStop();
            });
        }
    }
    qCDebug(dsrApp) << "onRecording method finished.";
}

主要修改点总结:

  1. 使用 QPointer 管理 m_serviceWatcher,省去繁琐的 deleteLater= nullptr 操作。
  2. 将 D-Bus 服务名提取为类静态常量 MAIN_RECORDER_SERVICE
  3. onRecording 中增加了 m_timeWidget 的判空检查。
  4. serviceUnregistered 的 Lambda 中,触发 onStop 前先 disconnect,彻底杜绝信号重入导致的递归崩溃。
  5. 修正了 m_timer->start(600)1000

希望这些意见对你有所帮助!如果有任何疑问,欢迎继续探讨。

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dengzhongyuan365-dev, lzwind

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@dengzhongyuan365-dev

Copy link
Copy Markdown
Member Author

/forcemerge

@dengzhongyuan365-dev

Copy link
Copy Markdown
Member Author

/merge

1 similar comment
@dengzhongyuan365-dev

Copy link
Copy Markdown
Member Author

/merge

@deepin-bot deepin-bot Bot merged commit b1a5c14 into linuxdeepin:master May 27, 2026
10 checks passed
@dengzhongyuan365-dev

Copy link
Copy Markdown
Member Author

/forcemerge

@deepin-bot

deepin-bot Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

This pr force merged! (status: unknown)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants