fix(screenshot): prevent multiple simultaneous screenshot requests

[dengzhongyuan365-dev]

• Updated the screenshot service to ignore new requests if a screenshot is already in progress, ensuring that only one instance of the screenshot process runs at a time.
• Added debug logging to indicate when a screenshot is already in progress and when a new screenshot instance is started.

This change improves the user experience by preventing overlapping screenshot operations.

bug: https://pms.uniontech.com/bug-view-357037.html

[deepin-ci-robot]

deepin pr auto review

这段代码的修改主要是为了修复一个竞态条件问题，即防止在截图操作进行时（特别是当模态对话框打开时）重复触发新的截图任务。以下是对这段代码的详细审查和改进建议：

1. 代码逻辑与语法审查

当前逻辑分析：
修改后的代码逻辑是正确的。

• 原代码问题：原代码先检查 !m_singleInstance，然后才设置 m_singleInstance = true。如果在检查之后、设置标志位之前，或者在 parent()->xxxScreenshot() 执行期间（如打开文件对话框时），D-Bus 再次收到调用，由于标志位尚未设置或事件循环正在运行，可能会导致重复调用。
• 修改后逻辑：先检查 if (m_singleInstance)，如果正在进行则直接 return。紧接着设置 m_singleInstance = true。这确保了在任何耗时操作（包括可能触发嵌套事件循环的 QFileDialog）开始前，标志位已经生效。这有效地防止了重入。

语法正确性：
代码符合 C++ 和 Qt 的语法规范，没有明显的语法错误。

2. 代码质量与改进建议

虽然逻辑是正确的，但在代码质量和健壮性方面还有提升空间：

建议一：使用 RAII 机制管理标志位（避免忘记重置）
目前代码在函数开始时设置 m_singleInstance = true，但并没有在当前作用域内显式地将其重置为 false。这意味着必须在 parent()->xxxScreenshot() 内部，或者在截图流程结束后的某个回调中将该标志位改回 false。如果截图流程异常退出或被取消，很容易导致标志位卡死，之后永远无法再截图。

改进方案：使用 Qt 的 QScopeGuard（C++17及以上）或者自定义的辅助类，确保在函数退出时（无论是正常返回还是异常抛出）自动重置标志位。

#include <QScopeGuard> // 需要包含头文件

void DBusScreenshotService::TopWindowScreenshot()
{
    // ... 日志记录代码 ...

    if (m_singleInstance) {
        qCDebug(dsrApp) << "Screenshot already in progress, ignoring.";
        return;
    }
    
    m_singleInstance = true;
    
    // 使用 QScopeGuard 确保函数退出时重置标志位
    // 注意：这要求截图操作是同步的，或者你需要更复杂的生命周期管理
    // 如果 parent()->topWindowScreenshot() 是异步的，这个 guard 会导致函数返回时标志位即失效，
    // 那么你需要将 guard 的管理移到异步回调中。
    // 假设当前是同步阻塞调用（如注释提到的 QFileDialog）：
    auto guard = qScopeGuard([this]() { 
        m_singleInstance = false; 
        qCDebug(dsrApp) << "Screenshot instance flag reset.";
    });

    qCDebug(dsrApp) << "Starting new top window screenshot instance";
    parent()->topWindowScreenshot();
    qCDebug(dsrApp) << "TopWindowScreenshot method finished.";
}

注意：如果 parent()->topWindowScreenshot() 是异步操作（即立即返回，截图在后台进行），则不能使用 QScopeGuard 在函数末尾重置，否则会导致并发。在这种情况下，必须保持原有的在回调中重置的逻辑，但要确保异常路径也能重置。

建议二：封装重复逻辑
TopWindowScreenshot 和 FullscreenScreenshot 的逻辑高度重复（检查标志位、设置标志位、调用父类方法）。建议提取公共逻辑以减少维护成本。

改进方案：

// 在类内部定义一个私有辅助方法
void DBusScreenshotService::startScreenshotTask(std::function<void()> task, const QString &logName) {
    if (m_singleInstance) {
        qCDebug(dsrApp) << "Screenshot already in progress, ignoring.";
        return;
    }

    m_singleInstance = true;
    // 如果是同步阻塞操作：
    auto guard = qScopeGuard([this]() { m_singleInstance = false; });

    qCDebug(dsrApp) << "Starting new" << logName << "instance";
    task();
    qCDebug(dsrApp) << logName << "method finished.";
}

// 调用方式
void DBusScreenshotService::TopWindowScreenshot() {
    // ... 日志 ...
    startScreenshotTask([this](){ parent()->topWindowScreenshot(); }, "top window screenshot");
}

3. 代码性能

• 当前性能：性能良好。增加的布尔检查和赋值操作耗时极低（纳秒级），对性能无影响。
• 潜在问题：如果 parent()->xxxScreenshot() 内部确实使用了 QFileDialog，这会阻塞 D-Bus 服务的主线程。虽然这防止了并发，但如果用户长时间不关闭对话框，D-Bus 服务的其他方法调用也会被阻塞。这在单线程 D-Bus 服务中是常见权衡，但需注意。

4. 代码安全

• 线程安全：

  • m_singleInstance 是一个成员变量。如果 DBusScreenshotService 运行在多线程环境中（例如 D-Bus 调用在不同线程处理），直接读写 bool m_singleInstance 是非线程安全的。
  • Qt 的 D-Bus 通常在主线程事件循环中处理，如果此服务仅运行在主线程，则是安全的。
  • 改进建议：如果存在多线程调用的可能，应使用 QAtomicBool 或 std::atomic<bool> 代替普通 bool，或者使用 QMutex 进行保护。

  // 头文件中
  std::atomic<bool> m_singleInstance{false};
  
  // 或者使用 QAtomicBool
  // QAtomicBool m_singleInstance;

• 重入安全：

  • 修改后的代码通过在调用耗时函数前设置标志位，很好地解决了嵌套事件循环（由 QFileDialog 引起）导致的重入问题。这是一个很好的安全修复。

总结

总体评价：这是一个高质量的 Bug 修复，准确地解决了模态对话框导致的并发问题。

最终建议：

1. 确认线程模型：确保 D-Bus 调用都在同一线程，否则请将 m_singleInstance 改为原子类型。
2. 异常安全：务必确认 m_singleInstance = false 在所有可能的退出路径（包括用户取消、程序崩溃、异常抛出）下都能被执行。使用 RAII (QScopeGuard) 是最佳实践，前提是截图任务是同步阻塞的。
3. 代码去重：考虑提取公共逻辑以保持代码整洁。

[deepin-ci-robot]

[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:

• debian/deepin/OWNERS

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

[dengzhongyuan365-dev]

/forcemerge