fix: improve process name resolution for DBus clients - #154
Merged
Conversation
1. Change process name detection to prefer /proc/{pid}/exe symlink for
accuracy
2. Add fallback to /proc/{pid}/cmdline on permission errors (daemon runs
as deepin-daemon)
3. Ensure command line content is non-empty before returning
4. In getAppid(): avoid caching stale app names across different DBus
services
5. Move m_appName assignment outside the isEmpty guard to refresh per
connection
6. Always return testappid only when not called from DBus
Log: Improved process identification for DConfig clients
Influence:
1. Test DConfig access from multiple DBus services with different PIDs
2. Verify process name is correctly resolved for processes with
permission constraints
3. Verify cached app name is refreshed when the same connection handles
different DBus services
4. Confirm fallback to PID number when both exe and cmdline fail
5. Test non-DBus calls (e.g., unit tests) return "testappid" correctly
fix: 改进DBus客户端的进程名称解析
1. 将进程名称检测改为优先使用 /proc/{pid}/exe 符号链接以提高准确性
2. 在权限错误时回退到 /proc/{pid}/cmdline（守护进程以deepin-daemon身份
运行）
3. 确保命令行内容非空后才返回
4. 在getAppid()中：避免在不同DBus服务间缓存过时的应用名称
5. 将m_appName赋值移出isEmpty检查，确保每个连接刷新
6. 仅在非DBus调用时返回"testappid"
Log: 改进DConfig客户端的进程识别功能
Influence:
1. 测试从多个不同DBus服务访问DConfig，验证PID正确
2. 验证受限权限下进程名称能正确解析
3. 验证同一连接处理不同DBus服务时，缓存的app名称刷新
4. 确认当exe和cmdline都失败时回退到PID
5. 测试非DBus调用（如单元测试）正确返回"testappid"
Reviewer's GuideImproves DConfig daemon process name resolution for DBus clients by preferring /proc/{pid}/exe with a robust fallback strategy, and fixes app ID caching so that application names are refreshed per DBus service and test calls consistently return a static app ID. Sequence diagram for updated DSGConfigConn::getAppid behaviorsequenceDiagram
participant Client
participant DSGConfigConn
participant DBusConnection as QDBusConnection
participant DBusInterface as QDBusConnectionInterface
Client->>DSGConfigConn: getAppid
alt calledFromDBus
DSGConfigConn->>DSGConfigConn: calledFromDBus
DSGConfigConn->>DSGConfigConn: message
DSGConfigConn->>DSGConfigConn: compare m_lastService with service
alt m_lastService != service
DSGConfigConn->>DBusConnection: connection
DBusConnection->>DBusInterface: servicePid service
DBusInterface-->>DSGConfigConn: pid
DSGConfigConn->>DSGConfigConn: getProcessNameByPid pid
DSGConfigConn->>DSGConfigConn: update m_lastService and m_appName
end
DSGConfigConn-->>Client: m_appName
else not calledFromDBus
DSGConfigConn-->>Client: QString testappid
end
Flow diagram for updated getProcessNameByPid logicflowchart TD
A[Start getProcessNameByPid pid] --> B[QFile::symLinkTarget /proc/pid/exe]
B --> C{exePath is empty?}
C -- No --> D[return exePath]
C -- Yes --> E[Open /proc/pid/cmdline]
E --> F{file opened?}
F -- No --> I[return QString::number pid]
F -- Yes --> G[readLine and build cmd]
G --> H{cmd is empty?}
H -- No --> J[return cmd]
H -- Yes --> I[return QString::number pid]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In getProcessNameByPid(), returning QFile::symLinkTarget("/proc/%1/exe") will give the full executable path rather than just the process name; if the intent is to keep the previous behavior (a short name), consider wrapping it with QFileInfo(exePath).fileName().
- The new m_lastService cache in getAppid() only invalidates when the service string changes; if a DBus service can respawn with a new PID but reuse the same service name, you may still end up with a stale m_appName and should consider keying the cache on PID (or service+PID) instead.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In getProcessNameByPid(), returning QFile::symLinkTarget("/proc/%1/exe") will give the full executable path rather than just the process name; if the intent is to keep the previous behavior (a short name), consider wrapping it with QFileInfo(exePath).fileName().
- The new m_lastService cache in getAppid() only invalidates when the service string changes; if a DBus service can respawn with a new PID but reuse the same service name, you may still end up with a stale m_appName and should consider keying the cache on PID (or service+PID) instead.
## Individual Comments
### Comment 1
<location path="dconfig-center/dde-dconfig-daemon/dconfigconn.cpp" line_range="244-245" />
<code_context>
QString DSGConfigConn::getAppid() const
{
- if (m_appName.isEmpty()) {
- if (calledFromDBus()) {
- const QString &service = message().service();
+ if (calledFromDBus()) {
+ const QString &service = message().service();
+ if (m_lastService != service) {
+ const_cast<DSGConfigConn *>(this)->m_lastService = service;
const_cast<DSGConfigConn *>(this)->m_appName = getProcessNameByPid(connection().interface()->servicePid(service));
</code_context>
<issue_to_address>
**issue (bug_risk):** Caching only on service name changes may miss updates when the PID behind a well-known service changes.
Because m_appName is updated only when the DBus service string changes, a well-known service that gets re-bound to a different process can end up with a new PID but the same service name. In that case, m_appName would still refer to the old process. If this value is used for logging, auditing, or access control, the stale mapping could be misleading.
To avoid this, consider also caching the last PID (e.g., m_lastPid) and refreshing m_appName when either the service name or its PID changes, so restarts behind a stable service name are handled correctly.
</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★ 总体评分：85分■ 【总体评价】
■ 【详细分析】
■ 【改进建议代码示例】 // dconfigconn.h
QString m_appName;
QString m_lastService;
// 建议修改为：
mutable QString m_appName;
mutable QString m_lastService;
};
// dconfigconn.cpp
QString DSGConfigConn::getAppid() const
{
if (calledFromDBus()) {
const QString &service = message().service();
if (m_lastService != service) {
m_lastService = service;
m_appName = getProcessNameByPid(connection().interface()->servicePid(service));
}
return m_appName;
}
return QString("testappid");
}
// dconfig_global.h
inline QString getProcessNameByPid(const uint pid)
{
#ifdef Q_OS_LINUX
// Prefer /proc/{pid}/exe symlink for process name.
// May fail due to permission (daemon runs as deepin-daemon, target process
// may belong to another user). Fallback to /proc/{pid}/cmdline in that case.
const QString exePath = QFile::symLinkTarget(QString("/proc/%1/exe").arg(pid));
if (!exePath.isEmpty()) {
// 建议提取纯文件名以保持一致性，或根据下游需求决定是否保留路径
return QFileInfo(exePath).fileName();
}
// Fallback: /proc/{pid}/cmdline is world-readable (0444)
const QString cmdlinePath = QString("/proc/%1/cmdline").arg(pid);
QFile file(cmdlinePath);
if (file.open(QIODevice::ReadOnly)) {
const QByteArray &name = file.readLine();
const QString cmd = name.split('\0').join(" ").trimmed();
if (!cmd.isEmpty())
return cmd;
}
#endif // Q_OS_LINUX
return QString::number(pid);
} |
BLumia
approved these changes
Jul 14, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, BLumia 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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
accuracy
as deepin-daemon)
services
connection
Log: Improved process identification for DConfig clients
Influence:
permission constraints
different DBus services
fix: 改进DBus客户端的进程名称解析
运行）
Log: 改进DConfig客户端的进程识别功能
Influence:
Summary by Sourcery
Improve process name resolution for DConfig DBus clients and adjust app ID handling for DBus vs non-DBus connections.
Bug Fixes:
Enhancements: