Skip to content

Conversation

@mhduiy
Copy link
Contributor

@mhduiy mhduiy commented Jan 20, 2026

如果不手动设置APPID,dtk的dsgapplication会通过am的接口去获取appid,如果使用dconfig的程序是am,则是自己阻塞调用自己,会卡死

Set the DSG_APP_ID environment variable to ApplicationManagerConfig before initializing the application. This prevents the application manager from calling its own Identify interface when accessing its own dconfig, which would cause a deadlock.

Log: Fixed a potential deadlock issue when the application manager accesses its own configuration

Influence:

  1. Test application manager startup and initialization
  2. Verify that the application manager can read its own dconfig without hanging
  3. Test interaction with other DSG applications to ensure environment variable doesn't interfere
  4. Monitor system logs for any identification-related errors during startup

fix: 设置DSG_APP_ID环境变量防止自身识别死锁

在应用程序初始化之前设置DSG_APP_ID环境变量为ApplicationManagerConfig。这 可以防止应用程序管理器在访问自身dconfig时调用自己的Identify接口,从而避
免死锁发生。

Log: 修复了应用程序管理器访问自身配置时可能出现的死锁问题

Influence:

  1. 测试应用程序管理器的启动和初始化过程
  2. 验证应用程序管理器能够正常读取自身dconfig而不出现挂起
  3. 测试与其他DSG应用程序的交互,确保环境变量设置不会产生干扰
  4. 监控系统日志,检查启动过程中是否有识别相关的错误

PMS: BUG-345379

Summary by Sourcery

Bug Fixes:

  • Prevent a potential deadlock caused by the application manager invoking its own Identify interface while reading its own configuration.

Set the DSG_APP_ID environment variable to ApplicationManagerConfig
before initializing the application. This prevents the application
manager from calling its own Identify interface when accessing its own
dconfig, which would cause a deadlock.

Log: Fixed a potential deadlock issue when the application manager
accesses its own configuration

Influence:
1. Test application manager startup and initialization
2. Verify that the application manager can read its own dconfig without
hanging
3. Test interaction with other DSG applications to ensure environment
variable doesn't interfere
4. Monitor system logs for any identification-related errors during
startup

fix: 设置DSG_APP_ID环境变量防止自身识别死锁

在应用程序初始化之前设置DSG_APP_ID环境变量为ApplicationManagerConfig。这
可以防止应用程序管理器在访问自身dconfig时调用自己的Identify接口,从而避
免死锁发生。

Log: 修复了应用程序管理器访问自身配置时可能出现的死锁问题

Influence:
1. 测试应用程序管理器的启动和初始化过程
2. 验证应用程序管理器能够正常读取自身dconfig而不出现挂起
3. 测试与其他DSG应用程序的交互,确保环境变量设置不会产生干扰
4. 监控系统日志,检查启动过程中是否有识别相关的错误

PMS: BUG-345379
@sourcery-ai
Copy link

sourcery-ai bot commented Jan 20, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Sets the DSG_APP_ID environment variable to ApplicationManagerConfig at application startup to prevent the application manager from self-identifying via D-Bus when reading its own configuration, which previously could cause a deadlock.

Sequence diagram for application manager startup with DSG_APP_ID set

sequenceDiagram
    participant System
    participant ApplicationManager
    participant DConfigService
    participant IdentifyInterface

    System->>ApplicationManager: start(argc, argv)
    ApplicationManager->>ApplicationManager: qputenv(DSG_APP_ID, ApplicationManagerConfig)
    ApplicationManager->>DConfigService: loadOwnDConfig()
    DConfigService->>IdentifyInterface: identify(DSG_APP_ID)
    IdentifyInterface-->>DConfigService: return identity for ApplicationManagerConfig
    DConfigService-->>ApplicationManager: return configuration
    ApplicationManager->>ApplicationManager: continue initialization without deadlock
Loading

Flow diagram for DSG_APP_ID preventing self-identification deadlock

flowchart TD
    A["ApplicationManager main() start"] --> B["Set environment DSG_APP_ID = ApplicationManagerConfig via qputenv"]
    B --> C["Access own dconfig via DConfigService"]
    C --> D["DConfigService calls IdentifyInterface with DSG_APP_ID"]
    D --> E["IdentifyInterface resolves to ApplicationManagerConfig without reentering ApplicationManager"]
    E --> F["Configuration returned successfully"]
    F --> G["ApplicationManager initialization completes without deadlock"]
Loading

File-Level Changes

Change Details Files
Initialize DSG_APP_ID before application startup to avoid self-identification deadlock in the application manager.
  • Call qputenv to set the DSG_APP_ID environment variable to ApplicationManagerConfig at the very beginning of main()
  • Ensure the environment is configured before any profiling, initialization, or D-Bus usage occurs so that the application manager does not call its own Identify interface when accessing its own dconfig
apps/dde-application-manager/src/main.cpp

Possibly linked issues

  • #(unknown): The PR implements support for the DSG_APP_ID environment variable as requested in the issue, preventing self-deadlock.

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue, and left some high level feedback:

  • Consider checking whether DSG_APP_ID is already set before calling qputenv so that the application manager does not unintentionally override a value provided by the environment or other components.
  • It would be helpful to add a brief code comment near the qputenv call explaining that this prevents the application manager from calling its own Identify interface and causing a deadlock, to make the rationale clear to future maintainers.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Consider checking whether DSG_APP_ID is already set before calling qputenv so that the application manager does not unintentionally override a value provided by the environment or other components.
- It would be helpful to add a brief code comment near the qputenv call explaining that this prevents the application manager from calling its own Identify interface and causing a deadlock, to make the rationale clear to future maintainers.

## Individual Comments

### Comment 1
<location> `apps/dde-application-manager/src/main.cpp:40` </location>
<code_context>

 int main(int argc, char *argv[])
 {
+    qputenv("DSG_APP_ID", ApplicationManagerConfig);
 #ifdef PROFILING_MODE
     auto start = std::chrono::high_resolution_clock::now();
</code_context>

<issue_to_address>
**suggestion (bug_risk):** Consider preserving a pre-set DSG_APP_ID instead of unconditionally overriding it

This unconditionally overrides any `DSG_APP_ID` already set in the environment, which could break setups that rely on their own value (e.g., debugging or multi-instance configurations). To treat this as a default instead, consider using `qEnvironmentVariableIsSet` / `qEnvironmentVariableIsEmpty` and only calling `qputenv` when the variable is not already defined.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@deepin-ci-robot
Copy link

deepin pr auto review

这段代码是在 main 函数中增加了一行设置环境变量的代码,用于设置 DSG (Deepin Style Guide) 应用 ID。以下是对这段代码的审查意见和改进建议:

1. 语法逻辑

  • 审查结果:语法正确。
  • 说明qputenv 是 Qt 提供的标准函数,用于设置环境变量。逻辑上,在 main 函数开始时尽早设置环境变量是合理的,这样可以确保后续初始化的 Qt 组件(如 QGuiApplication)能够读取到该环境变量。

2. 代码质量

  • 审查结果:存在改进空间。
  • 问题
    • 宏定义未展示:代码中使用了 ApplicationManagerConfig,但 diff 中未包含该宏的定义。如果该宏是一个字符串字面量(例如 "com.deepin.dde.ApplicationManager"),则没有问题。如果它是一个未定义的变量或宏,编译会失败。
    • 硬编码风险ApplicationManagerConfig 看起来像是一个宏。建议确保该宏定义在头文件中清晰可见,且值符合 DSG 的命名规范。
  • 改进建议
    • 确认 ApplicationManagerConfig 已在项目配置文件(如 .proCMakeLists.txt)或头文件中正确定义。
    • 如果 ApplicationManagerConfig 是一个字符串,建议使用 QLatin1StringQStringLiteral 包裹(如果 qputenv 支持重载或显式转换),以减少潜在的编码转换开销,尽管 qputenv 接受 const char* 通常意味着这里直接传 const char* 是最高效的。

3. 代码性能

  • 审查结果:良好。
  • 说明qputenv 操作非常快,且只在程序启动时执行一次,对整体性能影响可以忽略不计。

4. 代码安全

  • 审查结果:存在潜在风险。
  • 问题
    • 环境变量注入:直接设置环境变量可能会被外部环境覆盖,或者如果 ApplicationManagerConfig 包含用户可控的内容,可能会导致安全问题(虽然作为 App ID 通常是编译时确定的)。
    • 返回值检查qputenv 返回 bool 表示是否设置成功。当前代码忽略了返回值。如果设置失败(例如内存不足),程序可能会在后续行为中出现异常(例如无法正确加载主题或配置),且难以排查原因。
  • 改进建议
    • 建议检查 qputenv 的返回值,并在设置失败时输出警告日志。
    • 确保 ApplicationManagerConfig 的值是可信的。

改进后的代码示例

int main(int argc, char *argv[])
{
    // 使用 QLatin1String 或直接使用 const char* 宏
    // 假设 ApplicationManagerConfig 定义为: #define ApplicationManagerConfig "com.deepin.dde.ApplicationManager"
    
    // 检查环境变量是否设置成功
    if (!qputenv("DSG_APP_ID", ApplicationManagerConfig)) {
        qWarning() << "Failed to set DSG_APP_ID environment variable.";
        // 根据业务需求,可能需要退出程序或继续运行
    }

#ifdef PROFILING_MODE
    auto start = std::chrono::high_resolution_clock::now();
#endif
    // ... 后续代码

总结

这段代码改动本身逻辑简单,主要风险在于宏定义的确定性以及对环境变量设置失败的处理。建议增加返回值检查以提高程序的健壮性。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: BLumia, mhduiy

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

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