Skip to content

fix: change gnome-keyring-daemon startup order#191

Merged
fly602 merged 1 commit intolinuxdeepin:masterfrom
fly602:master
Feb 6, 2026
Merged

fix: change gnome-keyring-daemon startup order#191
fly602 merged 1 commit intolinuxdeepin:masterfrom
fly602:master

Conversation

@fly602
Copy link
Contributor

@fly602 fly602 commented Feb 6, 2026

Changed the startup sequence to launch gnome-keyring-daemon before deepin-keyring-whitebox. This prevents potential issues where the keyring's pkcs11 module might not be fully loaded when attempting to unlock, ensuring proper initialization of all keyring components.

Influence:

  1. Test system startup and login process
  2. Verify keyring functionality works correctly
  3. Check that pkcs11 module is properly loaded
  4. Test application access to keyring services
  5. Verify no race conditions during session initialization

fix: 调整 gnome-keyring-daemon 启动顺序

修改启动顺序,先启动 gnome-keyring-daemon 再启动 deepin-keyring- whitebox。这可以防止在尝试解锁时 keyring 的 pkcs11 模块可能尚未完全加载
的问题,确保所有 keyring 组件正确初始化。
PMS: BUG-350091
Influence:

  1. 测试系统启动和登录过程
  2. 验证 keyring 功能正常工作
  3. 检查 pkcs11 模块是否正确加载
  4. 测试应用程序对 keyring 服务的访问
  5. 验证会话初始化期间没有竞态条件

Summary by Sourcery

Bug Fixes:

  • Prevent race conditions during session initialization that could leave the keyring pkcs11 module unavailable when unlocking or accessing keyring services.

Changed the startup sequence to launch gnome-keyring-daemon before
deepin-keyring-whitebox. This prevents potential issues where the
keyring's pkcs11 module might not be fully loaded when attempting to
unlock, ensuring proper initialization of all keyring components.

Influence:
1. Test system startup and login process
2. Verify keyring functionality works correctly
3. Check that pkcs11 module is properly loaded
4. Test application access to keyring services
5. Verify no race conditions during session initialization

fix: 调整 gnome-keyring-daemon 启动顺序

修改启动顺序,先启动 gnome-keyring-daemon 再启动 deepin-keyring-
whitebox。这可以防止在尝试解锁时 keyring 的 pkcs11 模块可能尚未完全加载
的问题,确保所有 keyring 组件正确初始化。
PMS: BUG-350091
Influence:
1. 测试系统启动和登录过程
2. 验证 keyring 功能正常工作
3. 检查 pkcs11 模块是否正确加载
4. 测试应用程序对 keyring 服务的访问
5. 验证会话初始化期间没有竞态条件
@sourcery-ai
Copy link

sourcery-ai bot commented Feb 6, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Adjusts the X session startup script so that gnome-keyring-daemon is started before deepin-keyring-whitebox, avoiding races where the keyring’s PKCS#11 module is not yet loaded when deepin-keyring-whitebox runs.

File-Level Changes

Change Details Files
Reorder keyring-related startup commands so gnome-keyring-daemon is initialized before deepin-keyring-whitebox during X session startup.
  • Modify the Xsession.d script to launch gnome-keyring-daemon earlier in the sequence than deepin-keyring-whitebox
  • Ensure any environment variables or socket paths required by deepin-keyring-whitebox are available after gnome-keyring-daemon starts
  • Adjust comments or documentation in the script (if present) to reflect the new startup order and rationale
misc/Xsession.d/97deepin-keyring-wb

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 reviewed your changes and they look great!


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

这段代码的 diff 展示了对 Deepin 桌面环境(DDE)会话启动脚本 97deepin-keyring-wb 的修改。主要的改动是调整了启动 gnome-keyring-daemondeepin-keyring-whitebox 的顺序。

以下是对该 diff 的详细审查意见,包括语法逻辑、代码质量、代码性能和代码安全方面:

1. 语法逻辑

  • 审查意见:语法正确,逻辑清晰。
  • 分析
    • 脚本使用了标准的 POSIX shell (#!/bin/sh) 语法。
    • [ -x ... ] 用于判断可执行文件是否存在且具有执行权限,逻辑正确。
    • || true 的用法确保了即使前面的命令执行失败,脚本也不会因此退出(返回非零状态码),这在会话启动脚本中是合理的,防止因为一个后台服务启动失败而导致整个桌面环境无法启动。
    • 修改后的逻辑是:先启动 gnome-keyring-daemon,再启动 deepin-keyring-whitebox

2. 代码质量

  • 审查意见:代码质量良好,但存在可改进的细节。
  • 分析
    • 顺序调整:将 gnome-keyring-daemon 的启动移至 deepin-keyring-whitebox 之前。这通常是为了解决依赖关系。如果 deepin-keyring-whitebox 需要与 gnome-keyring-daemon 通信或依赖其提供的某些服务(如 SSH agent),确保 gnome-keyring-daemon 先启动是必要的。这是一个合理的改进。
    • 冗余输出echo "start ..." 语句会在 .xsession-errors 或系统日志中产生输出。在调试阶段很有用,但在生产环境中,过多的日志可能会干扰问题排查或占用不必要的 I/O。建议考虑移除或将其改为仅当脚本处于调试模式时才输出。
    • 硬编码路径/usr/bin/... 是绝对路径,这是好的做法,避免了依赖 PATH 环境变量。

3. 代码性能

  • 审查意见:对性能影响微乎其微。
  • 分析
    • 该脚本仅在用户登录图形界面时执行一次。
    • [ -x ... ] 检查文件系统,开销极小。
    • 启动两个守护进程是主要开销,但这属于功能必需,无法通过优化脚本本身来减少。
    • 调换顺序不会带来性能上的显著差异。

4. 代码安全

  • 审查意见:基本安全,但需注意守护进程的权限管理。
  • 分析
    • [ -x ... ] 检查了权限,防止了误执行不存在或无权限的脚本。
    • || true 的使用虽然保证了登录流程不被中断,但也可能掩盖关键的安全服务启动失败的问题(例如密钥环守护进程启动失败)。如果密钥环未启动,后续的自动解密或 SSH 登录可能会失败,且用户可能不知道原因。
    • 建议:如果安全策略要求严格,可以考虑移除 || true,或者将错误信息重定向到特定的日志文件中以便后续审计,而不是简单地忽略错误。

改进建议

基于以上分析,以下是优化后的代码建议:

#!/bin/sh
# Start deepin-keyring-whitebox and gnome-keyring-daemon for DDE session

# 定义一个日志函数(可选,用于减少标准输出干扰)
log_msg() {
    # 如果需要调试,可以取消下面的注释
    # echo "$@"
    :
}

# 启动 gnome-keyring-daemon
if [ -x /usr/bin/gnome-keyring-daemon ]; then
    log_msg "start gnome-keyring-daemon with components secrets,pkcs11,ssh"
    # 移除 || true 可以让启动失败更明显,或者可以记录错误
    /usr/bin/gnome-keyring-daemon --start --components=secrets,pkcs11,ssh
fi 

# 启动 deepin-keyring-whitebox
if [ -x /usr/bin/deepin-keyring-whitebox ]; then
    log_msg "start deepin-keyring-whitebox client"
    /usr/bin/deepin-keyring-whitebox --opt-client=waitfifonotify
fi

总结
原始的 diff 修改(调整启动顺序)在逻辑上是正确的,解决了潜在的依赖问题。代码本身没有严重的安全漏洞或性能问题。主要的改进空间在于日志处理的精细化以及是否需要静默忽略启动错误。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: fly602, yixinshark

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

@fly602 fly602 merged commit 076e63a into linuxdeepin:master Feb 6, 2026
15 of 17 checks passed
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