Skip to content

fix: preserve wallpaper FD imports and reuse valid cached images - #199

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
yixinshark:fix/wallpaper-fd-cache-bug-335593
Oct 16, 2026
Merged

deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
yixinshark:fix/wallpaper-fd-cache-bug-335593

Conversation

@yixinshark

@yixinshark yixinshark commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Repeated FD imports could fail to replace existing images, while deleting imported sources after scaling could invalidate fallback paths and later queued sizes. Publish imports atomically, preserve the source, and return complete valid size caches before reading the FD. Missing cache files are regenerated, and worker shutdown is synchronized.

The scaling, center-crop and pixmix algorithms and D-Bus interfaces are unchanged. Imported source files now remain in the cache for fallback and subsequent processing, which can increase disk usage.

Pms: BUG-335593

Validation

  • Production plugin built successfully on physical x86_64 hardware with Qt 6.8.0.
  • All 11 QtTest checks passed.
  • Actual system D-Bus regression probe: 12 checks passed; the original plugin failed 5 of the same checks.
  • Compatibility integration suite: 52 assertions passed; after restarting the service, 53 assertions passed as an ordinary user.
  • For a generated 1920x1080 BMP, 100 cached calls improved from 6.73 ms to 0.79 ms at P95; two scaled sizes completed in about 54 ms, and a removed cache entry was regenerated in about 20 ms.
  • git diff --check passed. The original test-machine plugin was restored and its hash verified.

Validation covers service behavior and interfaces; lock-screen/login visuals and prolonged workloads were not exercised.

Summary by Sourcery

Fix wallpaper FD imports and cache handling to safely reuse valid images while preserving sources needed for fallback and deferred processing.

Bug Fixes:

  • Preserve and atomically replace imported wallpaper sources so repeated imports, queued scaling tasks, and fallback paths remain valid.
  • Reuse complete valid cached image sets before reading file descriptors, while regenerating missing or invalid cache entries.
  • Reject invalid imports and ensure failed descriptor reads do not leave partial cache files.

Enhancements:

  • Synchronize scaling worker shutdown and retain imported source files for later processing and fallback use.

Build:

  • Exclude test sources from the main wallpapercache source list and support configurable cache directories for isolated builds.

Tests:

  • Add Qt unit coverage for repeated imports, invalid inputs, cache reuse and regeneration, fallback preservation, and worker shutdown.

Publish FD imports atomically so repeated imports replace earlier images and
failed reads leave published images intact. Validate cache keys, retry interrupted
reads, and keep imported sources readable for fallback clients and queued sizes.
Reuse complete, readable size caches before consuming the descriptor, regenerate
missing entries, and synchronize worker shutdown. Add isolated QtTest coverage
and exclude test sources from the production plugin.

原子发布 FD 导入图片,使重复导入正确替换旧图,读取失败时保留已发布图片。
校验缓存键并重试中断读取,保留可读原图供客户端回退和后续尺寸处理使用。
完整有效的尺寸缓存命中时不再读取描述符,补齐缺失缓存,并同步工作线程退出。
增加使用独立缓存目录的 QtTest 回归测试,并从生产插件构建中排除测试源码。

Log: preserve wallpaper FD imports and reuse valid cached images
Pms: BUG-335593
Change-Id: I3bfccfee1e4d23ee2c64997fe8f5f81e7de8c697
@sourcery-ai

sourcery-ai Bot commented Oct 16, 2026

Copy link
Copy Markdown

Reviewer's Guide

The PR hardens FD imports with validated temporary-file copying and atomic publication, preserves imported sources for fallback and future scaling, uses complete on-disk cache hits without consuming descriptors while regenerating missing sizes, and adds deterministic worker shutdown plus focused unit-test coverage.

Sequence diagram for atomic wallpaper FD import and cache reuse

sequenceDiagram
    participant Client
    participant Service as WallpaperCacheService
    participant Cache as CachedWallpaper
    participant FD as ImageFD
    participant Disk as CacheDisk

    Client->>Service: GetProcessedImagePathByFd(fd, imagePathMd5, sizeArray)
    Service->>Cache: findCachedImagePaths(imagePathMd5, sizes)
    Cache->>Disk: Validate every cached file
    alt Complete valid cache
        Cache-->>Service: cached paths
        Service-->>Client: cached paths
    else Cache miss or missing size
        Service->>Service: saveImageFromFd(fd, imagePathMd5)
        Service->>FD: read()
        Service->>Disk: Write temporary import
        Service->>Disk: QImageReader::imageFormat()
        Service->>Disk: rename() to publish atomically
        Service->>Cache: getCachedImagePaths(destinationPath, sizes, true)
        Cache-->>Service: generated or reused paths
        Service-->>Client: processed paths
    end
Loading

State diagram for synchronized scaling worker shutdown

stateDiagram-v2
    [*] --> Waiting
    Waiting --> Processing: tasks available
    Waiting --> Stopped: m_stop
    Processing --> Waiting: task complete
    Processing --> Stopped: m_stop
    Stopped --> [*]
Loading

Flow diagram for preserving imported wallpaper sources

flowchart LR
    A[FD import] --> B[Temporary private file]
    B --> C[Validate image format]
    C --> D[Atomic rename to MD5 format path]
    D --> E[Preserved source cache entry]
    E --> F[Fallback and queued size processing]
Loading

File-Level Changes

Change Details Files
Make FD image imports safe to replace and retain them as reusable cache sources.
  • Copy descriptors into private temporary files, validate MD5 keys and image formats, then atomically rename into the published cache path.
  • Allow repeated imports to replace existing images without exposing partial files.
  • Stop deleting imported sources after scaling so queued sizes and fallback responses remain readable.
  • Add coverage for replacement, invalid input, descriptor failures, key validation, and source retention.
src/plugin-qt/wallpapercache/wallpapercacheservice.cpp
src/plugin-qt/wallpapercache/scaleimagethread.cpp
src/plugin-qt/wallpapercache/tests/test_wallpaper_cache_unit.cpp
Reuse only complete, readable cached outputs and regenerate missing entries.
  • Validate every requested cached path on disk for existence, readability, regular-file status, and nonzero size.
  • Return complete cache hits before reading the incoming FD, while still queueing missing sizes for partial hits.
  • Route non-blurred FD requests through the cache-first path and add tests proving descriptors are not consumed on valid hits and missing entries regenerate.
src/plugin-qt/wallpapercache/cachedwallpaper.cpp
src/plugin-qt/wallpapercache/cachedwallpaper.h
src/plugin-qt/wallpapercache/wallpapercacheservice.cpp
src/plugin-qt/wallpapercache/tests/test_wallpaper_cache_unit.cpp
Improve worker shutdown and isolate unit-test cache storage.
  • Keep the scaling worker asleep while idle and exit deterministically when stop is requested.
  • Exclude tests from the production source glob and compile unit tests against a build-local cache directory.
  • Add tests for stopping an idle worker and synchronized worker completion.
src/plugin-qt/wallpapercache/scaleimagethread.cpp
src/plugin-qt/wallpapercache/CMakeLists.txt
src/plugin-qt/wallpapercache/cachedwallpaper.h
src/plugin-qt/wallpapercache/tests/CMakeLists.txt
src/plugin-qt/wallpapercache/tests/test_wallpaper_cache_unit.cpp

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

@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.

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/wallpapercache/wallpapercacheservice.cpp" line_range="232-235" />
<code_context>
+        qWarning() << "Failed to detect imported image format";
+        return {};
+    }
+    const QString destinationPath = kWallpaperCacheDir + "/" + imagePathMd5 + "." + QString::fromLatin1(format);
+    // POSIX rename atomically replaces an earlier import, including a longer
+    // image. Existing readers retain their opened file until they finish.
+    if (::rename(QFile::encodeName(originFile.fileName()).constData(),
+                 QFile::encodeName(destinationPath).constData()) != 0) {
+        qWarning() << "Failed to publish imported image:" << errno;
</code_context>
<issue_to_address>
**Old imports remain on disk**

When the same key is imported in a different image format, `rename` replaces only the destination with the current format suffix, leaving any earlier `<key>.<format>` source files untouched. Repeatedly changing formats for a key therefore accumulates obsolete imported images in the cache directory.

Before or after publishing the new import, remove obsolete source files for that key without deleting the newly published file.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

Comment thread src/plugin-qt/wallpapercache/wallpapercacheservice.cpp
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

🤖 AI 代码审查报告

总体评分: 98 分 (通过阈值: 70分)

Pass


📊 总体评价

项目 结果
审查结论 优秀
评分详情 代码质量高,修复了重要的安全问题和逻辑缺陷。路径遍历防护、原子文件操作、缓存验证等改进显著提升了代码安全性和可靠性。单元测试覆盖全面。仅存在少量注释缺失的轻微问题。

🔍 详细分析

1. 语法逻辑 ✅

评价: 优秀 ✅ 通过

潜在问题:
✅ 未发现明显问题

建议: ['代码逻辑正确,ScaleImageThread::run()的竞态条件修复采用标准等待-检查模式,处理得当', 'findCachedImagePaths方法的文件验证逻辑完善,正确处理了文件不存在、不可读、零大小等边界情况', 'saveImageFromFd的EINTR重试和errno处理符合POSIX最佳实践']


2. 代码质量 ✅

评价: 优秀 ✅ 通过

潜在问题:
✅ 未发现明显问题

建议: ['建议为findCachedImagePaths添加函数文档注释,说明其用于查找并验证缓存图片路径的行为', '单元测试覆盖全面,包含9个测试用例,覆盖了正常流程、边界条件和安全场景', 'CMakeLists.txt正确排除了测试目录,避免测试代码编译进主库']


3. 代码性能 ✅

评价: 优秀 ✅ 通过

潜在问题:
✅ 未发现明显问题

建议: ['缓冲区从4KB增加到64KB,显著提升了文件I/O吞吐量', 'GetProcessedImagePathByFd中的缓存命中优化避免了不必要的FD读取和文件写入', '使用constFind替代contains+operator[],避免了重复哈希查找', 'GetProcessedImagePathByFdWithBlur在不需要模糊处理时直接委托,避免了不必要的处理']


4. 代码安全 🔒

评价: 优秀 ✅ 通过

🔐 发现 0 个安全漏洞

安全漏洞详情:
✅ 未发现安全漏洞

建议: ['路径遍历防护:saveImageFromFd中添加了MD5格式验证(\A[0-9a-fA-F]{32}\z),防止通过构造imagePathMd5参数进行目录遍历攻击', '原子文件操作:使用QTemporaryFile + POSIX rename()实现原子性文件替换,防止并发读取到部分写入的文件', '文件描述符验证:检查fd.fileDescriptor() < 0,防止无效FD处理', 'EINTR处理:正确处理被信号中断的系统调用,进行重试', '文件有效性验证:findCachedImagePaths验证文件存在性、可读性和非零大小,防止返回失效缓存', '防御性编程充分,无安全漏洞']


💡 改进建议代码示例

// 暂无代码示例

本报告由 AI 代码审查工具自动生成

@yixinshark

Copy link
Copy Markdown
Contributor Author

/forcemerge

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: robertkill, 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

@deepin-bot

deepin-bot Bot commented Oct 16, 2026

Copy link
Copy Markdown

This pr force merged! (status: blocked)

@deepin-bot
deepin-bot Bot merged commit 1528393 into linuxdeepin:master Oct 16, 2026
6 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