-
Notifications
You must be signed in to change notification settings - Fork 42
fix: set DSG_APP_ID to prevent self-identification deadlock #321
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
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
Reviewer's guide (collapsed on small PRs)Reviewer's GuideSets 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 setsequenceDiagram
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
Flow diagram for DSG_APP_ID preventing self-identification deadlockflowchart 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"]
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this 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>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
deepin pr auto review这段代码是在 1. 语法逻辑
2. 代码质量
3. 代码性能
4. 代码安全
改进后的代码示例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
// ... 后续代码总结这段代码改动本身逻辑简单,主要风险在于宏定义的确定性以及对环境变量设置失败的处理。建议增加返回值检查以提高程序的健壮性。 |
|
[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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
如果不手动设置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:
fix: 设置DSG_APP_ID环境变量防止自身识别死锁
在应用程序初始化之前设置DSG_APP_ID环境变量为ApplicationManagerConfig。这 可以防止应用程序管理器在访问自身dconfig时调用自己的Identify接口,从而避
免死锁发生。
Log: 修复了应用程序管理器访问自身配置时可能出现的死锁问题
Influence:
PMS: BUG-345379
Summary by Sourcery
Bug Fixes: