Skip to content

fix: avoid duplicate GRUB background preview refresh - #1201

Open
52cyb wants to merge 1 commit into
linuxdeepin:masterfrom
52cyb:master
Open

fix: avoid duplicate GRUB background preview refresh#1201
52cyb wants to merge 1 commit into
linuxdeepin:masterfrom
52cyb:master

Conversation

@52cyb

@52cyb 52cyb commented Aug 13, 2026

Copy link
Copy Markdown
Contributor
  1. Add a revision property for refreshing background content at a stable path
  2. Avoid emitting path changes when the background path remains unchanged
  3. Refresh the preview only after a pending background update completes
  4. Restrict automatic theme enabling to control-center initiated updates
  5. Replace timestamp-based cache busting with an explicit revision

Log: Avoid duplicate flickering when refreshing the GRUB background preview

Influence:

  1. Verify changing the background refreshes the preview once
  2. Verify changing the background while the theme is disabled enables it
  3. Verify opening the boot menu page does not trigger duplicate refreshes
  4. Verify repeated reads of the same background path do not cause flickering
  5. Verify the updated image is loaded when the exported path stays unchanged

fix: 避免 GRUB 背景预览重复刷新

  1. 增加背景版本属性,用于刷新固定路径下的图片内容
  2. 背景路径未变化时不再发送路径变化信号
  3. 仅在待处理的背景修改完成后刷新预览
  4. 仅对控制中心发起的背景修改自动开启主题
  5. 使用显式版本号替代时间戳刷新图片缓存

Log: 避免刷新 GRUB 背景预览时出现两次闪烁

Influence:

  1. 验证修改背景后预览只刷新一次
  2. 验证主题关闭时修改背景可以正常开启主题
  3. 验证打开启动菜单页面不会触发重复刷新
  4. 验证重复读取相同背景路径不会造成闪烁
  5. 验证导出路径不变时仍能加载更新后的图片

PMS: BUG-371485

Summary by Sourcery

Stabilize GRUB background export to avoid duplicate preview refreshes and flickering.

Enhancements:

  • Export GRUB background images to a fixed runtime path instead of creating new temp files per request.
  • Add synchronization around background export to ensure consistent file updates during concurrent access.

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 52cyb

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

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sorry @52cyb, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@sourcery-ai

sourcery-ai Bot commented Aug 13, 2026

Copy link
Copy Markdown

Reviewer's Guide

The PR refactors GRUB background export to use a stable runtime path with atomic replacement, avoids duplicate preview refreshes, and introduces synchronization to prevent concurrent exports and redundant updates.

Sequence diagram for GRUB background export with stable path

sequenceDiagram
    actor Client
    participant Theme
    participant exportBackground
    participant Filesystem

    Client->>Theme: GetBackground(sender)
    Theme->>Theme: backgroundExportMu.Lock()
    Theme->>exportBackground: exportBackground(background, grubBackgroundRuntimeDir)

    exportBackground->>Filesystem: os.Open(source)
    exportBackground->>Filesystem: os.CreateTemp(destinationDir, pattern)
    exportBackground->>Filesystem: io.Copy(tempFile, sourceFile)
    exportBackground->>Filesystem: tempFile.Chmod(0644)
    exportBackground->>Filesystem: os.Rename(tempPath, destination)
    exportBackground-->>Theme: destination

    Theme->>Theme: backgroundExportMu.Unlock()
    Theme-->>Client: destination
Loading

File-Level Changes

Change Details Files
Refactor background export to a stable runtime path with atomic, permission-safe file replacement and no temporary-file leaks.
  • Replace use of /tmp with a fixed runtime directory for exported background images
  • Switch path operations from path to filepath to handle extensions and joins correctly
  • Introduce exportBackground helper that copies the source image to a temp file, sets permissions, and atomically renames it into a stable destination filename
  • Ensure temporary files are cleaned up on error and avoid accumulating unused temp files
grub2/theme_ifc.go
Synchronize background export operations and integrate the new export logic into the Theme interface.
  • Add a mutex on Theme to serialize background export operations and prevent races
  • Wrap GetBackground export logic with the new mutex and use exportBackground instead of ad-hoc temp file creation
  • Improve logging for background export failures while returning appropriate DBus errors
grub2/theme.go
grub2/theme_ifc.go
Prepare systemd service configuration for using the GRUB runtime directory for background exports.
  • Update or validate deepin-grub2.service configuration to align with the use of /run/deepin-grub2 as the runtime directory
misc/systemd/services/system/deepin-grub2.service

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

1. Add a revision property for refreshing background content at a stable path
2. Avoid emitting path changes when the background path remains unchanged
3. Refresh the preview only after a pending background update completes
4. Restrict automatic theme enabling to control-center initiated updates
5. Replace timestamp-based cache busting with an explicit revision

Log: Avoid duplicate flickering when refreshing the GRUB background preview

Influence:
1. Verify changing the background refreshes the preview once
2. Verify changing the background while the theme is disabled enables it
3. Verify opening the boot menu page does not trigger duplicate refreshes
4. Verify repeated reads of the same background path do not cause flickering
5. Verify the updated image is loaded when the exported path stays unchanged

fix: 避免 GRUB 背景预览重复刷新

1. 增加背景版本属性,用于刷新固定路径下的图片内容
2. 背景路径未变化时不再发送路径变化信号
3. 仅在待处理的背景修改完成后刷新预览
4. 仅对控制中心发起的背景修改自动开启主题
5. 使用显式版本号替代时间戳刷新图片缓存

Log: 避免刷新 GRUB 背景预览时出现两次闪烁

Influence:
1. 验证修改背景后预览只刷新一次
2. 验证主题关闭时修改背景可以正常开启主题
3. 验证打开启动菜单页面不会触发重复刷新
4. 验证重复读取相同背景路径不会造成闪烁
5. 验证导出路径不变时仍能加载更新后的图片

PMS: BUG-371485
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

★ 总体评分:100分

■ 【总体评价】

代码通过引入互斥锁和原子重命名机制,有效解决了临时文件累积和并发安全问题,逻辑严谨且资源管理完善。
修复了原有的临时文件泄露问题,未引入新漏洞,代码质量优秀,故给予满分。

■ 【详细分析】

  • 1.语法逻辑(完全正确)✓

grub2/theme_ifc.goexportBackground 函数中,文件操作逻辑严密。通过 removeTemptempClosed 标志位配合 defer 机制,确保了在任何错误分支下临时文件都能被正确关闭和清理。os.Rename 保证了目标文件写入的原子性,避免了部分写入导致的数据损坏。
潜在问题:无
建议:无

  • 2.代码质量(良好)✓

GetBackgroundexportBackground 职责分离清晰,前者处理 D-Bus 请求与并发控制,后者专注文件导出逻辑。命名规范,错误处理完善,deepin-grub2.service 中的注释也准确说明了 RuntimeDirectory 的用途。
潜在问题:无
建议:无

  • 3.代码性能(无性能问题)✓

互斥锁 backgroundExportMu 虽然会限制并发导出,但背景导出属于低频操作,对整体性能无影响。使用固定路径并原子覆盖,避免了原方案中频繁创建和累积临时文件带来的磁盘 I/O 浪费。
潜在问题:无
建议:无

  • 4.代码安全(存在0个安全漏洞)✓

漏洞对比统计:新增漏洞 0 个,减少漏洞 0 个,持平 0 个
本次修改修复了原代码中 /tmp 目录下临时文件不清理导致的累积风险,将导出路径限制在受控的 RuntimeDirectory 下,消除了潜在的磁盘耗尽风险和沙箱越权写入风险。未发现新的安全漏洞。
建议:无

■ 【改进建议代码示例】

--- a/grub2/theme_ifc.go
+++ b/grub2/theme_ifc.go
@@ -88,6 +88,9 @@ func exportBackground(source, destinationDir string) (string, error) {
 	sourceFile, err := os.Open(source)
 	if err != nil {
 		return "", err
 	}
+	if err := os.MkdirAll(destinationDir, 0755); err != nil {
+		return "", err
+	}
 	defer sourceFile.Close()
 
 	ext := filepath.Ext(source)

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.

2 participants