Repository navigation
test: run the screensaver bus test without dbus-run-session - #196
Merged
deepin-bot[bot] merged 1 commit intoOct 15, 2026
Merged
Conversation
Reviewer's GuideThe PR replaces dbus-run-session with a shell wrapper that launches an isolated, detached dbus-daemon, avoids inherited test-runner pipes and fakeroot UID spoofing, and treats daemon cleanup as best effort so the screensaver D-Bus test completes reliably on restricted kernels and in packaging builds. 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 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/plugin-qt/power/session/tests/run-with-private-bus.sh" line_range="61-63" />
<code_context>
+bus_address=$(cat "$address_file")
+daemon_pid=$(cat "$pid_file")
+if [ -z "$bus_address" ]; then
+ echo "$0: dbus-daemon did not print a bus address" >&2
+ exit 1
+fi
+
+DBUS_SESSION_BUS_ADDRESS=$bus_address
</code_context>
<issue_to_address>
**Private bus survives failed startup**
When `dbus-daemon` forks but writes no address, the `bus_address` check exits before the normal `kill` block, and the `EXIT` trap only removes temporary files; the private bus remains running after startup fails.
Route this exit through daemon cleanup before returning.
Also at `src/plugin-qt/power/session/tests/run-with-private-bus.sh:47`.
</issue_to_address>
### Comment 2
<location path="src/plugin-qt/power/session/tests/run-with-private-bus.sh" line_range="47" />
<code_context>
+
+address_file=$(mktemp) || exit 1
+pid_file=$(mktemp) || exit 1
+trap 'rm -f "$address_file" "$pid_file"' EXIT HUP INT TERM
+
+# stdin and both output streams to /dev/null: a daemon that survives cleanup
</code_context>
<issue_to_address>
**Signals leave tests running**
When the wrapper receives `HUP`, `INT`, or `TERM` while its command remains alive and does not receive the signal, the signal trap only removes temporary files, so the wrapper remains blocked waiting for the command and never reaches normal daemon cleanup; the command and private bus keep running until it returns.
Make the signal trap terminate the command and run daemon cleanup.
</issue_to_address>
Comment on lines
+61
to
+63
| echo "$0: dbus-daemon did not print a bus address" >&2 | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
🟡 Medium · Private bus survives failed startup
When dbus-daemon forks but writes no address, the bus_address check exits before the normal kill block, and the EXIT trap only removes temporary files; the private bus remains running after startup fails.
Route this exit through daemon cleanup before returning.
Also at src/plugin-qt/power/session/tests/run-with-private-bus.sh:47.
Prompt for AI agents
In `src/plugin-qt/power/session/tests/run-with-private-bus.sh` at lines 61-63:
**Private bus survives failed startup**
When `dbus-daemon` forks but writes no address, the `bus_address` check exits before the normal `kill` block, and the `EXIT` trap only removes temporary files; the private bus remains running after startup fails.
Route this exit through daemon cleanup before returning.
Also at `src/plugin-qt/power/session/tests/run-with-private-bus.sh:47`.|
|
||
| address_file=$(mktemp) || exit 1 | ||
| pid_file=$(mktemp) || exit 1 | ||
| trap 'rm -f "$address_file" "$pid_file"' EXIT HUP INT TERM |
There was a problem hiding this comment.
🟡 Medium · Signals leave tests running
When the wrapper receives HUP, INT, or TERM while its command remains alive and does not receive the signal, the signal trap only removes temporary files, so the wrapper remains blocked waiting for the command and never reaches normal daemon cleanup; the command and private bus keep running until it returns.
Make the signal trap terminate the command and run daemon cleanup.
Prompt for AI agents
In `src/plugin-qt/power/session/tests/run-with-private-bus.sh` at line 47:
**Signals leave tests running**
When the wrapper receives `HUP`, `INT`, or `TERM` while its command remains alive and does not receive the signal, the signal trap only removes temporary files, so the wrapper remains blocked waiting for the command and never reaches normal daemon cleanup; the command and private bus keep running until it returns.
Make the signal trap terminate the command and run daemon cleanup.
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/plugin-qt/power/session/tests/run-with-private-bus.sh" line_range="47" />
<code_context>
+
+address_file=$(mktemp) || exit 1
+pid_file=$(mktemp) || exit 1
+trap 'rm -f "$address_file" "$pid_file"' EXIT HUP INT TERM
+
+# stdin and both output streams to /dev/null: a daemon that survives cleanup
</code_context>
<issue_to_address>
**Early exits leave bus daemons**
When the command is interrupted or times out, or `dbus-daemon` starts without printing a bus address, the trap removes the PID file without signaling the daemon; interruption skips the end-of-script `kill`, and the missing-address path exits immediately, leaving the private bus running.
Make exit and signal cleanup read the PID and signal the daemon before removing the temporary files.
Also at `src/plugin-qt/power/session/tests/run-with-private-bus.sh:63`.
</issue_to_address>|
|
||
| address_file=$(mktemp) || exit 1 | ||
| pid_file=$(mktemp) || exit 1 | ||
| trap 'rm -f "$address_file" "$pid_file"' EXIT HUP INT TERM |
There was a problem hiding this comment.
⚪ Low · Early exits leave bus daemons
When the command is interrupted or times out, or dbus-daemon starts without printing a bus address, the trap removes the PID file without signaling the daemon; interruption skips the end-of-script kill, and the missing-address path exits immediately, leaving the private bus running.
Make exit and signal cleanup read the PID and signal the daemon before removing the temporary files.
Also at src/plugin-qt/power/session/tests/run-with-private-bus.sh:63.
Prompt for AI agents
In `src/plugin-qt/power/session/tests/run-with-private-bus.sh` at line 47:
**Early exits leave bus daemons**
When the command is interrupted or times out, or `dbus-daemon` starts without printing a bus address, the trap removes the PID file without signaling the daemon; interruption skips the end-of-script `kill`, and the missing-address path exits immediately, leaving the private bus running.
Make exit and signal cleanup read the PID and signal the daemon before removing the temporary files.
Also at `src/plugin-qt/power/session/tests/run-with-private-bus.sh:63`.
1. dbus-run-session shuts its bus down by signalling dbus-daemon. On UOS kernels the usec LSM labels that binary deepin_unkillable_t and denies the signal, so dbus-run-session waits forever; the daemon also inherited the test runner's stdout pipe, so ctest never saw EOF and reported a timeout although the test itself passed in under a second. 2. Run the test through run-with-private-bus.sh instead: a dbus-daemon --fork on a private bus whose stdio goes to /dev/null, with best-effort cleanup that never turns "cannot signal the daemon" into a failure. 3. Clear LD_PRELOAD before starting the bus. Under fakeroot libfakeroot fakes getuid() as 0, so the client sent AUTH EXTERNAL 30 while the daemon checked the real uid through SO_PEERCRED and refused the connection, making registerObject and registerService fail in packaging. Log: No user-visible change; the power screensaver test passes in ctest and in the packaging test step instead of timing out or failing. Influence: 1. Run ctest -R power-screensaver-proxy and verify it passes in well under a second rather than timing out at 20 s. 2. Run the packaging step under fakeroot (dh_auto_test) and verify the test passes instead of failing in registerObject. 3. Run the whole suite and verify 15/15 pass with a total time of a few seconds. test: 屏保总线测试不再依赖 dbus-run-session 1. dbus-run-session 靠给 dbus-daemon 发信号来关停总线。UOS 内核的 usec 模块把该二进制标为 deepin_unkillable_t 并拒绝该信号，于是它永久等待； 该 daemon 又继承了测试运行器的 stdout 管道，ctest 收不到 EOF，只能报 超时——而用例本身不到一秒就已通过。 2. 改用 run-with-private-bus.sh 运行：私有总线上以 --fork 启动 dbus-daemon，标准流全部接 /dev/null，清理尽力而为，绝不把"杀不掉 daemon"升级成测试失败。 3. 启动总线前清空 LD_PRELOAD。fakeroot 下 libfakeroot 把 getuid() 伪装成 0，客户端据此发 AUTH EXTERNAL 30，而 daemon 用 SO_PEERCRED 取真实 uid 校验后拒绝连接，导致打包时 registerObject / registerService 失败。 Log: 无用户可见变化；屏保用例在 ctest 与打包测试步骤中均通过，不再超时 或失败。 Influence: 1. 执行 ctest -R power-screensaver-proxy，确认秒级通过而非 20 秒超时。 2. 在 fakeroot 下执行打包测试步骤（dh_auto_test），确认用例通过而非在 registerObject 处失败。 3. 执行全量 ctest，确认 15/15 通过且总耗时数秒。
fly602
force-pushed
the
fix/power-screensaver-test-bus
branch
from
October 15, 2026 12:41
de75ad6 to
3542efd
Compare
deepin pr auto review🤖 AI 代码审查报告总体评分： 📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价： 优秀 ✅ 通过 潜在问题： 建议： 语法正确，逻辑清晰。Shell 脚本使用 set -u 检测未定义变量，trap 确保资源清理，参数校验完整，退出状态正确传播。CMakeLists.txt 语法正确，find_program 使用 REQUIRED 确保依赖检查。 2. 代码质量 ✅评价： 优秀 ✅ 通过 潜在问题： 建议： 代码结构清晰，注释完整。脚本头部注释详尽解释了使用 dbus-daemon 替代 dbus-run-session 的原因（usec LSM 信号拒绝、fakeroot LD_PRELOAD 认证问题），CMakeLists.txt 中也添加了对应说明。SPDX 许可证头部齐全。 3. 代码性能 ✅评价： 优秀 ✅ 通过 潜在问题： 建议： 性能良好，资源使用合理。dbus-daemon 以 --fork 模式启动，stdio 重定向到 /dev/null 避免管道阻塞。临时文件通过 trap 清理。最佳努力清理策略合理，LSM 拒绝信号时不影响测试结果。 4. 代码安全 🔒评价： 优秀 ✅ 通过
安全漏洞详情： 建议： 存在0个安全漏洞。所有变量使用双引号包裹防止分词和通配符展开，临时文件使用 mktemp 安全创建，trap 确保清理，无命令注入风险（使用 $"@" 传递参数），无硬编码密钥或敏感信息。 💡 改进建议代码示例# 当前代码已无需改进，以下为代码审查确认的最佳实践示例：
# 1. 安全的临时文件创建与清理
address_file=$(mktemp) || exit 1
pid_file=$(mktemp) || exit 1
trap 'rm -f "$address_file" "$pid_file"' EXIT HUP INT TERM
# 2. 正确的退出状态传播
"$@"
status=$?
# ... cleanup ...
exit $status
# 3. 最佳努力清理（不影响测试结果）
if [ -n "$daemon_pid" ]; then
kill "$daemon_pid" 2>/dev/null || :
fi本报告由 AI 代码审查工具自动生成 |
mhduiy
approved these changes
Oct 15, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: fly602, 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 |
Contributor
Author
|
/forcemerge |
|
This pr force merged! (status: blocked) |
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
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.
The power screensaver D-Bus test is driven through
dbus-run-session, whichshuts its private bus down by signalling
dbus-daemon. On UOS kernels theusecLSM labels that binarydeepin_unkillable_tand denies the signal:So
dbus-run-sessionwaits forever for its daemon, and because that daemoninherited the test runner's stdout pipe, ctest never sees EOF and reports a
timeout even though the test itself finished and passed in 364 ms.
A second failure appears in the packaging path:
dpkg-buildpackageruns underfakeroot, whoselibfakeroot-sysv.sois injected viaLD_PRELOADand fakesgetuid()as 0. The client then authenticates withAUTH EXTERNAL 30whilethe daemon checks the real uid through
SO_PEERCREDand refuses theconnection, so every
registerObject/registerServicecall fails.This change runs the test through a small wrapper instead:
dbus-daemon --forkprivate bus whose stdio goes to/dev/null, so adaemon that survives cleanup can never hold a pipe of the test runner open;
test failure;
LD_PRELOADcleared before the bus starts, restoring the real uid.debian/changelogis intentionally not touched; the version bump is generatedby the update-changelog workflow.
Verification
ctest(whole suite)power-screensaver-proxytimed out at 20 sctest -R power-screensaver-proxyfakeroot -- dh_auto_test(packaging)registerObjectfailedctest | cat,make testResidual cost: on kernels where the LSM denies the signal, one idle
dbus-daemonis left behind (a few MB); it no longer affects the test result.Summary by Sourcery
Run the power screensaver D-Bus test through a resilient private-bus wrapper instead of dbus-run-session.
Bug Fixes:
Enhancements:
Tests: