screensaverinterfacev2 实现的一些问题

[wineee]

🔴 必须修复(正确性 / 会崩)

1. wl_display_connect / wl_display_roundtrip / interfaceId 全无校验

  display = wl_display_connect(nullptr);                                                                                                                                                                              
  registry = wl_display_get_registry(display);                                                                                                                                                                        
  wl_registry_add_listener(registry, &registryListener, nullptr);                                                                                                                                                     
  wl_display_roundtrip(display);                                                                                                                                                                                      

连不上 treeland → display=nullptr,wl_display_get_registry(nullptr) 行为未定义;若 roundtrip 时尚未注册 treeland_screensaver_v2 global,interfaceId 保持为 0。任一情况发生,后续 Inhibit 必崩。应在 roundtrip 后判 if
(!display || !interfaceId) return 1;,并考虑监听 global 事件做就绪等待。

2. inhibit() 不检查 wl_registry_bind 返回值

  static treeland_screensaver_v2 *inhibit(...) {                                                                                                                                                                      
      treeland_screensaver_v2 *screensaver = static_cast<...>(                                                                                                                                                        
          wl_registry_bind(registry, interfaceId, ...));                                                                                                                                                              
      treeland_screensaver_v2_inhibit(screensaver, ...);  // null → 解引用崩                                                                                                                                          

bind 失败返回 nullptr,被直接存进 inhibits,之后 uninhibit(nullptr) / treeland_screensaver_v2_destroy(nullptr) 都会崩。Inhibit 必须判空并给 D-Bus caller 返回失败。

3. onServiceOwnerChanged 是死代码

  watcher->setWatchMode(QDBusServiceWatcher::WatchForUnregistration);  // 0x02                                                                                                                                        
  QObject::connect(watcher, &QDBusServiceWatcher::serviceOwnerChanged,                                                                                                                                                
                   onServiceOwnerChanged);                                                                                                                                                                            

头文件里 WatchForOwnerChange = 0x03(Registration|Unregistration 的并集),WatchForUnregistration(0x02)模式下 serviceOwnerChanged 不会 emit,只有 serviceUnregistered 会。这段 connect 永不触发,且 onServiceOwnerChanged
与 onServiceUnregistered 逻辑完全相同(DISCONNECTED 宏)。二选一,删掉 ownerChanged 那条即可。

🟡 建议修改(健壮性 / 可维护性)

4. 没有监听协议 error 事件
   bind 后没给 proxy 加 listener。若 treeland 返回 already_inhibited / not_yet_inhibited,daemon 毫无感知,仍向 D-Bus caller 返回成功 cookie —— 语义是错的。至少要处理 error,释放对应条目并通知 caller。

5. callers 是冗余的第二份状态
   QHash<QString, QHash<uint, ...>> inhibits 已经能判断 caller 是否存在,QStringList callers 完全多余,反而要在 DISCONNECTED 宏里和 inhibits 双重维护、容易不一致。删掉 callers,统一以 inhibits 为唯一来源
   (addWatchedService 重复调用可接受)。

6. DISCONNECTED 是裸多语句宏

  #define DISCONNECTED(caller)  \                                                                                                                                                                                     
      callers.removeAll(caller); \                                                                                                                                                                                    
      watcher->removeWatchedService(caller); \                                                                                                                                                                        
      if (...) { ... uninhibit(it.value()); ... }                                                                                                                                                                     

没有 do{}while(0) 包裹,且耦合全局 callers/watcher/inhibits。改成普通函数 static void disconnectCaller(const QString &); 更安全,也顺带消掉第 5 条。

7. new ScreenSaver() 无所有权管理
   registerObject(path, object, options) 不接管 object 所有权(Qt 文档明确),这里裸 new 无 parent、无智能指针,虽是 daemon 全生命周期泄漏、进程退出回收,但不规范。给个 parent(&app)即可。

8. wl_registry_bind 直接用 interfaceVersion
   若 treeland 注册版本高于本地头文件已知的版本,bind 用注册版本可能超出客户端编译期支持。应对 min(interfaceVersion, TREELAND_SCREENSAVER_V2_*_SINCE_VERSION) 取下界。

9. 全程零日志
   daemon 崩了 / inhibit 失败 / 客户端掉线清理,全无线索。至少加几条 qWarning/qCInfo。(若要接入,按项目规则走 logging-guidelines skill。)

🟢 仅供参考(风格)

• 整体是文件级全局可变状态(display/registry/notifier/inhibits/watcher...)+ 自由函数,可封装成一个 Daemon 类,构造里连接、成员持有状态,Inhibit/UnInhibit 作槽函数,可测试性和可读性都更好。
• inhibits.emplace(caller) 单参数 emplace 能编译但别扭,不如 inhibits[caller] 直观。
• watcher 缺 static,external linkage 暴露符号。
• UnInhibit 找不到 cookie 时静默忽略,可记一条日志便于排查误用。
• cookieCounter 全局 uint 单调递增有回绕(实际风险极低,可忽略)。

[wineee]

读完 helper、协议 xml 和 server 侧(src/modules/app-id-resolver/appidresolver.cpp)后,评审如下。

🔴 必须修复(架构 / 会挂)

1. identify_request 里同步阻塞 DBus 调用,堵死 Wayland 事件链

  void treeland_app_id_resolver_v2_identify_request(...) override {                                                                                                                                                   
      const QString appId = identifyViaDBus(pidfd);   // ← QDBus::BlockWithGui                                                                                                                                        
      respond(request_id, appId, kSandboxEngine);                                                                                                                                                                     
  }                                                                                                                                                                                                                   

identifyViaDBus 用 QDBusConnection::call(msg, QDBus::BlockWithGui)。这是在 Wayland 事件分发回调里嵌套一个阻塞事件循环:

• 协议文档明确 "Failure to respond in a timely manner may cause the compositor to treat the process as unidentified"。这里"及时"完全取决于 dde-application-manager 的响应速度。
• BlockWithGui 会进嵌套事件循环,期间新的 identify_request 也能进来,各自再进嵌套 BlockWithGui —— 多层递归嵌套事件循环,respond 的发出顺序与 request_id 不再保证对应,栈也可能增长。
• 对比 server 侧用的是 std::function 异步回调模型,helper 这端却用同步阻塞,两边风格不一致。

正解:改 async。QDBusPendingCallWatcher 监听 reply,在 ready 槽里再 respond(request_id, ...)。这样回调立即返回,Wayland 事件循环不阻塞,多个请求并发也安全。

2. 阻塞调用没有超时

call(msg, QDBus::BlockWithGui) 无 timeout 重载,AM 若卡死/不回,这条 identify_request 永久挂起,treeland 侧对应 callback 也永不触发。改 async 后用 QDBusPendingCallWatcher + 自带超时定时器兜底,或 callWithCallback(...,
timeout)。

[wineee]

两个客户端可以考虑合并